From: Oleg Nesterov <oleg@redhat.com>
To: Jason Baron <jbaron@akamai.com>
Cc: "David S. Miller" <davem@davemloft.net>,
Herbert Xu <herbert.xu@redhat.com>,
Yauheni Kaliuta <yauheni.kaliuta@redhat.com>,
netdev@vger.kernel.org, Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
Date: Mon, 23 Jan 2017 18:18:28 +0100 [thread overview]
Message-ID: <20170123171828.GA2476@redhat.com> (raw)
In-Reply-To: <f5f9c542-2b23-9df9-098c-ad4007bf7e78@akamai.com>
On 01/23, Jason Baron wrote:
>
> >It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
> >and the patch looks correct in that we need the barriers in tcp_check_space()
> >and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
> >
>
> Yes, I think it should be upgraded to an smp_mb() there. If you agree with
> this analysis, I will send a patch to upgrade it.
I think this makes sense, if nothing else to remove the obviously wrong
smp_mb__after_atomic() ;)
> Note, I did not actually
> run into this race in practice.
Yes, I am not sure it is actually necessary in practice, and even if
tcp_check_space() races with tcp_poll() and misses SOCK_NOSPACE it should
be probably called again later, but I know nothing about networking code.
We noticed this by accident, we have a bug report which really looks as a
missed wakeup in epoll, but this kernel is old and in particular it lacks
the commit 128dd1759 "epoll: prevent missed events on EPOLL_CTL_MOD" which
looks more promising. But we are not sure it can fully explain the problem
we can't reproduce, so we are looking for anything else which may contribute
to the problem.
Oleg.
next prev parent reply other threads:[~2017-01-23 17:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-23 14:30 wrong smp_mb__after_atomic() in tcp_check_space() ? Oleg Nesterov
2017-01-23 16:56 ` Jason Baron
2017-01-23 17:18 ` Oleg Nesterov [this message]
2017-01-23 18:04 ` Eric Dumazet
2017-01-23 18:45 ` Jason Baron
2017-01-24 9:18 ` Oleg Nesterov
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=20170123171828.GA2476@redhat.com \
--to=oleg@redhat.com \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=herbert.xu@redhat.com \
--cc=jbaron@akamai.com \
--cc=netdev@vger.kernel.org \
--cc=yauheni.kaliuta@redhat.com \
/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.