From: Oleg Nesterov oleg@redhat.com
[ Upstream commit 7904e53ed5a20fc678c01d5d1b07ec486425bb6a ]
do/while_each_thread should be avoided when possible.
Link: https://lkml.kernel.org/r/20230909164501.GA11581@redhat.com Signed-off-by: Oleg Nesterov oleg@redhat.com Cc: Eric W. Biederman ebiederm@xmission.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Stable-dep-of: 7601df8031fd ("fs/proc: do_task_stat: use sig->stats_lock to gather the threads/children stats") Cc: stable@vger.kernel.org [mheyne: adjusted context] Signed-off-by: Maximilian Heyne mheyne@amazon.de ---
Compile-tested only. We're seeing soft lock-ups with 5.10.237 because of the backport of commit 4fe85bdaabd6 ("fs/proc: do_task_stat: use sig->stats_lock to gather the threads/children stats").
--- fs/proc/array.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c index 8fba6d39e776..77b94c04e4af 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -512,18 +512,18 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns, cgtime = sig->cgtime;
if (whole) { - struct task_struct *t = task; + struct task_struct *t;
min_flt = sig->min_flt; maj_flt = sig->maj_flt; gtime = sig->gtime;
rcu_read_lock(); - do { + __for_each_thread(sig, t) { min_flt += t->min_flt; maj_flt += t->maj_flt; gtime += task_gtime(t); - } while_each_thread(task, t); + } rcu_read_unlock();
thread_group_cputime_adjusted(task, &utime, &stime);
On Thu, Jul 10, 2025 at 12:35:43PM +0000, Heyne, Maximilian wrote:
From: Oleg Nesterov oleg@redhat.com
[ Upstream commit 7904e53ed5a20fc678c01d5d1b07ec486425bb6a ]
do/while_each_thread should be avoided when possible.
Link: https://lkml.kernel.org/r/20230909164501.GA11581@redhat.com Signed-off-by: Oleg Nesterov oleg@redhat.com Cc: Eric W. Biederman ebiederm@xmission.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Stable-dep-of: 7601df8031fd ("fs/proc: do_task_stat: use sig->stats_lock to gather the threads/children stats") Cc: stable@vger.kernel.org [mheyne: adjusted context] Signed-off-by: Maximilian Heyne mheyne@amazon.de
Compile-tested only. We're seeing soft lock-ups with 5.10.237 because of the backport of commit 4fe85bdaabd6 ("fs/proc: do_task_stat: use sig->stats_lock to gather the threads/children stats").
And this fixes it?
How?
fs/proc/array.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c index 8fba6d39e776..77b94c04e4af 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -512,18 +512,18 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns, cgtime = sig->cgtime; if (whole) {
struct task_struct *t = task;
struct task_struct *t;min_flt = sig->min_flt; maj_flt = sig->maj_flt; gtime = sig->gtime; rcu_read_lock();
do {
__for_each_thread(sig, t) { min_flt += t->min_flt; maj_flt += t->maj_flt; gtime += task_gtime(t);
} while_each_thread(task, t);
}
Ideally, the code generated here should be identical as before, so why is this change needed?
confused,
greg k-h
Hi Greg,
On 10/07/25 18:39, Greg KH wrote:
On Thu, Jul 10, 2025 at 12:35:43PM +0000, Heyne, Maximilian wrote:
From: Oleg Nesterov oleg@redhat.com
[ Upstream commit 7904e53ed5a20fc678c01d5d1b07ec486425bb6a ]
do/while_each_thread should be avoided when possible.
Link: https://lkml.kernel.org/r/20230909164501.GA11581@redhat.com Signed-off-by: Oleg Nesterov oleg@redhat.com Cc: Eric W. Biederman ebiederm@xmission.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Stable-dep-of: 7601df8031fd ("fs/proc: do_task_stat: use sig->stats_lock to gather the threads/children stats") Cc: stable@vger.kernel.org [mheyne: adjusted context] Signed-off-by: Maximilian Heyne mheyne@amazon.de
Compile-tested only. We're seeing soft lock-ups with 5.10.237 because of the backport of commit 4fe85bdaabd6 ("fs/proc: do_task_stat: use sig->stats_lock to gather the threads/children stats").
And this fixes it?
Our testing also showed that after the backport of this commit(on 5.15.y based release), we don't see the soft lockup anymore.
How?
I think __for_each_thread() is safe whereas while_each_thread() is not safe.
This thread https://lore.kernel.org/all/20131202152437.GA10896@redhat.com/ explains why while_each_thread() is unsafe.
Thanks, Harshit
fs/proc/array.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c index 8fba6d39e776..77b94c04e4af 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -512,18 +512,18 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns, cgtime = sig->cgtime; if (whole) {
struct task_struct *t = task;
struct task_struct *t;min_flt = sig->min_flt; maj_flt = sig->maj_flt; gtime = sig->gtime; rcu_read_lock();
do {
__for_each_thread(sig, t) { min_flt += t->min_flt; maj_flt += t->maj_flt; gtime += task_gtime(t);
} while_each_thread(task, t);
}Ideally, the code generated here should be identical as before, so why is this change needed?
confused,>
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Harshit Mogalapalli -
Heyne, Maximilian