From: Alexander Duyck <alexander.h.duyck@redhat•com>
To: Cong Wang <cwang@twopensource•com>
Cc: Thomas Graf <tgraf@suug•ch>, Cong Wang <xiyou.wangcong@gmail•com>,
netdev <netdev@vger•kernel.org>
Subject: Re: [Patch net-next] fib: move fib_rules_cleanup_ops() under rtnl lock
Date: Mon, 30 Mar 2015 20:10:20 -0700 [thread overview]
Message-ID: <551A101C.8080005@redhat.com> (raw)
In-Reply-To: <CAHA+R7MRhvwUcDqAPFRpO8JiEizp_EhUwKyK31MbfMOV=YpW7w@mail.gmail.com>
On 03/30/2015 05:12 PM, Cong Wang wrote:
> On Mon, Mar 30, 2015 at 5:02 PM, Alexander Duyck
> <alexander.h.duyck@redhat•com> wrote:
>> On 03/30/2015 04:47 PM, Cong Wang wrote:
>>> As long as we agree rtnl lock should be taken, you already take my point
>>> here ($subject says so).
>>
>> Yes, I agree lock can be held. For fib4 it was already holding the RTNL
>> lock when it made that call. You can update the other users of
>> fib_rules_unregister so that they call it with the RTNL lock held as well.
>>
>>> It is just API change to move rtnl_lock up to caller or whatever
>>> appropriate.
>>
>> Right, so like I said for fib4 this is resolved. That just leaves ipmr,
>> ip6mr, fib6, and dn_rules that need to be updated so that they correctly
>> handle the RTNL locking in their exit/cleanup paths. Since you already have
>> some related patches out for these I will let you take them otherwise I
>> might try to go through and clean them up next week.
>>
> Ok, then we are finally on the same page. We need two patches:
>
> 1) move unregister under rtnl lock (as what this patch intended to do)
Yes I think the only disagreement was on how to do it. Your original
patch placed it in fib_rules_cleanup_ops, and the preference of myself
and Thomas was to hold the lock before you even call
fib_rules_unregister. So the IPv4 code is fine with the patch I
submitted, it is the other callers of fib_rules_unregister which must be
updated.
> 2) remove the unnecessary rules_mod_lock
>
> Thanks.
Please define "unnecessary" as we have had a bit of back and forth on
how our views can differ there. As far as I know it still has to be
held for the fib_rule_ops list manipulation, specifically the call to
list_del_rcu. However, it doesn't need to be held when we call
fib_rules_cleanup_ops.
- Alex
next prev parent reply other threads:[~2015-03-31 3:10 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-26 21:02 [Patch net-next] fib: move fib_rules_cleanup_ops() under rtnl lock Cong Wang
2015-03-26 21:47 ` Alexander Duyck
2015-03-26 21:55 ` Cong Wang
2015-03-26 22:17 ` Alexander Duyck
2015-03-26 22:32 ` Cong Wang
2015-03-26 23:05 ` Cong Wang
2015-03-26 23:47 ` Alexander Duyck
2015-03-27 12:01 ` Thomas Graf
2015-03-27 19:25 ` Cong Wang
2015-03-27 21:08 ` Alexander Duyck
2015-03-27 21:17 ` Cong Wang
2015-03-27 22:12 ` Alexander Duyck
2015-03-30 23:47 ` Cong Wang
2015-03-31 0:02 ` Alexander Duyck
2015-03-31 0:12 ` Cong Wang
2015-03-31 3:10 ` Alexander Duyck [this message]
2015-03-31 16:47 ` Cong Wang
2015-03-31 17:30 ` Alexander Duyck
2015-03-31 17:56 ` Cong Wang
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=551A101C.8080005@redhat.com \
--to=alexander.h.duyck@redhat$(echo .)com \
--cc=cwang@twopensource$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=tgraf@suug$(echo .)ch \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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