Re: [PATCH v11 8/9] perf scripts: Add perf-stuck, to tell where a running perf is stuck

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf scripts: incorrect parsing of /proc/pid/stat starttime causes premature exit
--

commit aa0fa29f56caddf899075009e74dfa310ea7373b
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf scripts: Add perf-stuck, to tell where a running perf is stuck

This commit adds a new script to monitor a running process's progress and
identify if it is stuck in a loop. It samples the target process at a fixed
interval and optionally invokes gdb when no progress is observed, verifying
process identity using the start time from the stat file.

> diff --git a/tools/perf/scripts/perf-stuck.sh b/tools/perf/scripts/perf-stuck.sh
> new file mode 100755
> index 0000000000000..57c979f6e58c5
> --- /dev/null
> +++ b/tools/perf/scripts/perf-stuck.sh
[ ... ]
> +# Its start time, in clock ticks, pins the process instance: a PID can
> +# name a different process later, samples or gdb would land on it.
> +pid_ticks=$(awk '{print $22}' /proc/"$pid"/stat)

[Severity: Medium]
In tools/perf/scripts/perf-stuck.sh, will this correctly parse the starttime
if the process name contains spaces? Because awk splits fields by whitespace,
spaces inside the parentheses of the comm field will shift the remaining
fields. If the name contains spaces, capturing field 22 might read volatile
metrics like num_threads or stime instead of the process starttime.

> +
> +echo "watching $pid ($cmdline) every ${interval}s"
> +
> +while :; do
> +	# The PID may name some other process by now, if the watched one
> +	# exited and its PID was recycled, e.g. within one interval:
> +	# checking existence alone is not enough, samples or gdb would
> +	# land on the stranger.
> +	if [ "$(awk '{print $22}' /proc/"$pid"/stat 2>/dev/null)" != "$pid_ticks" ]; then

[Severity: Medium]
Could this cause the script to exit prematurely? Since the initial extraction
of pid_ticks above might capture a volatile metric when the comm field
contains spaces, this check might trigger when that metric dynamically
updates, falsely detecting that the process has exited.

> +		echo "$(date +%T) process gone"
> +		break
> +	fi
> +
> +	# Field 2, the command name, is in parentheses and can contain
> +	# spaces, so drop it together with the pid before splitting so the
> +	# fields line up.  %d keeps the CPU time out of scientific
> +	# notation, that bash arithmetic can't parse past six digits.
> +	if ! stat_line=$(awk '{ sub(/^[^ ]+ \(.*\) /, "");
> +			       printf "%s %d %d\n", $1, $12 + $13, $22 }' \
> +			 /proc/"$pid"/stat 2>/dev/null); then

[Severity: Medium]
This isn't a bug here, but since the comm field is correctly stripped of
spaces in this awk call to avoid shifting fields, could the same protective
logic be applied to the earlier awk commands extracting the starttime?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004173000.2863935-1-acme@kernel.org?part=8