On Tue, Jul 7, 2020 at 8:25 AM Petr Machata [off-list ref] wrote:

Cong Wang [off-list ref] writes:

quoted

On Fri, Jun 26, 2020 at 3:46 PM Petr Machata [off-list ref] wrote:

quoted

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.