From Bugzilla Helper: User-Agent: Mozilla/5.0 (compatible; Konqueror/3.3; Linux) (KHTML, like Gecko) Description of problem: Just noticed booting RHEL3 U4 beta1 on an x445 or x440 that the cpu_sibling_map looks broken. cpu_sibling_map[0] = 2 cpu_sibling_map[1] = 3 cpu_sibling_map[2] = 0 cpu_sibling_map[3] = 1 cpu_sibling_map[4] = 0 cpu_sibling_map[5] = 1 cpu_sibling_map[6] = 0 cpu_sibling_map[7] = 1 cpu_sibling_map[8] = 0 cpu_sibling_map[9] = 1 cpu_sibling_map[10] = 0 cpu_sibling_map[11] = 1 cpu_sibling_map[12] = 0 cpu_sibling_map[13] = 1 cpu_sibling_map[14] = 0 cpu_sibling_map[15] = 1 Version-Release number of selected component (if applicable): kernel-smp-2.4.21-21.EL How reproducible: Always Steps to Reproduce: 1. Install RHEL3U4 beta1 on x445/x440 2. Boot 3. Look at /var/log/dmesg Actual Results: cpu_sibling_map[] is incorrect. Expected Results: cpu_sibling_map[] should be correct. I believe the proper pairing is (0,8)(1,9)(2,10)(3,11)(4,12)(5,13) (6,14)(7,15) Additional info: I believe there were previous disputed attempts by James Cleverdon at fixing this, but I don't recall exactly.
Created attachment 105565 [details] Full dmesg Full dmseg from x445 bootup.
Hmmm.... init_intel() in i386/kernel/setup.c already has the patch for this bug. However, the disassembly looks wrong: c03f74fe: 0f b6 05 23 d0 ff ff movzbl 0xffffd023,%eax c03f7505: c7 04 24 90 7a 2b c0 movl $0xc02b7a90,(%esp,1) c03f750c: 83 e0 0e and $0xe,%eax c03f750f: 89 04 b5 60 a2 45 c0 mov %eax,0xc045a260(,%esi,4) The optimizer is getting a bit frisky and changing hard_smp_processor_id's unsigned long read into a movzbl. Clever, but totally wrong in this case. The local APIC must always be read and written with 32-bit accesses, otherwise the results are undefined. And-ing %eax with 0xe instead of 0xfe isn't right either. Looks like the patches for 8 bit IDs have been reverted somehow in asm-i386/apicdef.h: #define APIC_ID_MASK (0x0F<<24) #define GET_APIC_ID(x) (((x)>>24)&0x0F) The mask values should be 0xFF instead of 0x0F.
Created attachment 105575 [details] 8 bit ID masks and first try at hard_smp_processor_id() This patch works, but as the comment promises, using the volatile storage class makes for really inefficient code.
Created attachment 105576 [details] Better hack to hard_smp_processor_id() and apicdef.h This produces better code from gcc.
Hmmm... The previous patch does work, but gcc loses some variable value info when I insert the barrier(). It doesn't realize that ~(smp_num_siblings - 1) is a constant. That doesn't matter in init_intel() because it only is called at init time, but we use hard_smp_processor_id a fair amount when running. Should this better be classed as a gcc bug, or maybe the kernel is being compiled with a few too many optimizations turned on?
James, I think the gcc behavior is correct (in both its reaction to your use of barrier() and originally in the validity of its optimization into a byte-wide access at +3). I believe the correct implementation of hard_smp_processor_id() is: return GET_APIC_ID(*(volatile unsigned long *)(APIC_BASE+APIC_ID)); But I have not tried building/testing/disassembling this. Cheers. -ernie
It should be noted that I did not see this problem w/ 2.4.21-20. Is there a list of what changed since then?
Ernie, As the comment in hard_smp_processor_id() notes, if you declare the access as volatile, the code quality suffers. Namely, it does a long read into eax, puts the value into a temp local on the stack, loads it back into eax, maybe copies it into another scratch register, then starts to use it. I have no idea why gcc does all the extra steps. 8^) In principle, I agree with you, but gcc has to cooperate.
John, there have been 158 BZs/FZs resolved in 149 patches during U4 development (since 2.4.21-20.EL). The complete list of bugs fixed is in the errata form for RHSA-2004:550, although you're probably not allowed access to that until the final U4 is pushed on RHN. If you want to know the U3->U4 diff of a specific source file, I can generate one and mail it to you. James, I'd hope that all the extra stack references wouldn't be generated simply from adding the "volatile" into the cast as I've done in comment #7. But if they are, then maybe reducing the high- frequency calls to hard_smp_processor_id() is a possibility. Anyway, this is Ingo's bug, so I should stop interfering! :-)
Created attachment 105877 [details] Recent change to setup.c Here is the recent change to setup.c (not present in 2.4.21-20.EL) which when reverted seems to fix this issue. It comes from part of linux-2.4.21-o1-ht.patch I know there was some dispute over this code awhile back, I'm not sure that reverting it is the correct thing to do, but I believe this isolates the relevant new bits.
Yeah, Jun Nakajima and I had some discussions on this in the past. He asserted that the code to deal with non-power-of-2 siblings was vital, and always preferred the cpuinfo APIC ID to the one in the local APIC's ID register. The non-power-of-2 thing I could understand, although I don't recall any such CPU on Intel's roadmap (Tusla will have 4 "agents" per package). I could never figure out his objection to the local APIC ID register. It seemed to have something to do with deranged BIOSes wildly renumbering siblings to very different numbers, thus losing the sibling association. This never seemed to happen in real life. But, don't take my misconceptions and misunderstandings too seriously. His replies are in the archives. Executive summary: as far as I'm concerned, the patch is a very good thing, vital for systems that must renumber APICs on boot (i.e. summit and other NUMA boxen). Too bad hard_smp_processor_id() is weirding out. One of the patches proposed above should fix it.
Any update on this? Is RedHat taking James' patch?
Red Hat, could you please let us know whether this patch was picked up for RHEL3 U4?
This did not make U4. Ingo, could you please follow up on this for U5?
Yes, please could someone look at this? James' fix has been sitting here for two months.
This issue is still present in the 2.4.21-31.ELsmp kernel.
Changing "Product" from Beta to U4
Linux version 2.4.21-37.ELsmp (bhcompile.redhat.com) (gcc version 3.2.3 20030502 (Red Hat Linux 3.2.3-53)) #1 SMP Wed Sep 7 13:28:55 EDT 2005 Still shows this problem.
posted the apicdef.h / smp.h patch for the next taroon update.
A fix for this problem has just been committed to the RHEL3 U7 patch pool this evening (in kernel version 2.4.21-37.3.EL).
An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on the solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHSA-2006-0144.html