Re: [PATCH] perf test: Add cgroup summary test case for perf trace

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Wed, May 28, 2025 at 11:59:44PM -0700, Howard Chu wrote:
> On Fri, May 23, 2025 at 3:12 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > On Thu, May 22, 2025 at 07:11:41PM -0300, Arnaldo Carvalho de Melo wrote:
> > > From 8c868979d886e2e88aa89f4e3d884e1b6450a7b2 Mon Sep 17 00:00:00 2001
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > Date: Thu, 22 May 2025 19:01:47 -0300
> > > Subject: [PATCH 1/1] perf tests trace_summary.sh: Run in exclusive mode

> > > And it is being successfull only when running alone, probably because
> > > there are some tests that add the vfs_getname probe that gets used by
> > > 'perf trace' and alter how it does syscall arg pathname resolution.

> > > This should be removed or made a fallback to the preferred BPF mode of
> > > getting syscall parameters, but till then, run this in exclusive mode.

> > > For reference, here are some of the tests that run close to this one:

> > >   127: perf record offcpu profiling tests                              : Ok
> > >   128: perf all PMU test                                               : Ok
> > >   129: perf stat --bpf-counters test                                   : Ok
> > >   130: Check Arm CoreSight trace data recording and synthesized samples: Skip
> > >   131: Check Arm CoreSight disassembly script completes without errors : Skip
> > >   132: Check Arm SPE trace data recording and synthesized samples      : Skip
> > >   133: Test data symbol                                                : Ok
> > >   134: Miscellaneous Intel PT testing                                  : Skip
> > >   135: test Intel TPEBS counting mode                                  : Skip
> > >   136: perf script task-analyzer tests                                 : Ok
> > >   137: Check open filename arg using perf trace + vfs_getname          : Ok
> > >   138: perf trace summary                                              : Ok

> > Looks good to me.

> > Acked-by: Namhyung Kim <namhyung@kernel.org>
 
> Nacked (sorry). I think running them tests in parallel is great
> because it points out a problem that perf trace has. Please check out
> this approach: https://lore.kernel.org/linux-perf-users/20250529065537.529937-1-howardchu95@gmail.com/T/#u

I'm not saying that perf trace shouldn't be used in parallel, but the
vfs_getname code, IIRC, checks for the existence of that probe to do
pathname collection (this predates the BPF method by a long time) and
then counts on it to do.

There are tests that put it in place and then at the end remove it,
multiple tests.

So there are possible races with that and out of being conservative I
made it exclusive for the time being.

The plan is to remove that vfs_getname code in builtin-trace.c and then
the tests, as we have the BPF method that is way better and should allow
for parallel use.

Probably in the meantime it would be better to mark the vfs_getname ones
as exclusive tho now that I that I wrote the above explanation... :-\

- Arnaldo