Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Mixing scope-based cleanup (scoped_guard) with goto-based error handling in the same function. --
commit e0076a589e993892daf78cfc52036d64c5fb68e6 Author: Ye Liu <[email protected]> fs: convert process/thread iterators to for_each_*_rculock This commit replaces manual rcu_read_lock() and rcu_read_unlock() pairs combined with process iterators in the fs/ directory with for_each_*_rculock() macros. These new macros scope the RCU read lock to the loop body using scoped_guard(rcu). > 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: Low] Does this code mix scope-based cleanup with goto-based error handling? Looking at __set_oom_adj(), the function still uses goto err_unlock; for error paths earlier in the function: fs/proc/base.c:__set_oom_adj() { ... if (legacy) { if (oom_adj < task->signal->oom_score_adj && !capable(CAP_SYS_RESOURCE)) { err = -EACCES; goto err_unlock; } ... } The newly introduced for_each_process_rculock() macro expands to use scoped_guard(rcu). The cleanup subsystem guidelines explicitly state that a function should either convert all resources that need cleanup to scope-based cleanup, or convert none of them, to avoid confusing ownership semantics. Could this function be fully converted, or should the rcu_read_lock() remain manual here until the rest of the error handling is updated? > if (same_thread_group(task, p)) > continue; [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
