Re: [PATCH 3/5] module/dups: Use scoped guards for kmod_dup_mutex
From: Petr Pavlu <petr.pavlu@suse.com>
Date: 2026-07-20 13:28:49
Also in:
sashiko-reviews
On 7/20/26 2:25 PM, sashiko-bot@kernel.org wrote:
quoted hunk ↗ jump to hunk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Mixing goto-based cleanup labels with scope-based cleanup (scoped_guard) in the same function. ----- Patch [3]: [PATCH 3/5] module/dups: Use scoped guards for kmod_dup_mutex ---commit 3299ca2ac8ae46392f412ae7c186f4907e7f8201 Author: Petr Pavlu [off-list ref] module/dups: Use scoped guards for kmod_dup_mutex This patch replaces explicit mutex_lock/unlock calls for kmod_dup_mutex with scoped_guard to shorten the code and clarify the lock's scope.quoted
diff --git a/kernel/module/dups.c b/kernel/module/dups.c index f0bdd7560d7e7..a62619ec057ed 100644 --- a/kernel/module/dups.c +++ b/kernel/module/dups.c[ ... ]quoted
@@ -122,10 +121,13 @@ bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret) INIT_DELAYED_WORK(&new_kmod_req->delete_work, kmod_dup_request_delete); init_completion(&new_kmod_req->first_req_done); - mutex_lock(&kmod_dup_mutex); + scoped_guard(mutex, &kmod_dup_mutex) { + kmod_req = kmod_dup_request_lookup(module_name); + if (kmod_req) { + get_kmod_req(kmod_req); + goto dup_req; + } - kmod_req = kmod_dup_request_lookup(module_name); - if (!kmod_req) {[Severity: Low] Does this mix scope-based cleanup with goto-based cleanup in the same function? Looking further down in kmod_dup_request_exists_wait(), there are still several goto statements that jump to an out label for manual cleanup: if (!wait) { /* ... */ *dup_ret = 0; goto out; } ret = wait_for_completion_state(&kmod_req->first_req_done, TASK_KILLABLE); if (ret) { *dup_ret = ret; goto out; } *dup_ret = kmod_req->dup_ret; out: put_kmod_req(kmod_req); return true; According to include/linux/cleanup.h, the expectation is that usage of "goto" and cleanup helpers is never mixed in the same function. All resources that need a "goto" cleanup should either be converted to scope-based cleanup, or none of them should. Should put_kmod_req() be converted to use scope-based cleanup as well, or should this function avoid using scoped_guard() to keep the cleanup pattern consistent?
Fair, I'll clean this up. Sashiko also reported the same issue in a reply to patch #5. -- Petr