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;
Date:   Sat, 13 Aug 2022 22:17:39 +0800
From:   Leo Yan <leo.yan@linaro.org>
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     carsten.haitzler@foss.arm.com, linux-kernel@vger.kernel.org,
        coresight@lists.linaro.org, suzuki.poulose@arm.com,
        mathieu.poirier@linaro.org, mike.leach@linaro.org,
        linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v7 00/14] perf: test: Add trace data quality tests for
 CoreSight
Message-ID: <20220813141739.GB687527@leoy-huanghe.lan>
References: <20220812121641.336465-1-carsten.haitzler@foss.arm.com>
 <Yvaj2ukXV+SLIBR0@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yvaj2ukXV+SLIBR0@kernel.org>
Precedence: bulk

On Fri, Aug 12, 2022 at 04:02:50PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Aug 12, 2022 at 01:16:27PM +0100, carsten.haitzler@foss.arm.com escreveu:
> > From: Carsten Haitzler <carsten.haitzler@arm.com>
> > 
> > This is a prelude to adding more tests to shell tests and in order to
> > support putting those tests into subdirectories, I need to change the
> > test code that scans/finds and runs them.
> > 
> > To support subdirs I have to recurse so it's time to refactor the code to
> > allow this and centralize the shell script finding into one location and
> > only one single scan that builds a list of all the found tests in memory
> > instead of it being duplicated in 3 places.
> > 
> > This code also optimizes things like knowing the max width of desciption
> > strings (as we can do that while we scan instead of a whole new pass
> > of opening files). It also more cleanly filters scripts to see only
> > *.sh files thus skipping random other files in directories like *~
> > backup files, other random junk/data files that may appear and the
> > scripts must be executable to make the cut (this ensures the script
> > lib dir is not seen as scripts to run). This avoids perf test running
> > previous older versions of test scripts that are editor backup files
> > as well as skipping perf.data files that may appear and so on.
> > 
> > Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com>
> 
> Leo, can you please check if your previous Reviewed-by tags stand for
> this new version?

Yeah, after applied this patch series on the repo:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git, branch perf/core;
it has merging conflict for the patch 03 "perf test: Add build infra for perf
test tools for CoreSight tests".  It's good to rebase this patch set on the
latest perf/core branch.

I compared patch set v5 and v7, confirmed the my comments have been addressed
in v7.  Carsten also has updated patches' author address, it is same with the
address used in SoB, this is the right thing to do (Sorry I missed that).

I tested this patch set on my Juno board and observed a regression related with
CTI driver, the regression is not relevant with this patch set, I have
reported it on CoreSight mailing list.  This will be investigated separatly.

When played these test cases, I observed a failure for below case; but when I
re-run the case again and it passed.

  # ./perf test -v 78
   78: CoreSight / Thread Loop 10 Threads - Check TID                  :
  --- start ---
  test child forked, pid 6233
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.380 MB ./perf-thread_loop-check-tid-10th.data ]
  Thread IDs  6242 6242 6242 6242 6242 6242 6242 6242 not found in perf AUX data
  test child finished with -1
  ---- end ----
  CoreSight / Thread Loop 10 Threads - Check TID: FAILED!

You could see the perf data file is abnormally small (0.380 MiB), it should be
an issue for perf cs-etm record.  I don't think this is an issue for the test
case itself, we need to look at this issue later.

So with fixing the merging conflict:

Tested-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Leo Yan <leo.yan@linaro.org>

These test cases are for trace data quality, I'd like wait a bit for Mike's
green light since he has much better understanding.

Thanks,
Leo