From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [Patch net-next] act_mirred: get rid of mirred_list_lock spinlock Date: Thu, 30 Nov 2017 15:12:04 -0800 Message-ID: <1512083524.19682.29.camel@gmail.com> References: <20171130225335.6957-1-xiyou.wangcong@gmail.com> <20171130225335.6957-2-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Cc: Jiri Pirko , Jamal Hadi Salim To: Cong Wang , netdev@vger.kernel.org Return-path: Received: from mail-io0-f195.google.com ([209.85.223.195]:46624 "EHLO mail-io0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751077AbdK3XMH (ORCPT ); Thu, 30 Nov 2017 18:12:07 -0500 Received: by mail-io0-f195.google.com with SMTP id x129so9324667iod.13 for ; Thu, 30 Nov 2017 15:12:07 -0800 (PST) In-Reply-To: <20171130225335.6957-2-xiyou.wangcong@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 2017-11-30 at 14:53 -0800, Cong Wang wrote: > TC actions are no longer freed in RCU callbacks and we should > always have RTNL lock, so this spinlock is no longer needed. > > Cc: Jiri Pirko > Cc: Jamal Hadi Salim > Signed-off-by: Cong Wang > --- >  net/sched/act_mirred.c | 8 -------- >  1 file changed, 8 deletions(-) > > diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c > index fe6489f9c3cf..2c51952bf2d4 100644 > --- a/net/sched/act_mirred.c > +++ b/net/sched/act_mirred.c > @@ -29,7 +29,6 @@ >  #include >   >  static LIST_HEAD(mirred_list); > -static DEFINE_SPINLOCK(mirred_list_lock); >   >  static bool tcf_mirred_is_act_redirect(int action) >  { > @@ -55,13 +54,10 @@ static void tcf_mirred_release(struct tc_action > *a, int bind) >   struct tcf_mirred *m = to_mirred(a); >   struct net_device *dev; >   > - /* We could be called either in a RCU callback or with RTNL > lock held. */ > - spin_lock_bh(&mirred_list_lock); >   list_del(&m->tcfm_list); >   dev = rcu_dereference_protected(m->tcfm_dev, 1); If RTNL is held at this point, I suggest to use rtnl_dereference() instead of rcu_dereference_protected() to get proper lockdep coverage. >   if (dev) >   dev_put(dev); > - spin_unlock_bh(&mirred_list_lock); >  } >