Re: netif_poll_enable() & barrier
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
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
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