On Fri, May 14, 2021 at 4:39 PM Jakub Kicinski <kuba@kernel.org> wrote:
On Fri, 14 May 2021 16:36:16 -0700 Cong Wang wrote:
@@ -176,8 +202,15 @@ 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); + + if (unlikely(test_bit(__QDISC_STATE_MISSED, + &qdisc->state))) { + clear_bit(__QDISC_STATE_MISSED, &qdisc->state);
We have test_and_clear_bit() which is atomic, test_bit()+clear_bit() is not.
It doesn't have to be atomic, right? I asked to split the test because test_and_clear is a locked op on x86, test by itself is not.
It depends on whether you expect the code under the true condition to run once or multiple times, something like: if (test_bit()) { clear_bit(); // this code may run multiple times } With the atomic test_and_clear_bit(), it only runs once: if (test_and_clear_bit()) { // this code runs once } This is why __netif_schedule() uses test_and_set_bit() instead of test_bit()+set_bit(). Thanks.