Re: [PATCH] kthread: Fix kthread_mod_delayed_work vs kthread_cancel_delayed_work_sync race

[Petr Mladek]

On Thu 2021-05-13 14:54:57, Martin Liu wrote:
> We encountered a system hang issue while doing the tests. The callstack
> is as following
> 
> 	schedule+0x80/0x100
> 	schedule_timeout+0x48/0x138
> 	wait_for_common+0xa4/0x134
> 	wait_for_completion+0x1c/0x2c
> 	kthread_flush_work+0x114/0x1cc
> 	kthread_cancel_work_sync.llvm.16514401384283632983+0xe8/0x144
> 	kthread_cancel_delayed_work_sync+0x18/0x2c
> 	xxxx_pm_notify+0xb0/0xd8
> 	blocking_notifier_call_chain_robust+0x80/0x194
> 	pm_notifier_call_chain_robust+0x28/0x4c
> 	suspend_prepare+0x40/0x260
> 	enter_state+0x80/0x3f4
> 	pm_suspend+0x60/0xdc
> 	state_store+0x108/0x144
> 	kobj_attr_store+0x38/0x88
> 	sysfs_kf_write+0x64/0xc0
> 	kernfs_fop_write_iter+0x108/0x1d0
> 	vfs_write+0x2f4/0x368
> 	ksys_write+0x7c/0xec
> 
> When we started investigating, we found race between
> kthread_mod_delayed_work vs kthread_cancel_delayed_work_sync. The race's
> result could be simply reproduced as a kthread_mod_delayed_work with
> a following kthread_flush_work call.

One more thing.

This scenario might mean that the kthread_worker API is used a wrong way.

kthread_cancel_delayed_work_sync() is typically called when the work
should never run any longer. But the parallel
kthread_mod_delayed_work() might queue it right after
kthread_cancel_delayed_work_sync() succeded. As a result that work
might get procced even when it was cancelled.

The API caller should prevent this race. It should have its own logic
around kthread_worker_queue*() and kthread_worker_mod_delayed_work()
calls that will prevent calling these when the work should
stay cancelled. For example, see

	if (clamping && w_data->clamping && cpu_online(w_data->cpu))
		kthread_queue_delayed_work(w_data->worker,
					   &w_data->idle_injection_work,
					   sleeptime);

in clamp_idle_injection_func().

Best Regards,
Petr