lkml.org 
[lkml]   [2023]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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.

    \
     
     \ /
      Last update: 2023-03-26 23:36    [W:3.389 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site