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

On Wed 23 May 2018 at 01:10, Cong Wang [off-list ref] wrote:

On Mon, May 21, 2018 at 1:03 PM, Vlad Buslov [off-list ref] wrote:

quoted

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