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 15:10:18 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Leo Yan <leo.yan@linaro.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: <YvfpCnZ30tKFBfG+@kernel.org>
References: <20220812121641.336465-1-carsten.haitzler@foss.arm.com>
 <Yvaj2ukXV+SLIBR0@kernel.org>
 <20220813141739.GB687527@leoy-huanghe.lan>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220813141739.GB687527@leoy-huanghe.lan>
Precedence: bulk

Em Sat, Aug 13, 2022 at 10:17:39PM +0800, Leo Yan escreveu:
> 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>

  [PATCH v7 14/14] perf test: Add relevant documentation about CoreSight testing
    + Tested-by: Leo Yan <leo.yan@linaro.org> (✓ DKIM/linaro.org)
    + Reviewed-by: Leo Yan <leo.yan@linaro.org> (✓ DKIM/linaro.org)
---
Total patches: 14
---
NOTE: some trailers ignored due to from/email mismatches:
    ! Trailer: Link: https://lore.kernel.org/r/20220728145256.2985298-1-carsten.haitzler@foss.arm.com
     Msg From: Arnaldo Carvalho de Melo <acme@kernel.org>
NOTE: Rerun with -S to apply them anyway
---
Cover: ./v7_20220812_carsten_haitzler_perf_test_add_trace_data_quality_tests_for_coresight.cover
 Link: https://lore.kernel.org/r/20220812121641.336465-1-carsten.haitzler@foss.arm.com
 Base: not specified
       git am ./v7_20220812_carsten_haitzler_perf_test_add_trace_data_quality_tests_for_coresight.mbx
⬢[acme@toolbox perf]$        git am ./v7_20220812_carsten_haitzler_perf_test_add_trace_data_quality_tests_for_coresight.mbx
Applying: perf test: Refactor shell tests allowing subdirs
.git/rebase-apply/patch:56: trailing whitespace.
/*
warning: 1 line adds whitespace errors.
tools/perf/tests/builtin-test-list.c:29: trailing whitespace.
+/*
⬢[acme@toolbox perf]$


Fixing... 

- Arnaldo