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;
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index cafd693568189..6b8f8da8d3b66 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -585,7 +585,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template,
 	return he;
 }
 
-static u8 symbol__parent_filter(const struct symbol *parent)
+static filter_mask_t symbol__parent_filter(const struct symbol *parent)
 {
 	if (symbol_conf.exclude_other && parent == NULL)
 		return 1 << HIST_FILTER__PARENT;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index a6e662d77dc24..4035106a74087 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -33,6 +33,8 @@ enum hist_filter {
 	HIST_FILTER__C2C,
 };
 
+typedef u16 filter_mask_t;
+
 enum hist_column {
 	HISTC_SYMBOL,
 	HISTC_TIME,
@@ -244,7 +246,7 @@ struct hist_entry {
 	bool			leaf;
 
 	char			level;
-	u8			filtered;
+	filter_mask_t		filtered;
 
 	u16			callchain_size;
 	union {
-- 
2.48.1.502.g6dc24dfdaf-goog

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]. */