Thread (49 messages) flat view 49 messages, 10 authors, 7d ago

Re: [PATCH v2 6/8] fs: convert process/thread iterators to for_each_*_rculock

From: sashiko-bot@kernel.org
Date: 2026-09-07 08:23:35

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 [off-list ref]
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).
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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/20260907081334.1152889-1-ye.liu@linux.dev?part=6
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help