On Tue, Aug 4, 2020 at 4:58 AM Zhihao Cheng <chengzhih...@huawei.com> wrote: > Oh, you're thinking about influence by schedule(), I get it. But I think > it still works. Because the ubi_thread is still on runqueue, it will be > scheduled to execute later anyway.
It will not get woken. This is the problem. > > op state of > ubi_thread on runqueue > set_current_state(TASK_INTERRUPTIBLE) TASK_INTERRUPTIBLE Yes > if (kthread_should_stop()) // not satisfy > TASK_INTERRUPTIBLE Yes > kthread_stop: > wake_up_process > ttwu_queue > ttwu_do_activate > ttwu_do_wakeup TASK_RUNNING Yes > schedule > __schedule(false) > > // prev->state is TASK_RUNNING, so we cannot move it from runqueue by > deactivate_task(). So just pick next task to execute, ubi_thread is > still on runqueue and will be scheduled to execute later. It will be in state TASK_RUNNING only if your check is reached. If kthread_stop() is called *before* your code: + if (kthread_should_stop()) { + set_current_state(TASK_RUNNING); + break; + } ...everything is fine. But there is still a race window between your if (kthread_should_stop()) and schedule() in the next line. So if kthread_stop() is called right *after* the if and *before* schedule(), the task state is still TASK_INTERRUPTIBLE --> schedule() will not return unless the task is explicitly woken, which does not happen. Before your patch, the race window was much larger, I fully agree, but your patch does not cure the problem it just makes it harder to hit. And using mdelay() to verify such a thing is also tricky because mdelay() will influence the task state. -- Thanks, //richard