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.