netdev
[Top] [All Lists]

Re: [PATCH] SMP support on 3c527 net driver for 2.6

To: Andrew Morton <akpm@xxxxxxxx>
Subject: Re: [PATCH] SMP support on 3c527 net driver for 2.6
From: Richard Procter <rnp@xxxxxxxxxxxxxxx>
Date: Tue, 21 Oct 2003 17:05:01 +1300 (NZDT)
Cc: jgarzik@xxxxxxxxx, felipewd@xxxxxxxxxxxx, netdev@xxxxxxxxxxx, linux-net@xxxxxxxxxxxxxxx
In-reply-to: <20031018184746.54a2e6b0.akpm@osdl.org>
Sender: netdev-bounce@xxxxxxxxxxx
On Sat, 18 Oct 2003, Andrew Morton wrote:

> I've regenerated this against the current driver.  I'll include it in
> test8-mm1.
>
> If the scsi problem is fixed could you please retest this driver on both
> SMP and UP builds?
 
Thanks --- will do. Note that it's only tested on UP. I'm trying to find
someone with the requisite hardware. 
(SMP + MCA + 3c527 + Linux = a rare beast).   

> If the scsi problem is not fixed could you please be sure to shout at the
> right people about that?

Sure thing. 

> It's a local variable?  Yes, it is atomic.  Partly because the compiler
> will probably align things but mainly because the stack is only used by one
> CPU at a time.

Ah, of course. Thanks. 
 
> If the u16 is a structure member then yes it is probably OK because the
> compiler will pad things.  But if the next slut in the struct is also a u16
> then it may not be atomic on some architectures.  I do not know.  It's best
> to avoid this.

I do share a volatile u16 in the dev struct between hard_start_xmit and
the interrupt handler. The only operations on it are 'set' on one hand,
and 'read' on the other, so the only possible race would be one of reading
an in-progress write. But it's not very obvious that's what's happening, 
looking at it with a fresh eye. 

I've replaced it with an atomic_t type; it makes things more explicit
without hurting performance on x86. I've also included a fix for a small
race pointed out by Manfred Spraul. Patch attached.

best wishes, 
Richard. 






Attachment: 3c527.h-2003.10.20-rnp.patch
Description: Text document

Attachment: 3c527.c-2003.10.20-rnp.patch
Description: Text document

<Prev in Thread] Current Thread [Next in Thread>