From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BCFFCECAAD5 for ; Fri, 2 Sep 2022 17:32:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UsgzX2Vutk9OtjdC3BSYicGpIemA8Q+C7OQ8MeBmUNA=; b=VztcSE7I2qOTju s9VbN43EWCQG+ULllilGpaz9fi8kXAx4UInpYCUx1sJYIpUBovnd/Xa44PKSUGuYkvuEIC4nyBEdh nrOKXge4iaLwPPzxFowPHy0Co92QRPaLHYoU7cBzk9RIaqq+so1hhgWRxNHeKh33ivIgPMEyXJVAk hBEBj2bfifaOA+xqORV2LgE4kcQbaNTbDBgflyCLHgqrU7AaJwJPRiit9hY2+hDsvxuwfp+x8FMJV 0hymcTHM4K3bHmAPAXjjwUWNzRyBlHzcU9vWpB+EWx5PnNSUrkSB21miMqZAZn5CfPXnQAFXlcqVa 1jAxRkvQygflDoMDwz6A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oUAW8-0089XB-8z; Fri, 02 Sep 2022 17:31:44 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oUAW5-0089VU-7d for linux-arm-kernel@bombadil.infradead.org; Fri, 02 Sep 2022 17:31:41 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=y5a2rQ8bfLnP2lnm8YmBeEUNj176MDvDfh1v+d7tPOQ=; b=dR5sjJ9O/x6o+UBbpY1wnqjt1l 9nfBH4aViyN3YF66w9sAhTfX716GjD8fHDXNoI6b1AJeOpvm1akBJ7iu7jg/+fljmDxsqBuoNtTXA OT9rGxrKUUK5H9dbZhIBShUXdy1/VUfW2Yz8ImBco8HJlOL92FrzzSB8VV490Y1MJIcdpAISZbsqe /7NkT06kTIUkc/s9Ygau8FCzDpybXfk4fX1iAeaqW2UaYgieucVvU62mJ5NnFw3CcH5BRZ4v8c7G8 j4aYWme4BGhlEa5mM94YhEPMqO+1sizijJBnmMVvEuPU+0pPH9415j4jUd4tvioqbeL7y22OjJacy /HpmnuAA==; Received: from [179.97.37.151] (helo=quaco.ghostprotocols.net) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1oUAW3-007Cns-P2; Fri, 02 Sep 2022 17:31:40 +0000 Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id 959F3404A1; Fri, 2 Sep 2022 14:31:34 -0300 (-03) Date: Fri, 2 Sep 2022 14:31:34 -0300 From: Arnaldo Carvalho de Melo To: Anshuman Khandual Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, peterz@infradead.org, alexander.shishkin@linux.intel.com, jolsa@redhat.com, mark.rutland@arm.com, Robin Murphy , Suzuki Poulose , James Clark , Ingo Molnar , Namhyung Kim , Thomas Gleixner , Will Deacon , Catalin Marinas , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH V7 6/8] perf/tools: Extend branch type classification Message-ID: References: <20220824044822.70230-1-anshuman.khandual@arm.com> <20220824044822.70230-7-anshuman.khandual@arm.com> <9b1a8ebd-0562-f104-7439-308282f7fb52@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <9b1a8ebd-0562-f104-7439-308282f7fb52@arm.com> X-Url: http://acmel.wordpress.com X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Em Thu, Sep 01, 2022 at 10:37:24AM +0530, Anshuman Khandual escreveu: > > > On 8/31/22 02:41, Arnaldo Carvalho de Melo wrote: > > Em Wed, Aug 24, 2022 at 10:18:20AM +0530, Anshuman Khandual escreveu: > >> This updates the perf tool with generic branch type classification with new > >> ABI extender place holder i.e PERF_BR_EXTEND_ABI, the new 4 bit branch type > >> field i.e perf_branch_entry.new_type, new generic page fault related branch > >> types and some arch specific branch types as added earlier in the kernel. > >> > >> Cc: Peter Zijlstra > >> Cc: Ingo Molnar > >> Cc: Arnaldo Carvalho de Melo > >> Cc: Mark Rutland > >> Cc: Alexander Shishkin > >> Cc: Jiri Olsa > >> Cc: Namhyung Kim > >> Cc: Thomas Gleixner > >> Cc: Will Deacon > >> Cc: linux-arm-kernel@lists.infradead.org > >> Cc: linux-perf-users@vger.kernel.org > >> Cc: linux-kernel@vger.kernel.org > >> Signed-off-by: Anshuman Khandual > >> --- > >> tools/include/uapi/linux/perf_event.h | 16 ++++++++- > >> tools/perf/builtin-script.c | 2 +- > >> tools/perf/util/branch.c | 52 ++++++++++++++++++++++++++- > >> tools/perf/util/branch.h | 6 +++- > >> tools/perf/util/session.c | 2 +- > >> 5 files changed, 73 insertions(+), 5 deletions(-) > >> > >> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h > >> index 146c137ff0c1..0f7c7ce29899 100644 > >> --- a/tools/include/uapi/linux/perf_event.h > >> +++ b/tools/include/uapi/linux/perf_event.h > >> @@ -255,9 +255,22 @@ enum { > >> PERF_BR_IRQ = 12, /* irq */ > >> PERF_BR_SERROR = 13, /* system error */ > >> PERF_BR_NO_TX = 14, /* not in transaction */ > >> + PERF_BR_EXTEND_ABI = 15, /* extend ABI */ > >> PERF_BR_MAX, > >> }; > >> > >> +enum { > >> + PERF_BR_NEW_FAULT_ALGN = 0, /* Alignment fault */ > >> + PERF_BR_NEW_FAULT_DATA = 1, /* Data fault */ > >> + PERF_BR_NEW_FAULT_INST = 2, /* Inst fault */ > >> + PERF_BR_NEW_ARCH_1 = 3, /* Architecture specific */ > >> + PERF_BR_NEW_ARCH_2 = 4, /* Architecture specific */ > >> + PERF_BR_NEW_ARCH_3 = 5, /* Architecture specific */ > >> + PERF_BR_NEW_ARCH_4 = 6, /* Architecture specific */ > >> + PERF_BR_NEW_ARCH_5 = 7, /* Architecture specific */ > >> + PERF_BR_NEW_MAX, > >> +}; > >> + > >> #define PERF_SAMPLE_BRANCH_PLM_ALL \ > >> (PERF_SAMPLE_BRANCH_USER|\ > >> PERF_SAMPLE_BRANCH_KERNEL|\ > >> @@ -1375,7 +1388,8 @@ struct perf_branch_entry { > >> abort:1, /* transaction abort */ > >> cycles:16, /* cycle count to last branch */ > >> type:4, /* branch type */ > >> - reserved:40; > >> + new_type:4, /* additional branch type */ > >> + reserved:36; > >> }; > >> > >> union perf_sample_weight { > >> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > >> index 13580a9c50b8..585171479876 100644 > >> --- a/tools/perf/builtin-script.c > >> +++ b/tools/perf/builtin-script.c > >> @@ -877,7 +877,7 @@ static int print_bstack_flags(FILE *fp, struct branch_entry *br) > >> br->flags.in_tx ? 'X' : '-', > >> br->flags.abort ? 'A' : '-', > >> br->flags.cycles, > >> - br->flags.type ? branch_type_name(br->flags.type) : "-"); > >> + get_branch_type(br)); > >> } > >> > >> static int perf_sample__fprintf_brstack(struct perf_sample *sample, > >> diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c > >> index abc673347bee..6d962b0a4532 100644 > >> --- a/tools/perf/util/branch.c > >> +++ b/tools/perf/util/branch.c > >> @@ -21,7 +21,10 @@ void branch_type_count(struct branch_type_stat *st, struct branch_flags *flags, > >> if (flags->type == PERF_BR_UNKNOWN || from == 0) > >> return; > >> > >> - st->counts[flags->type]++; > >> + if (flags->type == PERF_BR_EXTEND_ABI) > >> + st->new_counts[flags->new_type]++; > >> + else > >> + st->counts[flags->type]++; > >> > >> if (flags->type == PERF_BR_COND) { > >> if (to > from) > >> @@ -36,6 +39,25 @@ void branch_type_count(struct branch_type_stat *st, struct branch_flags *flags, > >> st->cross_4k++; > >> } > >> > >> +const char *branch_new_type_name(int new_type) > >> +{ > >> + const char *branch_new_names[PERF_BR_NEW_MAX] = { > >> + "FAULT_ALGN", > >> + "FAULT_DATA", > >> + "FAULT_INST", > >> + "ARCH_1", > >> + "ARCH_2", > >> + "ARCH_3", > >> + "ARCH_4", > >> + "ARCH_5" > >> + }; > >> + > >> + if (new_type >= 0 && new_type < PERF_BR_NEW_MAX) > >> + return branch_new_names[new_type]; > >> + > >> + return NULL; > >> +} > >> + > >> const char *branch_type_name(int type) > >> { > >> const char *branch_names[PERF_BR_MAX] = { > >> @@ -62,6 +84,17 @@ const char *branch_type_name(int type) > >> return NULL; > >> } > >> > >> +const char *get_branch_type(struct branch_entry *e) > >> +{ > >> + if (e->flags.type == PERF_BR_UNKNOWN) > >> + return ""; > >> + > >> + if (e->flags.type == PERF_BR_EXTEND_ABI) > >> + return branch_new_type_name(e->flags.new_type); > >> + > >> + return branch_type_name(e->flags.type); > >> +} > >> + > >> void branch_type_stat_display(FILE *fp, struct branch_type_stat *st) > >> { > >> u64 total = 0; > >> @@ -108,6 +141,15 @@ void branch_type_stat_display(FILE *fp, struct branch_type_stat *st) > >> 100.0 * > >> (double)st->counts[i] / (double)total); > >> } > >> + > >> + for (i = 0; i < PERF_BR_NEW_MAX; i++) { > >> + if (st->new_counts[i] > 0) > >> + fprintf(fp, "\n%8s: %5.1f%%", > >> + branch_new_type_name(i), > >> + 100.0 * > >> + (double)st->new_counts[i] / (double)total); > >> + } > >> + > > Strange: > > > > 75 8.89 ubuntu:20.04-x-powerpc64el : FAIL gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1~20.04) > > inlined from 'branch_type_stat_display' at util/branch.c:152:4: > > /usr/powerpc64le-linux-gnu/include/bits/stdio2.h:100:10: error: '%8s' directive argument is null [-Werror=format-overflow=] > > 100 | return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt, > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > 101 | __va_arg_pack ()); > > | ~~~~~~~~~~~~~~~~~ > > > > Indeed. But this new code block here looks exact same like the previous and existing one > i.e with branch_new_name() and PERF_BR_NEW_MAX. The complain is that - '%8s' directive > argument is NULL. This warning might just be a false positive [1], because of a compiler > problem on powerpc64el ? But please do let me know if something needs to be changed here > to avoid this warning. > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90036 > So, I tried not returning NULL in the functions that are ultimately called, but that didn't help, so I'll try just disabling that specific warning for this specific file. The patch that didn't help is below. - Arnaldo commit 07c96060c410db6d10dbbdffb22bb46afebfe2c0 Author: Arnaldo Carvalho de Melo Date: Wed Aug 31 13:26:22 2022 -0300 perf branch: Don't return NULL on function that is used in a %s printf format To address this warning: In file included from /usr/include/stdio.h:866, from /home/sfr/next/next/tools/perf/util/branch.h:9, from util/branch.c:2: In function 'fprintf', inlined from 'branch_type_stat_display' at util/branch.c:152:4: /usr/include/powerpc64le-linux-gnu/bits/stdio2.h:105:10: error: '%8s' directive argument is null [-Werror=format-overflow=] 105 | return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 106 | __va_arg_pack ()); | ~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors Fixes: 9781e500dcb87eeb ("perf branch: Extend branch type classification") Cc: Anshuman Khandual Signed-off-by: Arnaldo Carvalho de Melo diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c index d40776c44b060b7e..f30366999f01e5e7 100644 --- a/tools/perf/util/branch.c +++ b/tools/perf/util/branch.c @@ -68,7 +68,7 @@ const char *branch_new_type_name(int new_type) if (new_type >= 0 && new_type < PERF_BR_NEW_MAX) return branch_new_names[new_type]; - return NULL; + return "<>"; } const char *branch_type_name(int type) @@ -94,7 +94,7 @@ const char *branch_type_name(int type) if (type >= 0 && type < PERF_BR_MAX) return branch_names[type]; - return NULL; + return "<>"; } const char *get_branch_type(struct branch_entry *e) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel