All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.