[Linuxarm] Re: [PATCH net v2] net: sched: fix packet stuck problem for lockless qdisc

On Tue, Mar 23, 2021 at 7:24 PM Yunsheng Lin linyunsheng@huawei.com 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.