On Tue, Mar 23, 2021 at 7:24 PM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:
> @@ -176,8 +207,23 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc)
> static inline void qdisc_run_end(struct Qdisc *qdisc)
> {
> write_seqcount_end(&qdisc->running);
> - if (qdisc->flags & TCQ_F_NOLOCK)
> + if (qdisc->flags & TCQ_F_NOLOCK) {
> spin_unlock(&qdisc->seqlock);
> +
> + /* qdisc_run_end() is protected by RCU lock, and
> + * qdisc reset will do a synchronize_net() after
> + * setting __QDISC_STATE_DEACTIVATED, so testing
> + * the below two bits separately should be fine.
Hmm, why synchronize_net() after setting this bit is fine? It could
still be flipped right after you test RESCHEDULE bit.
> + * For qdisc_run() in net_tx_action() case, we
> + * really should provide rcu protection explicitly
> + * for document purposes or PREEMPT_RCU.
> + */
> + if (unlikely(test_bit(__QDISC_STATE_NEED_RESCHEDULE,
> + &qdisc->state) &&
> + !test_bit(__QDISC_STATE_DEACTIVATED,
> + &qdisc->state)))
Why do you want to test __QDISC_STATE_DEACTIVATED bit at all?
dev_deactivate_many() will wait for those scheduled but being
deactivated, so what's the problem of scheduling it even with this bit?
Thanks.
