Message ID | 49E5FF5E.50409@cosmosbay.com |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
On Wed, Apr 15, 2009 at 05:38:06PM +0200, Eric Dumazet wrote: > inet_register_protosw() is adding inet_protosw to inetsw[] with appropriate > locking section and rcu variant. No need to call synchronize_net() to wait > for a RCU grace period. Changes are immediatly visible to other cpus anyway. I agree with the conclusion (that this change is safe), but not with the reasoning process. ;-) The reason that this change is safe is that any inter-process communication mechanism used to tell other CPUs that this protocol has been registered must contain relevant memory barriers, otherwise, that mechanism won't be reliable. If an unreliable mechanism was to be used, the other CPU might not yet see the protocol. For example, if the caller did a simple non-atomic store to a variable that the other CPU accessed with a simple non-atomic load, then that other CPU could potentially see the inetsw[] without the new protocol, given that inet_create() is lockless. Unlikely, but possible. But if a proper inter-process communication mechanism is used to inform the other CPU, then the first CPU's memory operations will be seen. So I suggest a comment to this effect. > This saves about 13 ms on boot time on a HZ=1000 8 cpus machine ;) > (4 calls to inet_register_protosw(), and about 3200 us per call) > > But more seriously, we should audit all synchronize_{rcu|net}() calls > to make sure we dont waste time and hide some bugs because of artificial > delays. Good point! Thanx, Paul > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> > > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c > index 7f03373..1706896 100644 > --- a/net/ipv4/af_inet.c > +++ b/net/ipv4/af_inet.c > @@ -1003,8 +1003,6 @@ void inet_register_protosw(struct inet_protosw *p) > out: > spin_unlock_bh(&inetsw_lock); > > - synchronize_net(); > - > return; > > out_permanent: -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 7f03373..1706896 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1003,8 +1003,6 @@ void inet_register_protosw(struct inet_protosw *p) out: spin_unlock_bh(&inetsw_lock); - synchronize_net(); - return; out_permanent:
inet_register_protosw() is adding inet_protosw to inetsw[] with appropriate locking section and rcu variant. No need to call synchronize_net() to wait for a RCU grace period. Changes are immediatly visible to other cpus anyway. This saves about 13 ms on boot time on a HZ=1000 8 cpus machine ;) (4 calls to inet_register_protosw(), and about 3200 us per call) But more seriously, we should audit all synchronize_{rcu|net}() calls to make sure we dont waste time and hide some bugs because of artificial delays. Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html