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

[Howard Chu <howardchu95@gmail.com>]

Hello Arnaldo,

On Thu, May 29, 2025 at 1:46 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> 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.

Thanks for explaining this.

>
> 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.

Agreed.

>
> 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

:)

Thanks,
Howard