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:   Tue, 8 Nov 2022 18:00:54 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc:     James Clark <james.clark@arm.com>,
        LKML <linux-kernel@vger.kernel.org>, Anshuman.Khandual@arm.com,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Kajol Jain <kjain@linux.ibm.com>,
        linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf test: Fix skipping branch stack sampling test
Message-ID: <Y2rDhkyn9ta5skDm@kernel.org>
References: <20221028121913.745307-1-james.clark@arm.com>
 <2265e166-da9c-f540-a108-0d0b133f7da9@arm.com>
 <F79AE69A-4D87-4F4E-A3FD-EF06A6E12839@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <F79AE69A-4D87-4F4E-A3FD-EF06A6E12839@linux.vnet.ibm.com>
Precedence: bulk

Em Wed, Nov 02, 2022 at 12:21:11PM +0530, Athira Rajeev escreveu:
> 
> 
> > On 28-Oct-2022, at 5:52 PM, James Clark <james.clark@arm.com> wrote:
> > 
> > 
> > 
> > On 28/10/2022 13:19, James Clark wrote:
> >> Commit f4a2aade6809 ("perf tests powerpc: Fix branch stack sampling test
> >> to include sanity check for branch filter") added a skip if certain
> >> branch options aren't available. But the change added both -b
> >> (--branch-any) and --branch-filter options at the same time, which will
> >> always result in a failure on any platform because the arguments can't
> >> be used together.
> >> 
> >> Fix this by removing -b (--branch-any) and leaving --branch-filter which
> >> already specifies 'any'. Also add warning messages to the test and perf
> >> tool.
> >> 
> > 
> > Hi Athira,
> > 
> > Are you able to check if this still skips for you on PowerPC with this
> > new change?
> > 
> 
> Hi James,
> 
> Sorry for the late response. I was out on vacation couple of days.
> 
> I tested with the new change and verified it skips on powerpc.
> 
> Tested-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

Thanks, applied.

- Arnaldo

 
> Thanks
> Athira
> > Thanks
> > James
> > 
> >> Output on x86 before this fix:
> >> 
> >>   $ sudo ./perf test branch
> >>   108: Check branch stack sampling         : Skip
> >> 
> >> After:
> >> 
> >>   $ sudo ./perf test branch
> >>   108: Check branch stack sampling         : Ok
> >> 
> >> Fixes: f4a2aade6809 ("perf tests powerpc: Fix branch stack sampling test to include sanity check for branch filter")
> >> Signed-off-by: James Clark <james.clark@arm.com>
> >> ---
> >> tools/perf/tests/shell/test_brstack.sh | 5 ++++-
> >> tools/perf/util/parse-branch-options.c | 4 +++-
> >> 2 files changed, 7 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/tools/perf/tests/shell/test_brstack.sh b/tools/perf/tests/shell/test_brstack.sh
> >> index ec801cffae6b..d7ff5c4b4da4 100755
> >> --- a/tools/perf/tests/shell/test_brstack.sh
> >> +++ b/tools/perf/tests/shell/test_brstack.sh
> >> @@ -13,7 +13,10 @@ fi
> >> 
> >> # skip the test if the hardware doesn't support branch stack sampling
> >> # and if the architecture doesn't support filter types: any,save_type,u
> >> -perf record -b -o- -B --branch-filter any,save_type,u true > /dev/null 2>&1 || exit 2
> >> +if ! perf record -o- --no-buildid --branch-filter any,save_type,u -- true > /dev/null 2>&1 ; then
> >> +	echo "skip: system doesn't support filter types: any,save_type,u"
> >> +	exit 2
> >> +fi
> >> 
> >> TMPDIR=$(mktemp -d /tmp/__perf_test.program.XXXXX)
> >> 
> >> diff --git a/tools/perf/util/parse-branch-options.c b/tools/perf/util/parse-branch-options.c
> >> index 00588b9db474..31faf2bb49ff 100644
> >> --- a/tools/perf/util/parse-branch-options.c
> >> +++ b/tools/perf/util/parse-branch-options.c
> >> @@ -102,8 +102,10 @@ parse_branch_stack(const struct option *opt, const char *str, int unset)
> >> 	/*
> >> 	 * cannot set it twice, -b + --branch-filter for instance
> >> 	 */
> >> -	if (*mode)
> >> +	if (*mode) {
> >> +		pr_err("Error: Can't use --branch-any (-b) with --branch-filter (-j).\n");
> >> 		return -1;
> >> +	}
> >> 
> >> 	return parse_branch_str(str, mode);
> >> }

-- 

- Arnaldo