From: Jason Baron <jbaron@akamai.com>
To: Eric Dumazet <eric.dumazet@gmail.com>, Jason Baron <jbaron@akamai.com>
Cc: <davem@davemloft.net>, <netdev@vger.kernel.org>
Subject: Re: [PATCH v2 net-next 2/2] tcp: reduce cpu usage when SO_SNDBUF is set
Date: Wed, 22 Jun 2016 15:20:23 -0400 [thread overview]
Message-ID: <576AE4F7.4060204@akamai.com> (raw)
In-Reply-To: <1466621479.6850.75.camel@edumazet-glaptop3.roam.corp.google.com>
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
next prev parent reply other threads:[~2016-06-22 19:20 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-22 15:32 [PATCH v2 net-next 0/2] tcp: reduce cpu usage when SO_SNDBUF is set Jason Baron
2016-06-22 15:32 ` [PATCH v2 net-next 1/2] tcp: replace smp_mb__after_atomic() with smp_mb() in tcp_poll() Jason Baron
2016-06-22 15:32 ` [PATCH v2 net-next 2/2] tcp: reduce cpu usage when SO_SNDBUF is set Jason Baron
2016-06-22 17:34 ` Eric Dumazet
2016-06-22 18:18 ` Jason Baron
2016-06-22 18:43 ` Eric Dumazet
2016-06-22 18:51 ` Eric Dumazet
2016-06-22 19:20 ` Jason Baron [this message]
2016-06-22 20:15 ` Eric Dumazet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=576AE4F7.4060204@akamai.com \
--to=jbaron@akamai.com \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.