Re: [PATCH net-next v3] net: sched: don't disable bh when accessing action idr

[Vlad Buslov <vladbu@mellanox.com>]

On Wed 23 May 2018 at 23:14, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, May 23, 2018 at 1:52 AM, 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
>> in commit 555353cfa1ae ("netdev: The ingress_lock member is no longer
>> needed.").
>>
>> Another reason to disable bh was filters delete code, that released actions
>> in rcu callback. This code was changed to release actions from workqueue
>> context in patch set "net_sched: close the race between call_rcu() and
>> cleanup_net()".
>>
>> With these changes it is no longer necessary to continue disable bh while
>> accessing action map.
>>
>> Replace all action idr spinlock usage with regular calls that do not
>> disable bh.
>
> Looks much better now!
>
> I _guess_ we perhaps can even get rid of this spinlock since most of
> the callers hold RTNL lock, not sure about the dump() path where
> RTNL might be removed recently.

Actually, this change is a result of discussion in code review of my
patch set that removes RTNL dependency from TC rules update path.

>
> Anyway,
>
> Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

Thank you for reviewing my code!