From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Brivio Subject: Re: [PATCH net-next 16/17] net: sched: conditionally take rtnl lock on rules update path Date: Tue, 13 Nov 2018 10:40:16 +0100 Message-ID: <20181113104016.76d12436@redhat.com> References: <1542009346-23780-1-git-send-email-vladbu@mellanox.com> <1542009346-23780-17-git-send-email-vladbu@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net To: Vlad Buslov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:48598 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731295AbeKMThk (ORCPT ); Tue, 13 Nov 2018 14:37:40 -0500 In-Reply-To: <1542009346-23780-17-git-send-email-vladbu@mellanox.com> Sender: netdev-owner@vger.kernel.org List-ID: Hi Vlad, On Mon, 12 Nov 2018 09:55:45 +0200 Vlad Buslov wrote: > @@ -179,9 +179,25 @@ static void tcf_proto_destroy_work(struct work_struct *work) > rtnl_unlock(); > } > > +/* Helper function to lock rtnl mutex when specified condition is true and mutex > + * hasn't been locked yet. Will set rtnl_held to 'true' before taking rtnl lock. > + * Note that this function does nothing if rtnl is already held. This is > + * intended to be used by cls API rules update API when multiple conditions > + * could require rtnl lock and its state needs to be tracked to prevent trying > + * to obtain lock multiple times. > + */ > + > +static void tcf_require_rtnl(bool cond, bool *rtnl_held) > +{ > + if (!*rtnl_held && cond) { > + *rtnl_held = true; > + rtnl_lock(); > + } > +} I guess calls to this function are supposed to be serialised. If that's the case (which is my tentative understanding so far), I would indicate that in the comment. If that's not the case, you would be introducing a race I guess. Same applies to tcf_block_release() from 17/17. -- Stefano