On 26/07/22 12:17, Jason A. Donenfeld wrote:
if (rc <= 0) {
pr_warn("hwrng: no data available\n");msleep_interruptible(10000);
set_current_state(TASK_INTERRUPTIBLE);if (kthread_should_stop())__set_current_state(TASK_RUNNING);schedule_timeout(10 * HZ); continue; }Here you made a change whose utility I don't understand. My original hunk was:
if (kthread_should_stop())break;schedule_timeout_interruptible(HZ * 10);Which I think is a bit cleaner, as schedule_timeout_interruptible sets the state to INTERRUPTIBLE and such.
For any sort of wait loop, you need the state write to happen before the loop-break condition is checked.
Consider:
READ kthread_should_stop() == false kthread_stop() set_bit(KTHREAD_SHOULD_STOP, &kthread->flags); wake_up_process(k); // Reads TASK_RUNNING, schedule_timeout_interruptible(); // does nothing // We're now blocked, having missed a wakeup
That's why the canonical wait loop pattern is:
for (;;) { set_current_state(TASK_UNINTERRUPTIBLE);
if (CONDITION) break;
schedule(); } __set_current_state(TASK_RUNNING);
(grep "wait loop" in kernel/sched/core.c)
Regards, Jason