Re: [PATCH net] amt: use cancel_delayed_work() instead of flush_delayed_work() in amt_fini()
From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-11-11 15:37:52
On Mon, 8 Nov 2021 14:53:40 +0000 Taehee Yoo wrote:
When the amt module is being removed, it calls flush_delayed_work() to exit source_gc_wq. But it wouldn't be exited properly because the amt_source_gc_work(), which is the callback function of source_gc_wq internally calls mod_delayed_work() again. So, amt_source_gc_work() would be called after the amt module is removed. Therefore kernel panic would occur. In order to avoid it, cancel_delayed_work() should be used instead of flush_delayed_work().
Somehow I convinced myself this is correct but now I'm not sure, again.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/amt.c b/drivers/net/amt.c index c384b2694f9e..47a04c330885 100644 --- a/drivers/net/amt.c +++ b/drivers/net/amt.c@@ -3286,7 +3286,7 @@ static void __exit amt_fini(void) { rtnl_link_unregister(&amt_link_ops); unregister_netdevice_notifier(&amt_notifier_block); - flush_delayed_work(&source_gc_wq); + cancel_delayed_work(&source_gc_wq);
This doesn't guarantee that the work is not running _right_ now and
will re-arm itself on the next clock cycle, so to speak.
CPU 0 CPU 1
----- -----
worker gets the work
clears pending
work starts running
cancel_work
grabs pending
clears pending
mod_delayed_work()
You need cancel_delayed_work_sync(), right?
__amt_source_gc_work(); destroy_workqueue(amt_wq); }