Re: [PATCH] net: remove superfluous call to synchronize_net()

[Eric Dumazet <dada1@cosmosbay.com>]

Paul E. McKenney a écrit :
> 
> Please add a comment where the synchronize_rcu() used to be explaining why
> it is not needed.  The poor slob who copies your code isn't going to read
> theh Documentation/RCU, he is just going to expect it to magically work.
> 
> With the synchronize_rcu(), it does just magically work.  Without the
> synchronize_rcu(), you have to be careful.  Therefore, please add the
> comment saying that care is required.
> 

Sorry Paul, I dont understand why I should put a comment to say :

/*
 * Dont need to use synchronize_net() or call_rcu() or msleep(100) or
 * whatever function here because bla bla ...
 */

We could add this comment in about 99% of all functions in linux kernel ;)

I checked inet6_register_protosw(struct inet_protosw *p)
and it doesnt have this synchronize_rcu() neither the comment you advise...

Following construct is obvious and should not be commented in code itself.

spin_lock_bh(&somelock);
list_for_each(..., ...) {
	if (some_condition) {
		list_add_rcu(..., ...)
or		rcu_assign_pointer(...) 
		break;
	}
}
spin_unlock_bh(&somelock);

If it is not obvious, then it should be documented once in Documentation/RCU, since
we find hundred of similar code in kernel.

On the contrary, places where we *use* synchronize_{rcu|net}() should get a comment
to explain why this is really necessary since this function can be a real problem.

Thanks