From: Hannes Frederic Sowa <hannes@stressinduktion.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Eric Dumazet <eric.dumazet@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org,
sasha.levin@oracle.com, daniel@iogearbox.net, mkubecek@suse.cz
Subject: Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get
Date: Fri, 1 Apr 2016 06:12:49 +0200 [thread overview]
Message-ID: <56FDF541.6070106@stressinduktion.org> (raw)
In-Reply-To: <20160401040442.GA14661@ast-mbp.thefacebook.com>
On 01.04.2016 06:04, Alexei Starovoitov wrote:
> On Thu, Mar 31, 2016 at 08:03:38PM -0700, Eric Dumazet wrote:
>> On Thu, 2016-03-31 at 18:45 -0700, Alexei Starovoitov wrote:
>>
>>> Eric, what's your take on Hannes's patch 2 ?
>>> Is it more accurate to ask lockdep to check for actual lock
>>> or lockdep can rely on owned flag?
>>> Potentially there could be races between setting the flag and
>>> actual lock... but that code is contained, so unlikely.
>>> Will we find the real issues with this 'stronger' check or
>>> just spend a ton of time adapting to new model like your other
>>> patch for release_sock and whatever may need to come next...
>>
>> More precise lockdep checks are certainly good, I only objected to 4/4
>> trying to work around another bug.
>>
>> But why do we rush for 'net' tree ?
>>
>> This looks net-next material to me.
>>
>> Locking changes are often subtle, lets take the time to do them
>> properly.
>
> completely agree. I think only first patch belongs in net.
> Everything else is net-next material.
Problem with first patch is that it uses lock_sock_fast, thus the
current sock_owned_by_user check doesn't get rid the lockdep warning. :/
Thus we would need to go with the two first patches. Do you think it is
acceptable? I actually didn't see a problem and testing showed no
problems so far.
Bye,
Hannes
next prev parent reply other threads:[~2016-04-01 4:12 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-31 23:29 [PATCH net 0/4] net: fix and tighten rcu dereference checks Hannes Frederic Sowa
2016-03-31 23:29 ` [PATCH net 1/4] tun: add socket locking around sk_{attach,detach}_filter Hannes Frederic Sowa
2016-03-31 23:29 ` [PATCH net 2/4] net: proper check if we hold the socket lock during dereference Hannes Frederic Sowa
2016-03-31 23:29 ` [PATCH net 3/4] sock: use lockdep_sock_is_held were appropriate Hannes Frederic Sowa
2016-03-31 23:29 ` [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get Hannes Frederic Sowa
2016-03-31 23:39 ` Eric Dumazet
2016-04-01 0:01 ` Hannes Frederic Sowa
2016-04-01 0:12 ` Eric Dumazet
2016-04-01 0:21 ` Hannes Frederic Sowa
2016-04-01 1:19 ` Eric Dumazet
2016-04-01 1:36 ` Hannes Frederic Sowa
2016-04-01 1:39 ` Eric Dumazet
2016-04-01 1:45 ` Eric Dumazet
2016-04-01 2:01 ` Hannes Frederic Sowa
2016-04-01 3:13 ` Eric Dumazet
2016-04-01 3:31 ` Hannes Frederic Sowa
2016-04-01 1:58 ` Hannes Frederic Sowa
2016-04-01 1:45 ` Alexei Starovoitov
2016-04-01 3:03 ` Eric Dumazet
2016-04-01 3:06 ` Hannes Frederic Sowa
2016-04-01 4:04 ` Alexei Starovoitov
2016-04-01 4:12 ` Hannes Frederic Sowa [this message]
2016-04-01 4:26 ` Alexei Starovoitov
2016-04-01 4:33 ` Hannes Frederic Sowa
2016-04-01 8:10 ` Daniel Borkmann
2016-04-01 18:33 ` David Miller
2016-04-01 18:36 ` Daniel Borkmann
2016-04-01 0:30 ` Hannes Frederic Sowa
2016-04-01 1:23 ` Eric Dumazet
2016-04-01 1:37 ` Hannes Frederic Sowa
2016-03-31 23:48 ` [PATCH net 5/4] tcp: fix rcu usage around __sk_dst_get in tcp_update_metrics Hannes Frederic Sowa
2016-03-31 23:56 ` [PATCH net 6/4] tcp: fix __sk_dst_get usage in tcp_current_mss Hannes Frederic Sowa
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=56FDF541.6070106@stressinduktion.org \
--to=hannes@stressinduktion.org \
--cc=alexei.starovoitov@gmail.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=mkubecek@suse.cz \
--cc=netdev@vger.kernel.org \
--cc=sasha.levin@oracle.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.