Re: [PATCH][2.5] smp_call_function needs mb() - oopsable

Re: [PATCH][2.5] smp_call_function needs mb() - oopsable

[Chuck Ebbert]

>  <0>Kernel panic: Aiee, killing interrupt handler!


 Where is that extra space before the '<0>' coming from???

--
Chuck

Re: [PATCH][2.5] smp_call_function needs mb() - oopsable

[Zwane Mwaikambo]

On Thu, 3 Apr 2003, Zwane Mwaikambo wrote:

> On Thu, 3 Apr 2003, Zwane Mwaikambo wrote:
> 
> > I'm compiling with rmb before the APIC EOI, which is after the local 
> > variable assignments (i'll post the results in a bit, slow build box).

And here is one with the following changes, everything else is left as is. 

asmlinkage void smp_call_function_interrupt(void)
{
	void (*func) (void *info);
	void *info;
	int wait;
	ack_APIC_irq();

	rmb();
	func = call_data->func;
	info = call_data->info;
	wait = call_data->wait;
...

0xc0115b10 <smp_call_function_interrupt>:       push   %esi
0xc0115b11 <smp_call_function_interrupt+1>:     xor    %eax,%eax
0xc0115b13 <smp_call_function_interrupt+3>:     push   %ebx
0xc0115b14 <smp_call_function_interrupt+4>:     xchg   %eax,0xffffd0b0
0xc0115b1a <smp_call_function_interrupt+10>:    lock addl $0x0,0x0(%esp,1)
0xc0115b20 <smp_call_function_interrupt+16>:    mov    0xc05b6620,%eax
0xc0115b25 <smp_call_function_interrupt+21>:    mov    (%eax),%ecx
0xc0115b27 <smp_call_function_interrupt+23>:    mov    0x4(%eax),%edx
0xc0115b2a <smp_call_function_interrupt+26>:    mov    0x10(%eax),%esi
0xc0115b2d <smp_call_function_interrupt+29>:    lock addl $0x0,0x0(%esp,1)
0xc0115b33 <smp_call_function_interrupt+35>:    mov    0xc05b6620,%eax
0xc0115b38 <smp_call_function_interrupt+40>:    lock incl 0x8(%eax)
0xc0115b3c <smp_call_function_interrupt+44>:    mov    $0xffffe000,%ebx
0xc0115b41 <smp_call_function_interrupt+49>:    and    %esp,%ebx
0xc0115b43 <smp_call_function_interrupt+51>:    mov    0x14(%ebx),%eax
0xc0115b46 <smp_call_function_interrupt+54>:    add    $0x10000,%eax
0xc0115b4b <smp_call_function_interrupt+59>:    mov    %eax,0x14(%ebx)
0xc0115b4e <smp_call_function_interrupt+62>:    push   %edx
0xc0115b4f <smp_call_function_interrupt+63>:    call   *%ecx


Unable to handle kernel NULL pointer dereference at virtual address 00000014
 printing eip:
c033d1dc
*pde = 00000000
Oops: 0002 [#1]
CPU:    2
EIP:    0060:[<c033d1dc>]    Not tainted
EFLAGS: 00010006
EIP is at sr_do_ioctl+0x12c/0x250
eax: 00000000   ebx: cbf94000   ecx: c033d1d4   edx: cbf94000
esi: 00000000   edi: cbf94000   ebp: 00000000   esp: cbf95f6c
ds: 007b   es: 007b   ss: 0068
Process swapper (pid: 0, threadinfo=cbf94000 task=c151cc80)
Stack: c0115b51 cbe5e7d0 cbf94000 c0106ea0 c010a24a cbf94000 00000000 cbf94000 
       c0106ea0 cbf94000 00000000 00000000 0000007b 0000007b fffffffb c0106ece 
       00000060 00000246 c0106f5a 00000000 000075bc 00000000 0000000d c01217b7 
Call Trace:
 [<c0115b51>] smp_call_function_interrupt+0x41/0x87
 [<c0106ea0>] default_idle+0x0/0x40
 [<c010a24a>] call_function_interrupt+0x1a/0x20
 [<c0106ea0>] default_idle+0x0/0x40
 [<c0106ece>] default_idle+0x2e/0x40
 [<c0106f5a>] cpu_idle+0x3a/0x50
 [<c01217b7>] printk+0x1b7/0x230

Code: 89 50 14 8b 44 24 08 83 c4 10 5b 5e 5f 5d c3 90 8d 74 26 00 
 <0>Kernel panic: Aiee, killing interrupt handler!
In interrupt handler - not syncing

(gdb) list *sr_do_ioctl+0x12c
0xc033d1dc is in sr_do_ioctl (drivers/scsi/sr_ioctl.c:172).
167             /* Wake up a process waiting for device */
168           out_free:
169             scsi_release_request(SRpnt);
170             SRpnt = NULL;
171           out:
172             cgc->stat = err;
173             return err;
174     }
175

(gdb) list *smp_call_function_interrupt+0x40
0xc0115b50 is in smp_call_function_interrupt (arch/i386/kernel/smp.c:599).
594             atomic_inc(&call_data->started);
595             /*
596              * At this point the info structure may be out of scope unless wait==1
597              */
598             irq_enter();
599             (*func)(info);
600             irq_exit();
601
602             if (wait) {
603                     mb();

Code;  00000000 Before first symbol
00000000 <_EIP>:
Code;  00000000 Before first symbol
   0:   89 50 14                  mov    %edx,0x14(%eax)
Code;  00000003 Before first symbol
   3:   8b 44 24 08               mov    0x8(%esp,1),%eax
Code;  00000007 Before first symbol
   7:   83 c4 10                  add    $0x10,%esp
Code;  0000000a Before first symbol
   a:   5b                        pop    %ebx
Code;  0000000b Before first symbol
   b:   5e                        pop    %esi
Code;  0000000c Before first symbol
   c:   5f                        pop    %edi
Code;  0000000d Before first symbol
   d:   5d                        pop    %ebp
Code;  0000000e Before first symbol
   e:   c3                        ret
Code;  0000000f Before first symbol
   f:   90                        nop
Code;  00000010 Before first symbol
  10:   8d 74 26 00               lea    0x0(%esi,1),%esi

0xc033d1dc <sr_do_ioctl+300>:   mov    %edx,0x14(%eax)
0xc033d1df <sr_do_ioctl+303>:   mov    0x8(%esp,1),%eax
0xc033d1e3 <sr_do_ioctl+307>:   add    $0x10,%esp
0xc033d1e6 <sr_do_ioctl+310>:   pop    %ebx
0xc033d1e7 <sr_do_ioctl+311>:   pop    %esi
0xc033d1e8 <sr_do_ioctl+312>:   pop    %edi
0xc033d1e9 <sr_do_ioctl+313>:   pop    %ebp

-- 
function.linuxpower.ca

Re: [PATCH][2.5] smp_call_function needs mb() - oopsable

[Zwane Mwaikambo]

On Thu, 3 Apr 2003, Zwane Mwaikambo wrote:

> I'm compiling with rmb before the APIC EOI, which is after the local 
> variable assignments (i'll post the results in a bit, slow build box).

I just got this which can only be as a result of the changes (the kernel 
has otherwise passed this test case over 10 times over a period of 2 
days). It could be due to me hitting another interrupt whilst in that 
handler (we have interrupts enabled in smp_call_function_interrupt) but 
i'm trying to think of other ways we could otherwise trigger a GPF.

general protection fault: 0000 [#1]
CPU:    1
EIP:    0060:[<08410005>]    Not tainted
EFLAGS: 00210002
EIP is at 0x8410005
eax: 40a2d700   ebx: c010a24a   ecx: c033d1d4   edx: 40a2d760
esi: 40a2d760   edi: 00daf9f8   ebp: 000000c6   esp: c39f9fd4
ds: 007b   es: 007b   ss: 0068
Process rhn-applet (pid: 1507, threadinfo=c39f8000 task=c8316040)
Stack: c4e94f88 bfffec78 000000d6 0000007b 0000007b fffffffb 40a23ef9 
00000073 
       00200216 bfffec48 0000007b 
Call Trace:

Code:  Bad EIP value.
 <0>Kernel panic: Aiee, killing interrupt handler!
In interrupt handler - not syncing

-- 
function.linuxpower.ca

Re: [PATCH][2.5] smp_call_function needs mb() - oopsable

[Zwane Mwaikambo]

On Thu, 3 Apr 2003, Linus Torvalds wrote:

> On Thu, 3 Apr 2003, Zwane Mwaikambo wrote:
> >
> > I have a 3 Processor Pentium 133 system w/ 512k external cache which is 
> > oopsing reliably in the exact same location.
> 
> Whee. What a piece of "interesting hardware".

Yep, it appears to have PCI bus funnies too..

> I really think that your patch is a bit questionable. The wmb() on the 
> sender side looks correct as-is, and to me it looks like it is the 
> _receiver_ side that might need a read-barrier before it reads 
> call_data(). 

I'm compiling with rmb before the APIC EOI, which is after the local 
variable assignments (i'll post the results in a bit, slow build box).

> I really thought that the interrupt should be a serializing event, but I 
> can't find that in the intel databooks (they make "iret" a serializing 
> instruction, but not _taking_ an interrupt, unless I missed something).
> 
> Can you check if you get the right behaviour if you have a read barrier in 
> the receive path? I actually think we need a full mb() on _both_ paths, 
> since the current wmb() only guarantees that writes will be seen "in 
> order wrt other writes", and while the IPI generation really _is_ a write 
> in itself, I wonder if the Intel CPU's might not consider it something 
> special..

I haven't actually seen anything mentioning writing to ICR 
having a serializing side effect. However the forced read around write 
with family < P6 (CONFIG_X86_GOOD_APIC) in Linux should ensure that no 
reads or writes pass the APIC/ICR write due to the xchg, however wether 
'implied' lock ensures that it's treated the same as implicit lock i don't 
know.

> I'm not opposed to your patch per se, but I really do believe that it is 
> potentially wrong. If we have no serialization on the read side, your 
> patch might not actually fully plug the real bug, only hide it. I'd like 
> to know if a read barrier on the read side (without the full barrier on 
> the write side) is sufficient. It _should_ be (but see my worry about the 
> APIC write maybe being considered "outside the scope" of the normal cache 
> coherency protocols).

Wouldn't APIC writes be then treated differently on a P4 (which uses the 
system bus) and the P5/P6 which has it's own serial bus? Imo we should not 
rely on APIC loads/stores which is why i added an rmb after the APIC EOI 
for the rmb in smp_call_function_interrupt test.

	Zwane
-- 
function.linuxpower.ca

Re: [PATCH][2.5] smp_call_function needs mb() - oopsable

[Linus Torvalds]

On Thu, 3 Apr 2003, Zwane Mwaikambo wrote:
>
> I have a 3 Processor Pentium 133 system w/ 512k external cache which is 
> oopsing reliably in the exact same location.

Whee. What a piece of "interesting hardware".

>					 The problem is that the 
> current memory barrier in smp_call_function is a simple gcc optimisation 
> barrier, we really need a memory barrier there so that the other cpu's 
> get the updated value when they get IPI'd immediately afterwards. 

I really think that your patch is a bit questionable. The wmb() on the 
sender side looks correct as-is, and to me it looks like it is the 
_receiver_ side that might need a read-barrier before it reads 
call_data(). 

I really thought that the interrupt should be a serializing event, but I 
can't find that in the intel databooks (they make "iret" a serializing 
instruction, but not _taking_ an interrupt, unless I missed something).

Can you check if you get the right behaviour if you have a read barrier in 
the receive path? I actually think we need a full mb() on _both_ paths, 
since the current wmb() only guarantees that writes will be seen "in 
order wrt other writes", and while the IPI generation really _is_ a write 
in itself, I wonder if the Intel CPU's might not consider it something 
special..

I'm not opposed to your patch per se, but I really do believe that it is 
potentially wrong. If we have no serialization on the read side, your 
patch might not actually fully plug the real bug, only hide it. I'd like 
to know if a read barrier on the read side (without the full barrier on 
the write side) is sufficient. It _should_ be (but see my worry about the 
APIC write maybe being considered "outside the scope" of the normal cache 
coherency protocols).

			Linus

[PATCH][2.5] smp_call_function needs mb() - oopsable

[Zwane Mwaikambo]

I have a 3 Processor Pentium 133 system w/ 512k external cache which is 
oopsing reliably in the exact same location. The problem is that the 
current memory barrier in smp_call_function is a simple gcc optimisation 
barrier, we really need a memory barrier there so that the other cpu's 
get the updated value when they get IPI'd immediately afterwards. The 
sequence looks like this;

cpu0					cpu1	
smp_call_function() {			...
	...				...
	call_data = &data		...
	send_IPI()			smp_call_function_interrupt() {
						func = call_data->func <== [1]

[1] at this point cpu1 loads memory, which hasn't been written back yet, 
to a local register.

0xc0114e30 <smp_call_function_interrupt>:       push   %esi
0xc0114e31 <smp_call_function_interrupt+1>:     mov    0xc02e9830,%eax
0xc0114e36 <smp_call_function_interrupt+6>:     push   %ebx
0xc0114e37 <smp_call_function_interrupt+7>:     mov    (%eax),%ecx
0xc0114e39 <smp_call_function_interrupt+9>:     mov    0x4(%eax),%edx
0xc0114e3c <smp_call_function_interrupt+12>:    mov    0x10(%eax),%esi
0xc0114e3f <smp_call_function_interrupt+15>:    xor    %eax,%eax
0xc0114e41 <smp_call_function_interrupt+17>:    xchg   %eax,0xffffd0b0
0xc0114e47 <smp_call_function_interrupt+23>:    lock addl $0x0,0x0(%esp,1)
0xc0114e4d <smp_call_function_interrupt+29>:    mov    0xc02e9830,%eax
0xc0114e52 <smp_call_function_interrupt+34>:    lock incl 0x8(%eax)
0xc0114e56 <smp_call_function_interrupt+38>:    mov    $0xffffe000,%ebx
0xc0114e5b <smp_call_function_interrupt+43>:    and    %esp,%ebx
0xc0114e5d <smp_call_function_interrupt+45>:    mov    0x14(%ebx),%eax
0xc0114e60 <smp_call_function_interrupt+48>:    add    $0x10000,%eax
0xc0114e65 <smp_call_function_interrupt+53>:    mov    %eax,0x14(%ebx)
0xc0114e68 <smp_call_function_interrupt+56>:    push   %edx
0xc0114e69 <smp_call_function_interrupt+57>:    call   *%ecx

The only reason i appear to be seeing this problem is probably due to 
cache controller hardware which is present in the box (old 'highend' MP 
server). I have been unable to reproduce this on much larger SMP boxes 
(saner cache coherency hardware).

processor       : 2
vendor_id       : GenuineIntel
cpu family      : 5
model           : 2
model name      : Pentium 75 - 200
stepping        : 11
cpu MHz         : 133.314
fdiv_bug        : no
hlt_bug         : no
f00f_bug        : yes
coma_bug        : no
fpu             : yes
fpu_exception   : yes
cpuid level     : 1
wp              : yes
flags           : fpu vme de pse tsc msr mce cx8 apic
bogomips        : 266.24

Patch against 2.5.66 appended.

Unable to handle kernel NULL pointer dereference at virtual address 00000200
 printing eip:
00000200
*pde = 00000000
Oops: 0000 [#1]
CPU:    2
EIP:    0060:[<00000200>]    Not tainted
EFLAGS: 00210082
EIP is at 0x200
eax: 00000026   ebx: c7666000   ecx: c7666000   edx: c7666000
esi: 00000200   edi: c034ad74   ebp: c0bc8000   esp: c7667fa8
ds: 007b   es: 007b   ss: 0068
Process rhn-applet (pid: 1654, threadinfo=c7666000 task=c0785340)
Stack: c0116407 c034ad74 40a2d760 08458490 00000004 bfffec78 c010a41a 40a2d760 
       00000000 00000000 08458490 00000004 bfffec78 0830d800 0000007b 0000007b 
       fffffffb 40a23da4 00000073 00200202 bfffec48 0000007b 
Call Trace:
 [<c0116407>] smp_call_function_interrupt+0x57/0xb0
 [<c034ad74>] sr_do_ioctl+0x124/0x250
 [<c010a41a>] call_function_interrupt+0x1a/0x20

Code:  Bad EIP value.
 <0>Kernel panic: Aiee, killing interrupt handler!

Index: linux-2.5.66/arch/i386/kernel/smp.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.66/arch/i386/kernel/smp.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 smp.c
--- linux-2.5.66/arch/i386/kernel/smp.c	24 Mar 2003 23:40:27 -0000	1.1.1.1
+++ linux-2.5.66/arch/i386/kernel/smp.c	28 Mar 2003 05:08:54 -0000
@@ -522,7 +521,8 @@ int smp_call_function (void (*func) (voi
 
 	spin_lock(&call_lock);
 	call_data = &data;
-	wmb();
+	mb();
+	
 	/* Send a message to all other CPUs and wait for them to respond */
 	send_IPI_allbutself(CALL_FUNCTION_VECTOR);