All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: Add irq and exception return branch types
@ 2022-02-24  5:36 ` Anshuman Khandual
  0 siblings, 0 replies; 9+ messages in thread
From: Anshuman Khandual @ 2022-02-24  5:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Anshuman Khandual, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Thomas Gleixner, Will Deacon,
	linux-arm-kernel, linux-perf-users

This expands generic branch type classification by adding two more entries
there in i.e irq and exception return. Also updates the x86 implementation
to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
branch types reported to user space on x86 platform but it should not be a
problem. The possible scenarios and impacts are enumerated here.

--------------------------------------------------------------------------
| kernel | perf tool |                     Impact                        |
--------------------------------------------------------------------------
|   old  |    old    |  Works as before                                  |
--------------------------------------------------------------------------
|   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
--------------------------------------------------------------------------
|   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
--------------------------------------------------------------------------
|   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
--------------------------------------------------------------------------

When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
perf tool) the user space might throw up an warning complaining about some
unrecognized branch types being reported, but it is expected. PERF_BR_ERET
and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
platform.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
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 <anshuman.khandual@arm.com>
---
This applies on v5.17-rc5

These two new branch types expands generic branch type classification but
still leaves another three entries in 'type' field for later. Please refer
a previous discussion [1] for some further context.

[1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/

 arch/x86/events/intel/lbr.c           | 4 ++--
 include/uapi/linux/perf_event.h       | 2 ++
 tools/include/uapi/linux/perf_event.h | 2 ++
 tools/perf/util/branch.c              | 4 +++-
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 669c2be14784..fe1742c4ca49 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = {
 	PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
 	PERF_BR_SYSRET,		/* X86_BR_SYSRET */
 	PERF_BR_UNKNOWN,	/* X86_BR_INT */
-	PERF_BR_UNKNOWN,	/* X86_BR_IRET */
+	PERF_BR_ERET,		/* X86_BR_IRET */
 	PERF_BR_COND,		/* X86_BR_JCC */
 	PERF_BR_UNCOND,		/* X86_BR_JMP */
-	PERF_BR_UNKNOWN,	/* X86_BR_IRQ */
+	PERF_BR_IRQ,		/* X86_BR_IRQ */
 	PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
 	PERF_BR_UNKNOWN,	/* X86_BR_ABORT */
 	PERF_BR_UNKNOWN,	/* X86_BR_IN_TX */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 82858b697c05..d37629dbad72 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -251,6 +251,8 @@ enum {
 	PERF_BR_SYSRET		= 8,	/* syscall return */
 	PERF_BR_COND_CALL	= 9,	/* conditional function call */
 	PERF_BR_COND_RET	= 10,	/* conditional function return */
+	PERF_BR_ERET		= 11,	/* exception return */
+	PERF_BR_IRQ		= 12,	/* irq */
 	PERF_BR_MAX,
 };
 
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 82858b697c05..d37629dbad72 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -251,6 +251,8 @@ enum {
 	PERF_BR_SYSRET		= 8,	/* syscall return */
 	PERF_BR_COND_CALL	= 9,	/* conditional function call */
 	PERF_BR_COND_RET	= 10,	/* conditional function return */
+	PERF_BR_ERET		= 11,	/* exception return */
+	PERF_BR_IRQ		= 12,	/* irq */
 	PERF_BR_MAX,
 };
 
diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
index 2285b1eb3128..a9a909db8cc7 100644
--- a/tools/perf/util/branch.c
+++ b/tools/perf/util/branch.c
@@ -49,7 +49,9 @@ const char *branch_type_name(int type)
 		"SYSCALL",
 		"SYSRET",
 		"COND_CALL",
-		"COND_RET"
+		"COND_RET",
+		"ERET",
+		"IRQ"
 	};
 
 	if (type >= 0 && type < PERF_BR_MAX)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] perf: Add irq and exception return branch types
@ 2022-02-24  5:36 ` Anshuman Khandual
  0 siblings, 0 replies; 9+ messages in thread
From: Anshuman Khandual @ 2022-02-24  5:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Anshuman Khandual, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Thomas Gleixner, Will Deacon,
	linux-arm-kernel, linux-perf-users

This expands generic branch type classification by adding two more entries
there in i.e irq and exception return. Also updates the x86 implementation
to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
branch types reported to user space on x86 platform but it should not be a
problem. The possible scenarios and impacts are enumerated here.

--------------------------------------------------------------------------
| kernel | perf tool |                     Impact                        |
--------------------------------------------------------------------------
|   old  |    old    |  Works as before                                  |
--------------------------------------------------------------------------
|   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
--------------------------------------------------------------------------
|   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
--------------------------------------------------------------------------
|   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
--------------------------------------------------------------------------

When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
perf tool) the user space might throw up an warning complaining about some
unrecognized branch types being reported, but it is expected. PERF_BR_ERET
and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
platform.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
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 <anshuman.khandual@arm.com>
---
This applies on v5.17-rc5

These two new branch types expands generic branch type classification but
still leaves another three entries in 'type' field for later. Please refer
a previous discussion [1] for some further context.

[1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/

 arch/x86/events/intel/lbr.c           | 4 ++--
 include/uapi/linux/perf_event.h       | 2 ++
 tools/include/uapi/linux/perf_event.h | 2 ++
 tools/perf/util/branch.c              | 4 +++-
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 669c2be14784..fe1742c4ca49 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = {
 	PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
 	PERF_BR_SYSRET,		/* X86_BR_SYSRET */
 	PERF_BR_UNKNOWN,	/* X86_BR_INT */
-	PERF_BR_UNKNOWN,	/* X86_BR_IRET */
+	PERF_BR_ERET,		/* X86_BR_IRET */
 	PERF_BR_COND,		/* X86_BR_JCC */
 	PERF_BR_UNCOND,		/* X86_BR_JMP */
-	PERF_BR_UNKNOWN,	/* X86_BR_IRQ */
+	PERF_BR_IRQ,		/* X86_BR_IRQ */
 	PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
 	PERF_BR_UNKNOWN,	/* X86_BR_ABORT */
 	PERF_BR_UNKNOWN,	/* X86_BR_IN_TX */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 82858b697c05..d37629dbad72 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -251,6 +251,8 @@ enum {
 	PERF_BR_SYSRET		= 8,	/* syscall return */
 	PERF_BR_COND_CALL	= 9,	/* conditional function call */
 	PERF_BR_COND_RET	= 10,	/* conditional function return */
+	PERF_BR_ERET		= 11,	/* exception return */
+	PERF_BR_IRQ		= 12,	/* irq */
 	PERF_BR_MAX,
 };
 
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 82858b697c05..d37629dbad72 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -251,6 +251,8 @@ enum {
 	PERF_BR_SYSRET		= 8,	/* syscall return */
 	PERF_BR_COND_CALL	= 9,	/* conditional function call */
 	PERF_BR_COND_RET	= 10,	/* conditional function return */
+	PERF_BR_ERET		= 11,	/* exception return */
+	PERF_BR_IRQ		= 12,	/* irq */
 	PERF_BR_MAX,
 };
 
diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
index 2285b1eb3128..a9a909db8cc7 100644
--- a/tools/perf/util/branch.c
+++ b/tools/perf/util/branch.c
@@ -49,7 +49,9 @@ const char *branch_type_name(int type)
 		"SYSCALL",
 		"SYSRET",
 		"COND_CALL",
-		"COND_RET"
+		"COND_RET",
+		"ERET",
+		"IRQ"
 	};
 
 	if (type >= 0 && type < PERF_BR_MAX)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf: Add irq and exception return branch types
  2022-02-24  5:36 ` Anshuman Khandual
@ 2022-02-28 15:45   ` James Clark
  -1 siblings, 0 replies; 9+ messages in thread
From: James Clark @ 2022-02-28 15:45 UTC (permalink / raw)
  To: Anshuman Khandual, linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Will Deacon, linux-arm-kernel, linux-perf-users



On 24/02/2022 05:36, Anshuman Khandual wrote:
> This expands generic branch type classification by adding two more entries
> there in i.e irq and exception return. Also updates the x86 implementation
> to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
> branch types reported to user space on x86 platform but it should not be a
> problem. The possible scenarios and impacts are enumerated here.
> 
> --------------------------------------------------------------------------
> | kernel | perf tool |                     Impact                        |
> --------------------------------------------------------------------------
> |   old  |    old    |  Works as before                                  |
> --------------------------------------------------------------------------
> |   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
> --------------------------------------------------------------------------
> |   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
> --------------------------------------------------------------------------
> |   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
> --------------------------------------------------------------------------
> 
> When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
> perf tool) the user space might throw up an warning complaining about some
> unrecognized branch types being reported, but it is expected. PERF_BR_ERET
> and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
> platform.
> 
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will@kernel.org>
> 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 <anshuman.khandual@arm.com>
> ---
> This applies on v5.17-rc5
> 
> These two new branch types expands generic branch type classification but
> still leaves another three entries in 'type' field for later. Please refer
> a previous discussion [1] for some further context.
> 
> [1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/
> 
>  arch/x86/events/intel/lbr.c           | 4 ++--
>  include/uapi/linux/perf_event.h       | 2 ++
>  tools/include/uapi/linux/perf_event.h | 2 ++
>  tools/perf/util/branch.c              | 4 +++-
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 669c2be14784..fe1742c4ca49 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = {
>  	PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
>  	PERF_BR_SYSRET,		/* X86_BR_SYSRET */
>  	PERF_BR_UNKNOWN,	/* X86_BR_INT */
> -	PERF_BR_UNKNOWN,	/* X86_BR_IRET */
> +	PERF_BR_ERET,		/* X86_BR_IRET */
>  	PERF_BR_COND,		/* X86_BR_JCC */
>  	PERF_BR_UNCOND,		/* X86_BR_JMP */
> -	PERF_BR_UNKNOWN,	/* X86_BR_IRQ */
> +	PERF_BR_IRQ,		/* X86_BR_IRQ */
Hi Anshuman,

I couldn't verify if these changes to the lbr map matched up to the spec because
I couldn't find the right section. I suppose there might need to be a comment
about why BR_ERET == BR_IRET or what those abbreviations are.

I think it could also be possible to leave these entries as unknowns if we don't know
if they're even being used. It will always be possible to go back and update these
lbr mappings after you've added the new types.


>  	PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
>  	PERF_BR_UNKNOWN,	/* X86_BR_ABORT */
>  	PERF_BR_UNKNOWN,	/* X86_BR_IN_TX */
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 82858b697c05..d37629dbad72 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -251,6 +251,8 @@ enum {
>  	PERF_BR_SYSRET		= 8,	/* syscall return */
>  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
>  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> +	PERF_BR_ERET		= 11,	/* exception return */
> +	PERF_BR_IRQ		= 12,	/* irq */
>  	PERF_BR_MAX,>  };
>  
> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index 82858b697c05..d37629dbad72 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h
> @@ -251,6 +251,8 @@ enum {
>  	PERF_BR_SYSRET		= 8,	/* syscall return */
>  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
>  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> +	PERF_BR_ERET		= 11,	/* exception return */
> +	PERF_BR_IRQ		= 12,	/* irq */
>  	PERF_BR_MAX,
>  };
>  
> diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
> index 2285b1eb3128..a9a909db8cc7 100644
> --- a/tools/perf/util/branch.c
> +++ b/tools/perf/util/branch.c
> @@ -49,7 +49,9 @@ const char *branch_type_name(int type)
>  		"SYSCALL",
>  		"SYSRET",
>  		"COND_CALL",
> -		"COND_RET"
> +		"COND_RET",
> +		"ERET",
> +		"IRQ"
>  	};
>
Otherwise the new entries look good to me, so without the lbr changes:

Reviewed-by: James Clark <james.clark@arm.com>

If we're keeping the lbr mapping changes, then I will defer to someone else
to review.

Thanks
James

>  	if (type >= 0 && type < PERF_BR_MAX)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf: Add irq and exception return branch types
@ 2022-02-28 15:45   ` James Clark
  0 siblings, 0 replies; 9+ messages in thread
From: James Clark @ 2022-02-28 15:45 UTC (permalink / raw)
  To: Anshuman Khandual, linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Will Deacon, linux-arm-kernel, linux-perf-users



On 24/02/2022 05:36, Anshuman Khandual wrote:
> This expands generic branch type classification by adding two more entries
> there in i.e irq and exception return. Also updates the x86 implementation
> to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
> branch types reported to user space on x86 platform but it should not be a
> problem. The possible scenarios and impacts are enumerated here.
> 
> --------------------------------------------------------------------------
> | kernel | perf tool |                     Impact                        |
> --------------------------------------------------------------------------
> |   old  |    old    |  Works as before                                  |
> --------------------------------------------------------------------------
> |   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
> --------------------------------------------------------------------------
> |   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
> --------------------------------------------------------------------------
> |   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
> --------------------------------------------------------------------------
> 
> When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
> perf tool) the user space might throw up an warning complaining about some
> unrecognized branch types being reported, but it is expected. PERF_BR_ERET
> and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
> platform.
> 
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will@kernel.org>
> 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 <anshuman.khandual@arm.com>
> ---
> This applies on v5.17-rc5
> 
> These two new branch types expands generic branch type classification but
> still leaves another three entries in 'type' field for later. Please refer
> a previous discussion [1] for some further context.
> 
> [1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/
> 
>  arch/x86/events/intel/lbr.c           | 4 ++--
>  include/uapi/linux/perf_event.h       | 2 ++
>  tools/include/uapi/linux/perf_event.h | 2 ++
>  tools/perf/util/branch.c              | 4 +++-
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 669c2be14784..fe1742c4ca49 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = {
>  	PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
>  	PERF_BR_SYSRET,		/* X86_BR_SYSRET */
>  	PERF_BR_UNKNOWN,	/* X86_BR_INT */
> -	PERF_BR_UNKNOWN,	/* X86_BR_IRET */
> +	PERF_BR_ERET,		/* X86_BR_IRET */
>  	PERF_BR_COND,		/* X86_BR_JCC */
>  	PERF_BR_UNCOND,		/* X86_BR_JMP */
> -	PERF_BR_UNKNOWN,	/* X86_BR_IRQ */
> +	PERF_BR_IRQ,		/* X86_BR_IRQ */
Hi Anshuman,

I couldn't verify if these changes to the lbr map matched up to the spec because
I couldn't find the right section. I suppose there might need to be a comment
about why BR_ERET == BR_IRET or what those abbreviations are.

I think it could also be possible to leave these entries as unknowns if we don't know
if they're even being used. It will always be possible to go back and update these
lbr mappings after you've added the new types.


>  	PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
>  	PERF_BR_UNKNOWN,	/* X86_BR_ABORT */
>  	PERF_BR_UNKNOWN,	/* X86_BR_IN_TX */
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 82858b697c05..d37629dbad72 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -251,6 +251,8 @@ enum {
>  	PERF_BR_SYSRET		= 8,	/* syscall return */
>  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
>  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> +	PERF_BR_ERET		= 11,	/* exception return */
> +	PERF_BR_IRQ		= 12,	/* irq */
>  	PERF_BR_MAX,>  };
>  
> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index 82858b697c05..d37629dbad72 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h
> @@ -251,6 +251,8 @@ enum {
>  	PERF_BR_SYSRET		= 8,	/* syscall return */
>  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
>  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> +	PERF_BR_ERET		= 11,	/* exception return */
> +	PERF_BR_IRQ		= 12,	/* irq */
>  	PERF_BR_MAX,
>  };
>  
> diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
> index 2285b1eb3128..a9a909db8cc7 100644
> --- a/tools/perf/util/branch.c
> +++ b/tools/perf/util/branch.c
> @@ -49,7 +49,9 @@ const char *branch_type_name(int type)
>  		"SYSCALL",
>  		"SYSRET",
>  		"COND_CALL",
> -		"COND_RET"
> +		"COND_RET",
> +		"ERET",
> +		"IRQ"
>  	};
>
Otherwise the new entries look good to me, so without the lbr changes:

Reviewed-by: James Clark <james.clark@arm.com>

If we're keeping the lbr mapping changes, then I will defer to someone else
to review.

Thanks
James

>  	if (type >= 0 && type < PERF_BR_MAX)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip: perf/core] perf: Add irq and exception return branch types
  2022-02-24  5:36 ` Anshuman Khandual
  (?)
  (?)
@ 2022-03-01 15:24 ` tip-bot2 for Anshuman Khandual
  -1 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Anshuman Khandual @ 2022-03-01 15:24 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Peter Zijlstra (Intel), x86, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     cedd3614e5d9c80908099c19f8716714ce0610b1
Gitweb:        https://git.kernel.org/tip/cedd3614e5d9c80908099c19f8716714ce0610b1
Author:        Anshuman Khandual <anshuman.khandual@arm.com>
AuthorDate:    Thu, 24 Feb 2022 11:06:54 +05:30
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 01 Mar 2022 16:19:01 +01:00

perf: Add irq and exception return branch types

This expands generic branch type classification by adding two more entries
there in i.e irq and exception return. Also updates the x86 implementation
to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
branch types reported to user space on x86 platform but it should not be a
problem. The possible scenarios and impacts are enumerated here.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/1645681014-3346-1-git-send-email-anshuman.khandual@arm.com
---
 arch/x86/events/intel/lbr.c           | 4 ++--
 include/uapi/linux/perf_event.h       | 2 ++
 tools/include/uapi/linux/perf_event.h | 2 ++
 tools/perf/util/branch.c              | 4 +++-
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 669c2be..fe1742c 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = {
 	PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
 	PERF_BR_SYSRET,		/* X86_BR_SYSRET */
 	PERF_BR_UNKNOWN,	/* X86_BR_INT */
-	PERF_BR_UNKNOWN,	/* X86_BR_IRET */
+	PERF_BR_ERET,		/* X86_BR_IRET */
 	PERF_BR_COND,		/* X86_BR_JCC */
 	PERF_BR_UNCOND,		/* X86_BR_JMP */
-	PERF_BR_UNKNOWN,	/* X86_BR_IRQ */
+	PERF_BR_IRQ,		/* X86_BR_IRQ */
 	PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
 	PERF_BR_UNKNOWN,	/* X86_BR_ABORT */
 	PERF_BR_UNKNOWN,	/* X86_BR_IN_TX */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 1b65042..7dc7176 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -251,6 +251,8 @@ enum {
 	PERF_BR_SYSRET		= 8,	/* syscall return */
 	PERF_BR_COND_CALL	= 9,	/* conditional function call */
 	PERF_BR_COND_RET	= 10,	/* conditional function return */
+	PERF_BR_ERET		= 11,	/* exception return */
+	PERF_BR_IRQ		= 12,	/* irq */
 	PERF_BR_MAX,
 };
 
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 4cd39aa..d1324c4 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -251,6 +251,8 @@ enum {
 	PERF_BR_SYSRET		= 8,	/* syscall return */
 	PERF_BR_COND_CALL	= 9,	/* conditional function call */
 	PERF_BR_COND_RET	= 10,	/* conditional function return */
+	PERF_BR_ERET		= 11,	/* exception return */
+	PERF_BR_IRQ		= 12,	/* irq */
 	PERF_BR_MAX,
 };
 
diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
index 2285b1e..a9a909d 100644
--- a/tools/perf/util/branch.c
+++ b/tools/perf/util/branch.c
@@ -49,7 +49,9 @@ const char *branch_type_name(int type)
 		"SYSCALL",
 		"SYSRET",
 		"COND_CALL",
-		"COND_RET"
+		"COND_RET",
+		"ERET",
+		"IRQ"
 	};
 
 	if (type >= 0 && type < PERF_BR_MAX)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf: Add irq and exception return branch types
  2022-02-28 15:45   ` James Clark
@ 2022-03-05 19:04     ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-03-05 19:04 UTC (permalink / raw)
  To: James Clark
  Cc: Anshuman Khandual, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Will Deacon, linux-arm-kernel, linux-perf-users

Em Mon, Feb 28, 2022 at 03:45:25PM +0000, James Clark escreveu:
> 
> 
> On 24/02/2022 05:36, Anshuman Khandual wrote:
> > This expands generic branch type classification by adding two more entries
> > there in i.e irq and exception return. Also updates the x86 implementation
> > to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
> > branch types reported to user space on x86 platform but it should not be a
> > problem. The possible scenarios and impacts are enumerated here.
> > 
> > --------------------------------------------------------------------------
> > | kernel | perf tool |                     Impact                        |
> > --------------------------------------------------------------------------
> > |   old  |    old    |  Works as before                                  |
> > --------------------------------------------------------------------------
> > |   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
> > --------------------------------------------------------------------------
> > |   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
> > --------------------------------------------------------------------------
> > |   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
> > --------------------------------------------------------------------------
> > 
> > When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
> > perf tool) the user space might throw up an warning complaining about some
> > unrecognized branch types being reported, but it is expected. PERF_BR_ERET
> > and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
> > platform.
> > 
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Will Deacon <will@kernel.org>
> > 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 <anshuman.khandual@arm.com>
> > ---
> > This applies on v5.17-rc5
> > 
> > These two new branch types expands generic branch type classification but
> > still leaves another three entries in 'type' field for later. Please refer
> > a previous discussion [1] for some further context.
> > 
> > [1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/
> > 
> >  arch/x86/events/intel/lbr.c           | 4 ++--
> >  include/uapi/linux/perf_event.h       | 2 ++

Please try to avoid lockstep development of kernel and tools/, submit
patches to the kernel maintainers for the kernel parts, and to the perf
tools maintainer in separate patches.

It is important that changes to the API are flagged, for instance via
tools/perf/check-headers.sh so that opportunity is given for the various
people involved in perf (u/k) development to see what is going on.

Thanks,

- Arnaldo

> >  tools/include/uapi/linux/perf_event.h | 2 ++
> >  tools/perf/util/branch.c              | 4 +++-
> >  4 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> > index 669c2be14784..fe1742c4ca49 100644
> > --- a/arch/x86/events/intel/lbr.c
> > +++ b/arch/x86/events/intel/lbr.c
> > @@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = {
> >  	PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
> >  	PERF_BR_SYSRET,		/* X86_BR_SYSRET */
> >  	PERF_BR_UNKNOWN,	/* X86_BR_INT */
> > -	PERF_BR_UNKNOWN,	/* X86_BR_IRET */
> > +	PERF_BR_ERET,		/* X86_BR_IRET */
> >  	PERF_BR_COND,		/* X86_BR_JCC */
> >  	PERF_BR_UNCOND,		/* X86_BR_JMP */
> > -	PERF_BR_UNKNOWN,	/* X86_BR_IRQ */
> > +	PERF_BR_IRQ,		/* X86_BR_IRQ */
> Hi Anshuman,
> 
> I couldn't verify if these changes to the lbr map matched up to the spec because
> I couldn't find the right section. I suppose there might need to be a comment
> about why BR_ERET == BR_IRET or what those abbreviations are.
> 
> I think it could also be possible to leave these entries as unknowns if we don't know
> if they're even being used. It will always be possible to go back and update these
> lbr mappings after you've added the new types.
> 
> 
> >  	PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
> >  	PERF_BR_UNKNOWN,	/* X86_BR_ABORT */
> >  	PERF_BR_UNKNOWN,	/* X86_BR_IN_TX */
> > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> > index 82858b697c05..d37629dbad72 100644
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -251,6 +251,8 @@ enum {
> >  	PERF_BR_SYSRET		= 8,	/* syscall return */
> >  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
> >  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> > +	PERF_BR_ERET		= 11,	/* exception return */
> > +	PERF_BR_IRQ		= 12,	/* irq */
> >  	PERF_BR_MAX,>  };
> >  
> > diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> > index 82858b697c05..d37629dbad72 100644
> > --- a/tools/include/uapi/linux/perf_event.h
> > +++ b/tools/include/uapi/linux/perf_event.h
> > @@ -251,6 +251,8 @@ enum {
> >  	PERF_BR_SYSRET		= 8,	/* syscall return */
> >  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
> >  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> > +	PERF_BR_ERET		= 11,	/* exception return */
> > +	PERF_BR_IRQ		= 12,	/* irq */
> >  	PERF_BR_MAX,
> >  };
> >  
> > diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
> > index 2285b1eb3128..a9a909db8cc7 100644
> > --- a/tools/perf/util/branch.c
> > +++ b/tools/perf/util/branch.c
> > @@ -49,7 +49,9 @@ const char *branch_type_name(int type)
> >  		"SYSCALL",
> >  		"SYSRET",
> >  		"COND_CALL",
> > -		"COND_RET"
> > +		"COND_RET",
> > +		"ERET",
> > +		"IRQ"
> >  	};
> >
> Otherwise the new entries look good to me, so without the lbr changes:
> 
> Reviewed-by: James Clark <james.clark@arm.com>
> 
> If we're keeping the lbr mapping changes, then I will defer to someone else
> to review.
> 
> Thanks
> James
> 
> >  	if (type >= 0 && type < PERF_BR_MAX)

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf: Add irq and exception return branch types
@ 2022-03-05 19:04     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-03-05 19:04 UTC (permalink / raw)
  To: James Clark
  Cc: Anshuman Khandual, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Will Deacon, linux-arm-kernel, linux-perf-users

Em Mon, Feb 28, 2022 at 03:45:25PM +0000, James Clark escreveu:
> 
> 
> On 24/02/2022 05:36, Anshuman Khandual wrote:
> > This expands generic branch type classification by adding two more entries
> > there in i.e irq and exception return. Also updates the x86 implementation
> > to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
> > branch types reported to user space on x86 platform but it should not be a
> > problem. The possible scenarios and impacts are enumerated here.
> > 
> > --------------------------------------------------------------------------
> > | kernel | perf tool |                     Impact                        |
> > --------------------------------------------------------------------------
> > |   old  |    old    |  Works as before                                  |
> > --------------------------------------------------------------------------
> > |   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
> > --------------------------------------------------------------------------
> > |   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
> > --------------------------------------------------------------------------
> > |   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
> > --------------------------------------------------------------------------
> > 
> > When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
> > perf tool) the user space might throw up an warning complaining about some
> > unrecognized branch types being reported, but it is expected. PERF_BR_ERET
> > and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
> > platform.
> > 
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Will Deacon <will@kernel.org>
> > 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 <anshuman.khandual@arm.com>
> > ---
> > This applies on v5.17-rc5
> > 
> > These two new branch types expands generic branch type classification but
> > still leaves another three entries in 'type' field for later. Please refer
> > a previous discussion [1] for some further context.
> > 
> > [1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/
> > 
> >  arch/x86/events/intel/lbr.c           | 4 ++--
> >  include/uapi/linux/perf_event.h       | 2 ++

Please try to avoid lockstep development of kernel and tools/, submit
patches to the kernel maintainers for the kernel parts, and to the perf
tools maintainer in separate patches.

It is important that changes to the API are flagged, for instance via
tools/perf/check-headers.sh so that opportunity is given for the various
people involved in perf (u/k) development to see what is going on.

Thanks,

- Arnaldo

> >  tools/include/uapi/linux/perf_event.h | 2 ++
> >  tools/perf/util/branch.c              | 4 +++-
> >  4 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> > index 669c2be14784..fe1742c4ca49 100644
> > --- a/arch/x86/events/intel/lbr.c
> > +++ b/arch/x86/events/intel/lbr.c
> > @@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = {
> >  	PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
> >  	PERF_BR_SYSRET,		/* X86_BR_SYSRET */
> >  	PERF_BR_UNKNOWN,	/* X86_BR_INT */
> > -	PERF_BR_UNKNOWN,	/* X86_BR_IRET */
> > +	PERF_BR_ERET,		/* X86_BR_IRET */
> >  	PERF_BR_COND,		/* X86_BR_JCC */
> >  	PERF_BR_UNCOND,		/* X86_BR_JMP */
> > -	PERF_BR_UNKNOWN,	/* X86_BR_IRQ */
> > +	PERF_BR_IRQ,		/* X86_BR_IRQ */
> Hi Anshuman,
> 
> I couldn't verify if these changes to the lbr map matched up to the spec because
> I couldn't find the right section. I suppose there might need to be a comment
> about why BR_ERET == BR_IRET or what those abbreviations are.
> 
> I think it could also be possible to leave these entries as unknowns if we don't know
> if they're even being used. It will always be possible to go back and update these
> lbr mappings after you've added the new types.
> 
> 
> >  	PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
> >  	PERF_BR_UNKNOWN,	/* X86_BR_ABORT */
> >  	PERF_BR_UNKNOWN,	/* X86_BR_IN_TX */
> > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> > index 82858b697c05..d37629dbad72 100644
> > --- a/include/uapi/linux/perf_event.h
> > +++ b/include/uapi/linux/perf_event.h
> > @@ -251,6 +251,8 @@ enum {
> >  	PERF_BR_SYSRET		= 8,	/* syscall return */
> >  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
> >  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> > +	PERF_BR_ERET		= 11,	/* exception return */
> > +	PERF_BR_IRQ		= 12,	/* irq */
> >  	PERF_BR_MAX,>  };
> >  
> > diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> > index 82858b697c05..d37629dbad72 100644
> > --- a/tools/include/uapi/linux/perf_event.h
> > +++ b/tools/include/uapi/linux/perf_event.h
> > @@ -251,6 +251,8 @@ enum {
> >  	PERF_BR_SYSRET		= 8,	/* syscall return */
> >  	PERF_BR_COND_CALL	= 9,	/* conditional function call */
> >  	PERF_BR_COND_RET	= 10,	/* conditional function return */
> > +	PERF_BR_ERET		= 11,	/* exception return */
> > +	PERF_BR_IRQ		= 12,	/* irq */
> >  	PERF_BR_MAX,
> >  };
> >  
> > diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
> > index 2285b1eb3128..a9a909db8cc7 100644
> > --- a/tools/perf/util/branch.c
> > +++ b/tools/perf/util/branch.c
> > @@ -49,7 +49,9 @@ const char *branch_type_name(int type)
> >  		"SYSCALL",
> >  		"SYSRET",
> >  		"COND_CALL",
> > -		"COND_RET"
> > +		"COND_RET",
> > +		"ERET",
> > +		"IRQ"
> >  	};
> >
> Otherwise the new entries look good to me, so without the lbr changes:
> 
> Reviewed-by: James Clark <james.clark@arm.com>
> 
> If we're keeping the lbr mapping changes, then I will defer to someone else
> to review.
> 
> Thanks
> James
> 
> >  	if (type >= 0 && type < PERF_BR_MAX)

-- 

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf: Add irq and exception return branch types
  2022-03-05 19:04     ` Arnaldo Carvalho de Melo
@ 2022-03-08  3:20       ` Anshuman Khandual
  -1 siblings, 0 replies; 9+ messages in thread
From: Anshuman Khandual @ 2022-03-08  3:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, James Clark
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Will Deacon, linux-arm-kernel, linux-perf-users



On 3/6/22 00:34, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 28, 2022 at 03:45:25PM +0000, James Clark escreveu:
>>
>> On 24/02/2022 05:36, Anshuman Khandual wrote:
>>> This expands generic branch type classification by adding two more entries
>>> there in i.e irq and exception return. Also updates the x86 implementation
>>> to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
>>> branch types reported to user space on x86 platform but it should not be a
>>> problem. The possible scenarios and impacts are enumerated here.
>>>
>>> --------------------------------------------------------------------------
>>> | kernel | perf tool |                     Impact                        |
>>> --------------------------------------------------------------------------
>>> |   old  |    old    |  Works as before                                  |
>>> --------------------------------------------------------------------------
>>> |   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
>>> --------------------------------------------------------------------------
>>> |   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
>>> --------------------------------------------------------------------------
>>> |   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
>>> --------------------------------------------------------------------------
>>>
>>> When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
>>> perf tool) the user space might throw up an warning complaining about some
>>> unrecognized branch types being reported, but it is expected. PERF_BR_ERET
>>> and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
>>> platform.
>>>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>>> Cc: Jiri Olsa <jolsa@redhat.com>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Will Deacon <will@kernel.org>
>>> 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 <anshuman.khandual@arm.com>
>>> ---
>>> This applies on v5.17-rc5
>>>
>>> These two new branch types expands generic branch type classification but
>>> still leaves another three entries in 'type' field for later. Please refer
>>> a previous discussion [1] for some further context.
>>>
>>> [1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/
>>>
>>>  arch/x86/events/intel/lbr.c           | 4 ++--
>>>  include/uapi/linux/perf_event.h       | 2 ++
> Please try to avoid lockstep development of kernel and tools/, submit
> patches to the kernel maintainers for the kernel parts, and to the perf
> tools maintainer in separate patches.

Sure, will split this patch into two i.e kernel and user space changes. I have
an updated series which has some more kernel and user space API changes. But I
am wondering if there should be just a single patch updating user space API for
all the preceding kernel changes, or there should be one user space API patch
for each corresponding kernel change ?

> 
> It is important that changes to the API are flagged, for instance via
> tools/perf/check-headers.sh so that opportunity is given for the various
> people involved in perf (u/k) development to see what is going on.

Will run the diff after the series has been applied to demonstrate all the API
changes and update that in the cover letter.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf: Add irq and exception return branch types
@ 2022-03-08  3:20       ` Anshuman Khandual
  0 siblings, 0 replies; 9+ messages in thread
From: Anshuman Khandual @ 2022-03-08  3:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, James Clark
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Will Deacon, linux-arm-kernel, linux-perf-users



On 3/6/22 00:34, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 28, 2022 at 03:45:25PM +0000, James Clark escreveu:
>>
>> On 24/02/2022 05:36, Anshuman Khandual wrote:
>>> This expands generic branch type classification by adding two more entries
>>> there in i.e irq and exception return. Also updates the x86 implementation
>>> to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes
>>> branch types reported to user space on x86 platform but it should not be a
>>> problem. The possible scenarios and impacts are enumerated here.
>>>
>>> --------------------------------------------------------------------------
>>> | kernel | perf tool |                     Impact                        |
>>> --------------------------------------------------------------------------
>>> |   old  |    old    |  Works as before                                  |
>>> --------------------------------------------------------------------------
>>> |   old  |    new    |  PERF_BR_UNKNOWN is processed                     |
>>> --------------------------------------------------------------------------
>>> |   new  |    old    |  PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX |
>>> --------------------------------------------------------------------------
>>> |   new  |    new    |  PERF_BR_ERET/IRQ are recognized                  |
>>> --------------------------------------------------------------------------
>>>
>>> When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old
>>> perf tool) the user space might throw up an warning complaining about some
>>> unrecognized branch types being reported, but it is expected. PERF_BR_ERET
>>> and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64
>>> platform.
>>>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>>> Cc: Jiri Olsa <jolsa@redhat.com>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Will Deacon <will@kernel.org>
>>> 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 <anshuman.khandual@arm.com>
>>> ---
>>> This applies on v5.17-rc5
>>>
>>> These two new branch types expands generic branch type classification but
>>> still leaves another three entries in 'type' field for later. Please refer
>>> a previous discussion [1] for some further context.
>>>
>>> [1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/
>>>
>>>  arch/x86/events/intel/lbr.c           | 4 ++--
>>>  include/uapi/linux/perf_event.h       | 2 ++
> Please try to avoid lockstep development of kernel and tools/, submit
> patches to the kernel maintainers for the kernel parts, and to the perf
> tools maintainer in separate patches.

Sure, will split this patch into two i.e kernel and user space changes. I have
an updated series which has some more kernel and user space API changes. But I
am wondering if there should be just a single patch updating user space API for
all the preceding kernel changes, or there should be one user space API patch
for each corresponding kernel change ?

> 
> It is important that changes to the API are flagged, for instance via
> tools/perf/check-headers.sh so that opportunity is given for the various
> people involved in perf (u/k) development to see what is going on.

Will run the diff after the series has been applied to demonstrate all the API
changes and update that in the cover letter.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-03-08  3:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24  5:36 [PATCH] perf: Add irq and exception return branch types Anshuman Khandual
2022-02-24  5:36 ` Anshuman Khandual
2022-02-28 15:45 ` James Clark
2022-02-28 15:45   ` James Clark
2022-03-05 19:04   ` Arnaldo Carvalho de Melo
2022-03-05 19:04     ` Arnaldo Carvalho de Melo
2022-03-08  3:20     ` Anshuman Khandual
2022-03-08  3:20       ` Anshuman Khandual
2022-03-01 15:24 ` [tip: perf/core] " tip-bot2 for Anshuman Khandual

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.