Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220903000210.1112014-1-namhyung@kernel.org> <CANpmjNMPh5QjdxXtrCc5FApjgzV=81CNNiwbeg_rE3NxN_WCZw@mail.gmail.com>
 <c4f7e14b-a229-6b23-b28d-880b8369e6d8@arm.com>
In-Reply-To: <c4f7e14b-a229-6b23-b28d-880b8369e6d8@arm.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Mon, 26 Sep 2022 09:19:59 -0700
Message-ID: <CAM9d7ch36RGYJJEg2dnrauQnVX_f9MiE0RB99EU4ynNL-8CUVA@mail.gmail.com>
Subject: Re: [PATCH] perf test: Skip sigtrap test on old kernels
To:     James Clark <james.clark@arm.com>
Cc:     Marco Elver <elver@google.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Ian Rogers <irogers@google.com>,
        linux-perf-users <linux-perf-users@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hello,

On Mon, Sep 26, 2022 at 8:06 AM James Clark <james.clark@arm.com> wrote:
>
>
>
> On 03/09/2022 07:52, Marco Elver wrote:
> > On Sat, 3 Sept 2022 at 02:02, Namhyung Kim <namhyung@kernel.org> wrote:
> >>
> >> If it runs on an old kernel, perf_event_open would fail because of the
> >> new fields sigtrap and sig_data.  Just skip the test if it failed.
> >>
> >> Cc: Marco Elver <elver@google.com>
> >> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >> ---
> >>  tools/perf/tests/sigtrap.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/tools/perf/tests/sigtrap.c b/tools/perf/tests/sigtrap.c
> >> index e32ece90e164..7057566e6ae4 100644
> >> --- a/tools/perf/tests/sigtrap.c
> >> +++ b/tools/perf/tests/sigtrap.c
> >> @@ -140,6 +140,7 @@ static int test__sigtrap(struct test_suite *test __maybe_unused, int subtest __m
> >>         fd = sys_perf_event_open(&attr, 0, -1, -1, perf_event_open_cloexec_flag());
> >>         if (fd < 0) {
> >>                 pr_debug("FAILED sys_perf_event_open(): %s\n", str_error_r(errno, sbuf, sizeof(sbuf)));
> >> +               ret = TEST_SKIP;
> >
> > Wouldn't we be interested if perf_event_open() fails because it could
> > actually be a bug? By skipping we'll be more likely to miss the fact
> > there's a real problem.
> >
> > That's my naive thinking at least - what do other perf tests usually
> > do in this case?
>
> I missed this discussion but I just submitted a patch with a similar
> issue [1]. To me, it doesn't make sense to have the tests pass on older
> kernels if this lowers the value of the tests by accepting possibly
> invalid values. If you want to test older kernels then just use older
> tests, but maybe there is some use case that I'm not aware of.

Thanks for your opinion.  But my test environment is running the tests
on random machines which may run some old kernel.  I agree that it
should not skip the real problems but I think we can find a good way
to detect old, unsupported kernels reliably like using BTF.

Thanks,
Namhyung