Messages in this thread | ![/](/images/icornerl.gif) | | From | Wei Wang <> | Date | Wed, 11 Jan 2023 11:39:46 -0800 | Subject | Re: [PATCH v1] net/dev.c : Remove redundant state settings after waking up |
| |
On Wed, Jan 11, 2023 at 10:21 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 11 Jan 2023 08:32:42 +0100 Sebastian Andrzej Siewior wrote: > > It made sense in the beginning but now the suggested patch is a clean > > up. First the `woken' parameter was added in commit > > cb038357937ee ("net: fix race between napi kthread mode and busy poll") > > > > and then the `napi_disable_pending' check was removed in commit > > 27f0ad71699de ("net: fix hangup on napi_disable for threaded napi") > > > > which renders the code to: > > | while (!kthread_should_stop()) { > > | if (test_bit(NAPI_STATE_SCHED_THREADED, &napi->state) || woken) { > > | WARN_ON(!list_empty(&napi->poll_list)); > > | __set_current_state(TASK_RUNNING); > > | return 0; > > | } > > | > > | schedule(); > > | /* woken being true indicates this thread owns this napi. */ > > | woken = true; > > | set_current_state(TASK_INTERRUPTIBLE); > > | } > > | __set_current_state(TASK_RUNNING); > > > > so when you get out of schedule() woken is set and even if > > NAPI_STATE_SCHED_THREADED is not set, the while() loop is left due to > > `woken = true'. So changing state to TASK_INTERRUPTIBLE makes no sense > > since it will be set back to TASK_RUNNING cycles later. > > Ah, fair point, forgot about the woken optimization.
Agree. I think it is OK to remove this, since woken is set, and this function will set TASK_RUNNING and return 0. And the next time, it will enter with TASK_INTERRUPTIBLE and woken = false.
| ![\](/images/icornerr.gif) |