From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [Patch net 8/9] act_ife: move tcfa_lock down to where necessary Date: Mon, 20 Aug 2018 11:29:52 -0700 (PDT) Message-ID: <20180820.112952.2052634913677104782.davem@davemloft.net> References: <20180819192213.14196-1-xiyou.wangcong@gmail.com> <20180819192213.14196-9-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, jhs@mojatatu.com, vladbu@mellanox.com To: xiyou.wangcong@gmail.com Return-path: Received: from shards.monkeyblade.net ([23.128.96.9]:52558 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726089AbeHTVqh (ORCPT ); Mon, 20 Aug 2018 17:46:37 -0400 In-Reply-To: <20180819192213.14196-9-xiyou.wangcong@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: From: Cong Wang Date: Sun, 19 Aug 2018 12:22:12 -0700 > The only time we need to take tcfa_lock is when adding > a new metainfo to an existing ife->metalist. We don't need > to take tcfa_lock so early and so broadly in tcf_ife_init(). > > This means we can always take ife_mod_lock first, avoid the > reverse locking ordering warning as reported by Vlad. > > Reported-by: Vlad Buslov > Tested-by: Vlad Buslov > Cc: Vlad Buslov > Cc: Jamal Hadi Salim > Signed-off-by: Cong Wang After this change we no longer call populate_metalist() in an atomic context via tcf_ife_init(), and populate_metalist passes 'exists' down to add_metainfo() as an 'atomic' indicator. It doesn't have this meaning if you aren't holding the tcfa_lock in the callers with BH disabled. Therefore, add_metainfo()'s 'atomic' indication is inaccurate in this call chain and will use GFP_ATOMIC unnecessarily. Probably the thing to just is just pass 'false' down to add_metainfo() in populate_metalist().