Re: netif_poll_enable() & barrier

2006-12-28 Thread Benjamin Herrenschmidt
On Thu, 2006-12-28 at 21:09 -0800, David Miller wrote:
> From: Benjamin Herrenschmidt <[EMAIL PROTECTED]>
> Date: Wed, 20 Dec 2006 14:44:12 +1100
> 
> > I stumbled accross what might be a bug on out of order architecture:
> > 
> > netif_poll_enable() only does a clear_bit(). However,
> > netif_poll_disable/enable pairs are often used as simili-spinlocks.
> > 
> > (netif_poll_enable() has pretty much spin_lock semantics except that it
> > schedules instead of looping).
> > 
> > Thus, shouldn't netif_poll_disable() do an smp_wmb(); before clearing
> > the bit to make sure that any stores done within the poll-disabled
> > section are properly visible to the rest of the system before clearing
> > the bit ?
> 
> Although I couldn't find a problematic case with any current
> in-tree drivers, it's better to be safe than sorry :-)
> 
> So I'll add a smp_mb__before_clear_bit() to netif_poll_enable() :)

Heh, thanks ! :-)

I haven't seen any problematic case neither, though if there was one, it
would result in weird problems very hard to track down, so as you said,
better safe than sorry (unless you see a flaw in my reasoning).

Cheers,

Oh, and happy new year too ! :-)

Ben.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: netif_poll_enable() & barrier

2006-12-28 Thread David Miller
From: Benjamin Herrenschmidt <[EMAIL PROTECTED]>
Date: Wed, 20 Dec 2006 14:44:12 +1100

> I stumbled accross what might be a bug on out of order architecture:
> 
> netif_poll_enable() only does a clear_bit(). However,
> netif_poll_disable/enable pairs are often used as simili-spinlocks.
> 
> (netif_poll_enable() has pretty much spin_lock semantics except that it
> schedules instead of looping).
> 
> Thus, shouldn't netif_poll_disable() do an smp_wmb(); before clearing
> the bit to make sure that any stores done within the poll-disabled
> section are properly visible to the rest of the system before clearing
> the bit ?

Although I couldn't find a problematic case with any current
in-tree drivers, it's better to be safe than sorry :-)

So I'll add a smp_mb__before_clear_bit() to netif_poll_enable() :)
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


netif_poll_enable() & barrier

2006-12-19 Thread Benjamin Herrenschmidt
Hi !

I stumbled accross what might be a bug on out of order architecture:

netif_poll_enable() only does a clear_bit(). However,
netif_poll_disable/enable pairs are often used as simili-spinlocks.

(netif_poll_enable() has pretty much spin_lock semantics except that it
schedules instead of looping).

Thus, shouldn't netif_poll_disable() do an smp_wmb(); before clearing
the bit to make sure that any stores done within the poll-disabled
section are properly visible to the rest of the system before clearing
the bit ?

Cheers,
Ben.


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html