On 06/22/2016 02:51 PM, Eric Dumazet wrote:
On Wed, 2016-06-22 at 11:43 -0700, Eric Dumazet wrote:
On Wed, 2016-06-22 at 14:18 -0400, Jason Baron wrote:
For 1/2, the getting the correct memory barrier, should I re-submit
that as a separate patch?
Are you sure a full memory barrier (smp_mb() is needed ?
Maybe smp_wmb() would be enough ?
(And smp_rmb() in tcp_poll() ?)
Well, in tcp_poll() smp_mb__after_atomic() is fine as it follows
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
(although we might add a comment why we should keep
sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk) before the set_bit() !)
But presumably smp_wmb() would be enough in tcp_check_space()
hmm, I think we need the smp_mb() there. From
tcp_poll() we have:
1) set_bit(SOCK_NOSPACE, ...) (write)
2) smp_mb__after_atomic();
3) if (sk_stream_is_writeable(sk)) (read)
while in tcp_check_space() its:
1) the state that sk_stream_is_writeable() cares about (write)
2) smp_mb();
3) if (sk->sk_socket && test_bit(SOCK_NOSPACE,...) (read)
So if we can show that there are sufficient barriers
for #1 (directly above), maybe it can be down-graded or
eliminated. But it would still seem somewhat fragile.
Note I didn't observe any missing wakeups here, but I
just wanted to make sure we didn't miss any, since they
can be quite hard to debug.
Thanks,
-Jason
