Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Mixing `goto`-based error handling with scope-based cleanup helpers in `__set_oom_adj()`. --
commit dca26f99f6fe0ae1bf52a29a296c82180cb53c70 Author: Ye Liu <[email protected]> Subject: fs: convert process/thread iterators to for_each_*_rculock This commit replaces the manual rcu_read_lock() and rcu_read_unlock() pairs combined with for_each_process() and for_each_process_thread() loops in the fs/ tree with the for_each_*_rculock() macros. These new macros scope the RCU read lock to the loop body using scoped_guard(). > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 58be389424605..2c47d838a1c76 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -1161,8 +1161,7 @@ static int __set_oom_adj(struct file *file, int > oom_adj, bool legacy) > if (mm) { > struct task_struct *p; > > - rcu_read_lock(); > - for_each_process(p) { > + for_each_process_rculock(p) { [Severity: Medium] Does this code introduce a regression by mixing scope-based cleanup helpers with goto-based error handling? The newly introduced for_each_process_rculock() macro expands to use scoped_guard(rcu). However, __set_oom_adj() still relies on goto-based cleanup paths earlier in the function: fs/proc/base.c:__set_oom_adj() { // ... mutex_lock(&oom_adj_mutex); if (legacy) { if (oom_adj < task->signal->oom_score_adj && !capable(CAP_SYS_RESOURCE)) { err = -EACCES; goto err_unlock; } // ... err_unlock: mutex_unlock(&oom_adj_mutex); put_task_struct(task); return err; } The kernel cleanup subsystem guidelines in include/linux/cleanup.h note that goto and cleanup helpers should never be mixed in the same function, as it creates confusing ownership semantics. Could this function either avoid using the scoped cleanup loop or fully convert the oom_adj_mutex to a guard() pattern as well? > if (same_thread_group(task, p)) > continue; [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
