linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: irogers@google.com, acme@kernel.org, ak@linux.intel.com,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 3/9] perf report: Switch filtered from u8 to u16
Date: Tue, 18 Feb 2025 10:46:05 -0800	[thread overview]
Message-ID: <Z7TVbfUtD2LhYcj4@google.com> (raw)
In-Reply-To: <32b4ce1731126c88a2d9e191dc87e39ae4651cb7.1739437531.git.dvyukov@google.com>

On Thu, Feb 13, 2025 at 10:08:16AM +0100, Dmitry Vyukov wrote:
> We already have all u8 bits taken, adding one more filter leads to unpleasant
> failure mode, where code compiles w/o warnings, but the last filters silently
> don't work. Add a typedef and switch to u16.
> 
> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Ian Rogers <irogers@google.com>
> Cc: linux-perf-users@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  tools/perf/util/addr_location.h | 3 ++-
>  tools/perf/util/hist.c          | 2 +-
>  tools/perf/util/hist.h          | 4 +++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/addr_location.h b/tools/perf/util/addr_location.h
> index 36aaa45445f24..f83d74e370b2f 100644
> --- a/tools/perf/util/addr_location.h
> +++ b/tools/perf/util/addr_location.h
> @@ -3,6 +3,7 @@
>  #define __PERF_ADDR_LOCATION 1
>  
>  #include <linux/types.h>
> +#include "hist.h"
>  
>  struct thread;
>  struct maps;
> @@ -17,7 +18,7 @@ struct addr_location {
>  	const char    *srcline;
>  	u64	      addr;
>  	char	      level;
> -	u8	      filtered;
> +	filter_mask_t filtered;
>  	u8	      cpumode;
>  	s32	      cpu;
>  	s32	      socket;

This change introduced a build failure on Fedora 40 with libcapstone
like below.

  In file included from /usr/include/capstone/capstone.h:325,                     
                   from util/disasm.c:1333:                                       
  /usr/include/capstone/bpf.h:94:14: error: 'bpf_insn' defined as wrong kind of tag
     94 | typedef enum bpf_insn {                                                 
        |              ^~~~~~~~      
  make[4]: *** [/linux/tools/build/Makefile.build:86: /build/util/disasm.o] Error 1
  make[4]: *** Waiting for unfinished jobs....
  make[3]: *** [/linux/tools/build/Makefile.build:138: util] Error 2              
  make[2]: *** [Makefile.perf:822: /build/perf-util-in.o] Error 2                 
  make[2]: *** Waiting for unfinished jobs....                                    
  make[1]: *** [Makefile.perf:321: sub-make] Error 2                              
  make: *** [Makefile:76: all] Error 2     

I think we need the below change.  I'll fold it.

Thanks,
Namhyung


---8<---
diff --git a/tools/perf/util/addr_location.h b/tools/perf/util/addr_location.h
index 663e9a55d8ed3e46..64b5510252169239 100644
--- a/tools/perf/util/addr_location.h
+++ b/tools/perf/util/addr_location.h
@@ -3,7 +3,6 @@
 #define __PERF_ADDR_LOCATION 1
 
 #include <linux/types.h>
-#include "hist.h"
 
 struct thread;
 struct maps;
@@ -18,8 +17,8 @@ struct addr_location {
 	const char    *srcline;
 	u64	      addr;
 	char	      level;
-	filter_mask_t filtered;
 	u8	      cpumode;
+	u16	      filtered;
 	s32	      cpu;
 	s32	      socket;
 	/* Same as machine.parallelism but within [1, nr_cpus]. */

  reply	other threads:[~2025-02-18 18:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-13  9:08 [PATCH v7 0/9] perf report: Add latency and parallelism profiling Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 1/9] perf report: Add machine parallelism Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 2/9] perf report: Add parallelism sort key Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 3/9] perf report: Switch filtered from u8 to u16 Dmitry Vyukov
2025-02-18 18:46   ` Namhyung Kim [this message]
2025-02-13  9:08 ` [PATCH v7 4/9] perf report: Add parallelism filter Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 5/9] perf report: Add latency output field Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 6/9] perf report: Add --latency flag Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 7/9] perf report: Add latency and parallelism profiling documentation Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 8/9] perf test: Add tests for latency and parallelism profiling Dmitry Vyukov
2025-02-13  9:08 ` [PATCH v7 9/9] perf hist: Shrink struct hist_entry size Dmitry Vyukov
2025-02-14 18:05 ` [PATCH v7 0/9] perf report: Add latency and parallelism profiling Andi Kleen
2025-02-18  6:27   ` Dmitry Vyukov
2025-02-19 20:32 ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z7TVbfUtD2LhYcj4@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=dvyukov@google.com \
    --cc=irogers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).