Received-SPF: pass (google.com: domain of linux-kernel+bounces-138212-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <8d776c30-0e5c-4caf-9b7e-52adb6115eab@arm.com>
Date: Wed, 10 Apr 2024 10:07:26 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/3] perf tests: Skip "test data symbol" on Neoverse N1
To: Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>
Cc: linux-perf-users@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
 Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>,
 Mark Rutland <mark.rutland@arm.com>,
 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
 Jiri Olsa <jolsa@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>,
 "Liang, Kan" <kan.liang@linux.intel.com>,
 Athira Rajeev <atrajeev@linux.vnet.ibm.com>, Leo Yan <leo.yan@linux.dev>,
 linux-kernel@vger.kernel.org
References: <20240409084741.405433-1-james.clark@arm.com>
 <20240409084741.405433-4-james.clark@arm.com>
 <CAP-5=fXRODCKJexjxqkW_Wkf5MTxFJeq8zmu-iUbpEJM-9gjgg@mail.gmail.com>
 <CAM9d7cio+JYnzwtiVSpsmGrxJWyqv3m0ppmqGidHs5psZdi93w@mail.gmail.com>
Content-Language: en-US
From: James Clark <james.clark@arm.com>
In-Reply-To: <CAM9d7cio+JYnzwtiVSpsmGrxJWyqv3m0ppmqGidHs5psZdi93w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 10/04/2024 00:17, Namhyung Kim wrote:
> Hi Ian and James,
> 
> On Tue, Apr 9, 2024 at 8:39 AM Ian Rogers <irogers@google.com> wrote:
>>
>> On Tue, Apr 9, 2024 at 1:48 AM James Clark <james.clark@arm.com> wrote:
>>>
>>> To prevent anyone from seeing a test failure appear as a regression and
>>> thinking that it was caused by their code change, just skip the test on
>>> N1.
>>>
>>> It can be caused by any unrelated change that shifts the loop into an
>>> unfortunate position in the Perf binary which is almost impossible to
>>> debug as the root cause of the test failure. Ultimately it's caused by
>>> the referenced errata.
>>>
>>> Fixes: 60abedb8aa90 ("perf test: Introduce script for data symbol testing")
>>> Signed-off-by: James Clark <james.clark@arm.com>
>>
>> This change makes me sad :-( Is there no hope of aligning the loop? We
>> have little enough testing coverage for memory events and even precise
>> events on ARM that anything take away testing coverage feels like we
>> should try to do better.
> 
> Can we just add some noise in the loop?
> 
> Thanks,
> Namhyung
> 

Yes that would probably work. I decided to skip rather than touch the
test because I didn't want the errata on one specific core to affect
testing everywhere else.

But if we don't think that it's too hacky to include that in the test
then I can add it. To be honest maybe it makes the test more "realistic"
because a very short infinite loop doesn't really represent a real workload.

> 
> diff --git a/tools/perf/tests/workloads/datasym.c
> b/tools/perf/tests/workloads/datasym.c
> index ddd40bc63448..e2514bf393cd 100644
> --- a/tools/perf/tests/workloads/datasym.c
> +++ b/tools/perf/tests/workloads/datasym.c
> @@ -16,6 +16,8 @@ static int datasym(int argc __maybe_unused, const
> char **argv __maybe_unused)
>  {
>         for (;;) {
>                 buf1.data1++;
> +               if ((buf1.data1 % 100129) == 0)
> +                       buf1.data1++;
>                 buf1.data2 += buf1.data1;
>         }
>         return 0;
>