2020-11-05 23:14:22

by Vineet Gupta

Subject: [RFC] proc: get_wchan() stack unwind only makes sense for sleeping/non-self tasks

Most architectures currently check this in their get_wchan() implementation
(ARC doesn't hence this patch). However doing this in core code shows
the semantics better so move the check one level up (eventually remove
the boiler-plate code from arches)

Signed-off-by: Vineet Gupta <[email protected]>

# tools/perf/arch/arc/util/
---
fs/proc/array.c | 4 +++-
fs/proc/base.c | 6 ++++--
2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 65ec2029fa80..081fade5a361 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -519,8 +519,10 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
unlock_task_sighand(task, &flags);
}

- if (permitted && (!whole || num_threads < 2))
+ if (task != current && task->state != TASK_RUNNING &&
+ permitted && (!whole || num_threads < 2))
wchan = get_wchan(task);
+
if (!whole) {
min_flt = task->min_flt;
maj_flt = task->maj_flt;
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 0f707003dda5..abd7ec6324c5 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -385,13 +385,15 @@ static const struct file_operations proc_pid_cmdline_ops = {
static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
struct pid *pid, struct task_struct *task)
{
- unsigned long wchan;
+ unsigned long wchan = 0;
char symname[KSYM_NAME_LEN];

if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
goto print0;

- wchan = get_wchan(task);
+ if (task != current && task->state != TASK_RUNNING)
+ wchan = get_wchan(task);
+
if (wchan && !lookup_symbol_name(wchan, symname)) {
seq_puts(m, symname);
return 0;
--
2.25.1

2020-11-07 05:10:38

by Andrew Morton

Subject: Re: [RFC] proc: get_wchan() stack unwind only makes sense for sleeping/non-self tasks

On Thu, 5 Nov 2020 15:11:32 -0800 Vineet Gupta <[email protected]> wrote:

> Most architectures currently check this in their get_wchan() implementation
> (ARC doesn't hence this patch). However doing this in core code shows
> the semantics better so move the check one level up (eventually remove
> the boiler-plate code from arches)

It would be nice to clean up the arch callees in the same patch, at
least so it doesn't get forgotten about. Are you prepared to propose
such a change?