Messages in this thread | | | From | Cong Wang <> | Date | Mon, 17 May 2021 17:49:25 -0700 | Subject | Re: [PATCH net v8 1/3] net: sched: fix packet stuck problem for lockless qdisc |
| |
On Fri, May 14, 2021 at 7:25 PM Yunsheng Lin <linyunsheng@huawei.com> wrote: > > On 2021/5/15 8:17, Jakub Kicinski wrote: > > On Fri, 14 May 2021 16:57:29 -0700 Cong Wang wrote: > >> 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: > >> [...] > >>>> > >>>> 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 > >> } > > I am not sure if the above really matter when the test and clear > does not need to be atomic. > > In order for the above to happens, the MISSED has to set between > test and clear, right?
Nope, see the following:
// MISSED bit is already set CPU0 CPU1 if (test_bit(MISSED) ( //true if (test_bit(MISSED)) { // also true clear_bit(MISSED); do_something(); clear_bit(MISSED); do_something(); } }
Now do_something() is called twice instead of once. This may or may not be a problem, hence I asked this question.
> > >> > >> This is why __netif_schedule() uses test_and_set_bit() instead of > >> test_bit()+set_bit(). > > I think test_and_set_bit() is needed in __netif_schedule() mainly > because STATE_SCHED is also used to indicate if the qdisc is in > sd->output_queue, so it has to be atomic.
If you replace the "do_something()" above with __netif_reschedule(), this is exactly my point. An entry can not be inserted twice into a list, hence it should never be called twice like above.
Thanks.
| |