From: Lee R. <rlr...@jo...> - 2004-10-19 22:55:20
|
On Tue, 2004-10-19 at 18:42, David S. Miller wrote: > AIUI the only valid reason to use preempt_disable/enable is in > > the case of per-CPU data. This is not "real" per-CPU data, it's a > > performance hack. Therefore it would be incorrect to add the preemption > > protection, the fix is not to manually call do_softirq but to let the > > softirq run by the normal mechanism. > > > > Am I missing something? > > In code paths where netif_rx_ni() is called, there is not a softirq return > path check, which is why it is checked here. > > Theoretically, if you remove the check, softirq processing can be deferred > indefinitely. > > What I'm saying, therefore, is that netif_rx_ni() it not just a performance > hack, it is necessary for correctness as well. > OK, thanks for clarifying. The correct patch is therefore: --- include/linux/netdevice.h~ 2004-10-19 18:50:18.000000000 -0400 +++ include/linux/netdevice.h 2004-10-19 18:51:01.000000000 -0400 @@ -696,9 +696,11 @@ */ static inline int netif_rx_ni(struct sk_buff *skb) { + preempt_disable(); int err = netif_rx(skb); if (softirq_pending(smp_processor_id())) do_softirq(); + preempt_enable(); return err; } Lee |