From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1757345AbdJKUlG (ORCPT <rfc822;w@1wt.eu>);
        Wed, 11 Oct 2017 16:41:06 -0400
Received: from mail-wm0-f65.google.com ([74.125.82.65]:37544 "EHLO
        mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752230AbdJKUlC (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 11 Oct 2017 16:41:02 -0400
X-Google-Smtp-Source: AOwi7QA4+zLTZ9Fk0v0o/n7xRW40Q+AFtcd8iFcOb1ha3koZItbwL5+BlkjEMAXrUBOK5JcNlLwBbA==
Date: Wed, 11 Oct 2017 22:41:00 +0200
From: Jiri Pirko <jiri@resnulli.us>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Manish Kurup <kurup.manish@gmail.com>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        David Miller <davem@davemloft.net>,
        Linux Kernel Network Developers <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Alexander Aring <aring@mojatatu.com>, Roman Mashak <mrv@mojatatu.com>,
        manish.kurup@verizon.com
Subject: Re: [PATCH net-next 2/2] net sched act_vlan: VLAN action rewrite to
 use RCU lock/unlock and update
Message-ID: <20171011204100.GB9297@nanopsycho>
References: <1507689219-22993-1-git-send-email-manish.kurup@verizon.com>
 <CAM_iQpUYGUzvvM3Weaw5ertgVpwn0p9j6B5xd5Ar5ysJwrdVvg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAM_iQpUYGUzvvM3Weaw5ertgVpwn0p9j6B5xd5Ar5ysJwrdVvg@mail.gmail.com>
User-Agent: Mutt/1.8.3 (2017-05-23)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Wed, Oct 11, 2017 at 06:27:07PM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Oct 10, 2017 at 7:33 PM, Manish Kurup <kurup.manish@gmail.com> wrote:

[...]


>> @@ -187,16 +196,33 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
>>
>>         v = to_vlan(*a);
>>
>> -       spin_lock_bh(&v->tcf_lock);
>> -
>> -       v->tcfv_action = action;
>> -       v->tcfv_push_vid = push_vid;
>> -       v->tcfv_push_prio = push_prio;
>> -       v->tcfv_push_proto = push_proto;
>> +       ASSERT_RTNL();
>> +       p = kzalloc(sizeof(*p), GFP_KERNEL);
>> +       if (unlikely(!p)) {
>> +               if (ovr)
>> +                       tcf_idr_release(*a, bind);
>> +               return -ENOMEM;
>> +       }
>>
>>         v->tcf_action = parm->action;
>>
>> -       spin_unlock_bh(&v->tcf_lock);
>> +       p_old = rtnl_dereference(v->vlan_p);
>> +
>> +       if (ovr)
>> +               spin_lock_bh(&v->tcf_lock);
>
>Why still take spinlock when you already have RTNL lock?
>What's the point?

Yeah, I believe this is copy&paste bug from act_skbmod