From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Duyck Subject: Re: [Patch net-next] fib: move fib_rules_cleanup_ops() under rtnl lock Date: Mon, 30 Mar 2015 20:10:20 -0700 Message-ID: <551A101C.8080005@redhat.com> References: <1427403769-31208-1-git-send-email-xiyou.wangcong@gmail.com> <55147E5D.2070600@redhat.com> <55148576.1010303@redhat.com> <55149A99.6040704@redhat.com> <20150327120135.GC12265@casper.infradead.org> <5515C6C4.4080200@redhat.com> <5515D5E0.2060800@redhat.com> <5519E40F.6090708@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Thomas Graf , Cong Wang , netdev To: Cong Wang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:54093 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753991AbbCaDKV (ORCPT ); Mon, 30 Mar 2015 23:10:21 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On 03/30/2015 05:12 PM, Cong Wang wrote: > On Mon, Mar 30, 2015 at 5:02 PM, Alexander Duyck > 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