Messages in this thread | | | From | Richard Weinberger <> | Date | Tue, 4 Aug 2020 23:56:13 +0200 | Subject | Re: [PATCH] ubi: check kthread_should_stop() after the setting of task state |
| |
On Tue, Aug 4, 2020 at 4:58 AM Zhihao Cheng <chengzhihao1@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
| |