Messages in this thread | | | Date | Thu, 05 Nov 2009 14:19:28 +0100 | From | Eric Dumazet <> | Subject | Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie |
| |
William Allen Simpson a écrit : > Paul E. McKenney wrote: >> On Tue, Nov 03, 2009 at 05:38:10PM -0500, William Allen Simpson wrote: >>> Documentation/RCU/checklist.txt #7 says: >>> >>> One exception to this rule: rcu_read_lock() and rcu_read_unlock() >>> may be substituted for rcu_read_lock_bh() and rcu_read_unlock_bh() >>> in cases where local bottom halves are already known to be >>> disabled, for example, in irq or softirq context. Commenting >>> such cases is a must, of course! And the jury is still out on >>> whether the increased speed is worth it. >> >> I strongly suggest using the matching primitives unless you have a >> really strong reason not to. >> > Eric gave contrary advice. But he also suggested (in an earlier message) > clearing the secrets with a timer, which could be a separate context -- > although much later in time. > > As you suggest, I'll use the _bh suffix everywhere until every i is dotted > and t is crossed. Then, check for efficiency later after thorough > analysis by experts such as yourself. > > This code will be hit on every SYN and SYNACK that has a cookie option. > But it's just prior to a CPU intensive sha_transform -- in comparison, > it's trivial. >
I think you misunderstood my advice ;)
In the same function, you *cannot* use both variants like your last patch did :
spin_lock(&tcp_secret_locker);
...
rcu_read_lock_bh(); memcpy(&xvp->cookie_bakery[0], &rcu_dereference(tcp_secret_generating)->secrets[0], sizeof(tcp_secret_generating->secrets)); rcu_read_unlock_bh();
Reasoning is :
If you need _bh() for the rcu_read_lock_bh(), thats because you know soft irq can happen anytime (they are not masked).
Then you also need _bh for the spin_lock() call, or risk deadlock.
-> tcp_cookie_generator(); spin_lock(); -> interrupt -> softirq -> SYN frame received -> tcp_cookie_generator() -> spin_lock(); hang
Your choices are : ------------------
1) Caller took care of disabling softirqs (or is only called from softirq handler), then _bh suffixes are not necessary in tcp_cookie_generator(). -> spin_lock() & rcu_read_lock();
2) You dont know what called you (process context or softirq context) -> you MUST use _bh prefixes on spin_lock_bh() & rcu_read_lock_bh();
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |