Re: [PATCH v3 13/15] fs: convert process/thread iterators to for_each_*_rculock

[sashiko-bot@kernel.org]

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 <liuye@kylinos.cn>
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/20260911075800.491472-1-ye.liu@linux.dev?part=13