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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aDjHsJqV7L24qjvT@x1>
Date: Thu, 29 May 2025 17:46:40 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Howard Chu <howardchu95@...il.com>
Cc: Namhyung Kim <namhyung@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Kan Liang <kan.liang@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH] perf test: Add cgroup summary test case for perf trace

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@...nel.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@...hat.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@...nel.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