Re: [PATCH net v2] net: sched: add barrier to ensure correct ordering for lockless qdisc
From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-06-21 23:29:54
Also in:
bpf, lkml, netdev
From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-06-21 23:29:54
Also in:
bpf, lkml, netdev
On Sat, 19 Jun 2021 10:30:09 +0000 Yunsheng Lin wrote:
When debugging pointed to the misordering between STATE_MISSED
setting/clearing and STATE_MISSED checking, only _after_atomic()
was added first, and it did not fix the misordering problem,
when both _before_atomic() and _after_atomic() were added, the
misordering problem disappeared.
I suppose _before_atomic() matters because the STATE_MISSED
setting and the lock rechecking is only done when first check of
STATE_MISSED returns false. _before_atomic() is used to make sure
the first check returns correct result, if it does not return the
correct result, then we may have misordering problem too.
cpu0 cpu1
clear MISSED
_after_atomic()
dequeue
enqueue
first trylock() #false
MISSED check #*true* ?
As above, even cpu1 has a _after_atomic() between clearing
STATE_MISSED and dequeuing, we might stiil need a barrier to
prevent cpu0 doing speculative MISSED checking before cpu1
clearing MISSED?
And the implicit load-acquire barrier contained in the first
trylock() does not seems to prevent the above case too.
And there is no load-acquire barrier in pfifo_fast_dequeue()
too, which possibly make the above case more likely to happen.Ah, you're right. The test_bit() was not in the patch context, I forgot it's there... Both barriers are indeed needed.