From: David Miller <davem@davemloft.net>
To: ktkhai@virtuozzo.com
Cc: yoshfuji@linux-ipv6.org, edumazet@google.com,
yanhaishuang@cmss.chinamobile.com, nikolay@cumulusnetworks.com,
yotamg@mellanox.com, soheil@google.com, avagin@virtuozzo.com,
nicolas.dichtel@6wind.com, ebiederm@xmission.com, fw@strlen.de,
roman.kapl@sysgo.com, netdev@vger.kernel.org,
xiyou.wangcong@gmail.com, dvyukov@google.com,
andreyknvl@google.com, lkp@intel.com
Subject: Re: [PATCH net-next v2 2/5] net: Revert "ipv4: fix a deadlock in ip_ra_control"
Date: Thu, 22 Mar 2018 14:41:34 -0400 (EDT) [thread overview]
Message-ID: <20180322.144134.17715767505857944.davem@davemloft.net> (raw)
In-Reply-To: <41aba98d-6e38-0789-f562-4eada70a84b6@virtuozzo.com>
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Tue, 20 Mar 2018 22:25:35 +0300
> On 20.03.2018 19:23, David Miller wrote:
>> From: Kirill Tkhai <ktkhai@virtuozzo.com>
>> Date: Mon, 19 Mar 2018 12:14:54 +0300
>>
>>> This reverts commit 1215e51edad1.
>>> Since raw_close() is used on every RAW socket destruction,
>>> the changes made by 1215e51edad1 scale sadly. This clearly
>>> seen on endless unshare(CLONE_NEWNET) test, and cleanup_net()
>>> kwork spends a lot of time waiting for rtnl_lock() introduced
>>> by this commit.
>>>
>>> Next patches in series will rework this in another way,
>>> so now we revert 1215e51edad1. Also, it doesn't seen
>>> mrtsock_destruct() takes sk_lock, and the comment to the commit
>>> does not show the actual stack dump. So, there is a question
>>> did we really need in it.
>>>
>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>
>> Kirill, I think the commit you are reverting is legitimate.
>>
>> The IP_RAW_CONTROL path has an ABBA deadlock with other paths once
>> you revert this, so you are reintroducing a bug.
>
> The talk is about IP_ROUTER_ALERT, I assume there is just an erratum.
My bad, I did indeed mean IP_ROUTER_ALERT.
>> All code paths that must take both RTNL and the socket lock must
>> do them in the same order. And that order is RTNL then socket
>> lock.
>
> The place I change in this patch is IP_ROUTER_ALERT. There is only
> a call of ip_ra_control(), while this function does not need socket
> lock. Please, see next patch. It moves this ip_ra_control() out
> of socket lock. And it fixes the problem pointed in reverted patch
> in another way. So, if there is ABBA, after next patch it becomes
> solved. Does this mean I have to merge [2/5] and [3/5] together?
Yes, that is what should happen, because the revert by itself
reintroduces the potential ABBA deadlock between the socket lock
and the RTNL mutex.
I'll take a look at the new version of your series.
Thank you.
next prev parent reply other threads:[~2018-03-22 18:41 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-19 9:14 [PATCH net-next v2 0/5] Rework ip_ra_chain protection Kirill Tkhai
2018-03-19 9:14 ` [PATCH net-next v2 1/5] net: Revert "ipv4: get rid of ip_ra_lock" Kirill Tkhai
2018-03-19 9:14 ` [PATCH net-next v2 2/5] net: Revert "ipv4: fix a deadlock in ip_ra_control" Kirill Tkhai
2018-03-20 16:23 ` David Miller
2018-03-20 19:25 ` Kirill Tkhai
2018-03-20 21:50 ` Kirill Tkhai
2018-03-22 18:41 ` David Miller [this message]
2018-03-19 9:15 ` [PATCH net-next v2 3/5] net: Move IP_ROUTER_ALERT out of lock_sock(sk) Kirill Tkhai
2018-03-19 9:15 ` [PATCH net-next v2 4/5] net: Make ip_ra_chain per struct net Kirill Tkhai
2018-03-19 9:15 ` [PATCH net-next v2 5/5] net: Replace ip_ra_lock with per-net mutex Kirill Tkhai
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=20180322.144134.17715767505857944.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=andreyknvl@google.com \
--cc=avagin@virtuozzo.com \
--cc=dvyukov@google.com \
--cc=ebiederm@xmission.com \
--cc=edumazet@google.com \
--cc=fw@strlen.de \
--cc=ktkhai@virtuozzo.com \
--cc=lkp@intel.com \
--cc=netdev@vger.kernel.org \
--cc=nicolas.dichtel@6wind.com \
--cc=nikolay@cumulusnetworks.com \
--cc=roman.kapl@sysgo.com \
--cc=soheil@google.com \
--cc=xiyou.wangcong@gmail.com \
--cc=yanhaishuang@cmss.chinamobile.com \
--cc=yoshfuji@linux-ipv6.org \
--cc=yotamg@mellanox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).