linux-kernel - Re: [PATCH 3/4] perf test: Do not skip when some metrics tests succeeded

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: <aWGcxpQXZZH5znwq@google.com>
Date: Fri, 9 Jan 2026 16:26:46 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
	James Clark <james.clark@...aro.org>, 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 3/4] perf test: Do not skip when some metrics tests
 succeeded

Hi Ian,

On Fri, Jan 09, 2026 at 03:37:01PM -0800, Ian Rogers wrote:
> On Thu, Dec 18, 2025 at 5:18 PM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > I think the return value of SKIP (2) should be used when it skipped the
> > entire test suite rather than a few of them.  While the FAIL should be
> > reserved if any of test failed.
> 
> This doesn't make sense to me. The current behavior is to set err to 0
> (success):
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/stat_all_metrics.sh?h=perf-tools-next#n18
> If a failure condition occurs then err is set to 1 (fail)
> unconditionally as you can't be more failing than failing, like:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/stat_all_metrics.sh?h=perf-tools-next#n38
> If a skip condition is encountered then we set err to 2 (skip)
> conditional on the err state only being currently 0 (success) - ie
> don't turn a fail into a skip:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/stat_all_metrics.sh?h=perf-tools-next#n45
> 
> All metrics are always tested by test, so I'm not sure what "entire
> test suite" means. The test should report success if nothing skips or
> fails, skip if something skips and there are no failures, and
> otherwise failure.

We have different ideas for the return values. :)  I think it should
report SKIP if it cannot test anything, FAILURE if anything fails and
otherwise SUCCESS.

So the gray area is when some subtests succeed and some skip (and no
failures).  It'd be great if we can agree on one way or another.

> 
> I can see the test being simplified by having a failure flag and a
> skip flag, then determining the 0, 1 or 2 value from these two flags.
> It would be avoiding testing the err value when setting it to 2
> (skip), but the change here isn't doing that. The change introduces a
> new value of 3, which seems more complex, and the skip flag is set
> depending on the global err value rather than just being a sticky flag
> showing a metric failed.

3 is not supposed to return, but I think it'd be nice if we can do
without introducing it.

Thanks,
Namhyung