Thread (5 messages) 5 messages, 3 authors, 2021-06-21

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

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help