On Tue, Jul 7, 2020 at 8:25 AM Petr Machata <pe...@mellanox.com> wrote: > > > Cong Wang <xiyou.wangc...@gmail.com> writes: > > > On Fri, Jun 26, 2020 at 3:46 PM Petr Machata <pe...@mellanox.com> wrote: > >> > >> A following patch introduces qevents, points in qdisc algorithm where > >> packet can be processed by user-defined filters. Should this processing > >> lead to a situation where a new packet is to be enqueued on the same port, > >> holding the root lock would lead to deadlocks. To solve the issue, qevent > >> handler needs to unlock and relock the root lock when necessary. > >> > >> To that end, add the root lock argument to the qdisc op enqueue, and > >> propagate throughout. > > > > Hmm, but why do you pass root lock down to each ->enqueue()? > > > > You can find root lock with sch_tree_lock() (or qdisc_lock() if you don't > > care about hierarchy), and you already have qdisc as a parameter of > > tcf_qevent_handle(). > > I know, I wanted to make it clear that the lock may end up being used, > instead of doing it "stealthily". If you find this inelegant I can push > a follow-up that converts tcf_qevent_handle() to sch_tree_unlock().
So far only sch_red uses tcf_qevent_handle(), changing the rest qdisc's just for sch_red seems overkill here. Of course, if we could eliminate the root lock release, all the pains would go away. Thanks.
