From: Stephen Hemminger <[EMAIL PROTECTED]> Date: Sat, 03 Nov 2007 11:43:19 -0700
> The rx_flags variable is redundant. Turning rx on/off is done > via setting the rx_np pointer. > > Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]> Not all of these transformations are equivalent, and as a result you're adding a bug. > @@ -51,12 +50,11 @@ static inline int netpoll_rx(struct sk_b > unsigned long flags; > int ret = 0; > > - if (!npinfo || (!npinfo->rx_np && !npinfo->rx_flags)) > + if (!npinfo || !npinfo->rx_np) > return 0; > > spin_lock_irqsave(&npinfo->rx_lock, flags); > - /* check rx_flags again with the lock held */ > - if (npinfo->rx_flags && __netpoll_rx(skb)) > + if (__netpoll_rx(skb)) > ret = 1; > spin_unlock_irqrestore(&npinfo->rx_lock, flags); If you're using ->rx_np == NULL as your new guard, you have to duplicate that test inside of holding the lock. ->rx_np goes to NULL under the lock, but in the previous code if the rx_flags is set we know that ->rx_np is also NULL, that's why it was OK to only recheck ->rx_flags in the lock and not ->rx_np as well. Therefore we have to replace the test on ->rx_flags with ->rx_np in all spots to retain correct semantics. I've made this correction to your patch and applied it to net-2.6.25 Thanks. - 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
