From mboxrd@z Thu Jan  1 00:00:00 1970
From: Vlad Buslov <vladbu@mellanox.com>
Subject: Re: [PATCH net-next v2] net: sched: don't disable bh when accessing action idr
Date: Wed, 23 May 2018 09:57:34 +0300
Message-ID: <vbfy3gaj1up.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
References: <20180519.230258.1374885458106197707.davem@davemloft.net> <1526932984-11544-1-git-send-email-vladbu@mellanox.com> <CAM_iQpVMFSxnN6dEi0U4=aw7s7TcJ88ShiNTDXcoRUVL82jTZw@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain
Cc: David Miller <davem@davemloft.net>,
        Linux Kernel Network Developers <netdev@vger.kernel.org>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Jiri Pirko <jiri@resnulli.us>,
        LKML <linux-kernel@vger.kernel.org>
To: Cong Wang <xiyou.wangcong@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-eopbgr40061.outbound.protection.outlook.com ([40.107.4.61]:61440
        "EHLO EUR03-DB5-obe.outbound.protection.outlook.com"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1754005AbeEWHAB (ORCPT <rfc822;netdev@vger.kernel.org>);
        Wed, 23 May 2018 03:00:01 -0400
In-reply-to: <CAM_iQpVMFSxnN6dEi0U4=aw7s7TcJ88ShiNTDXcoRUVL82jTZw@mail.gmail.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>


On Wed 23 May 2018 at 01:10, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, May 21, 2018 at 1:03 PM, Vlad Buslov <vladbu@mellanox.com> wrote:
>> Initial net_device implementation used ingress_lock spinlock to synchronize
>> ingress path of device. This lock was used in both process and bh context.
>> In some code paths action map lock was obtained while holding ingress_lock.
>> Commit e1e992e52faa ("[NET_SCHED] protect action config/dump from irqs")
>> modified actions to always disable bh, while using action map lock, in
>> order to prevent deadlock on ingress_lock in softirq. This lock was removed
>> from net_device, so disabling bh, while accessing action map, is no longer
>> necessary.
>>
>> Replace all action idr spinlock usage with regular calls that do not
>> disable bh.
>
> While your patch is probably fine, the above justification seems not.

Sorry if I missed something. My justification is based on commit
description that added bh disable in subject code.

>
> In the past, tc actions could be released in BH context because tc
> filters use call_rcu(). However, I moved them to a workqueue recently.
> So before my change I don't think you can remove the BH protection,
> otherwise race with idr_remove()...

Found commit series that you described. Will modify commit message
accordingly.

Thanks,
Vlad