2023-07-11 08:41:06

by Anshuman Khandual

[permalink] [raw]
Subject: [PATCH V13 - RESEND 01/10] drivers: perf: arm_pmu: Add new sched_task() callback

This adds armpmu_sched_task(), as generic pmu's sched_task() override which
in turn can utilize a new arm_pmu.sched_task() callback when available from
the arm_pmu instance. This new callback will be used while enabling BRBE in
ARMV8 PMU.

Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: [email protected]
Cc: [email protected]
Tested-by: James Clark <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Signed-off-by: Anshuman Khandual <[email protected]>
---
drivers/perf/arm_pmu.c | 9 +++++++++
include/linux/perf/arm_pmu.h | 1 +
2 files changed, 10 insertions(+)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index f6ccb2cd4dfc..c0475a96c2a0 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -519,6 +519,14 @@ static int armpmu_event_init(struct perf_event *event)
return __hw_perf_event_init(event);
}

+static void armpmu_sched_task(struct perf_event_pmu_context *pmu_ctx, bool sched_in)
+{
+ struct arm_pmu *armpmu = to_arm_pmu(pmu_ctx->pmu);
+
+ if (armpmu->sched_task)
+ armpmu->sched_task(pmu_ctx, sched_in);
+}
+
static void armpmu_enable(struct pmu *pmu)
{
struct arm_pmu *armpmu = to_arm_pmu(pmu);
@@ -865,6 +873,7 @@ struct arm_pmu *armpmu_alloc(void)
}

pmu->pmu = (struct pmu) {
+ .sched_task = armpmu_sched_task,
.pmu_enable = armpmu_enable,
.pmu_disable = armpmu_disable,
.event_init = armpmu_event_init,
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index a0801f68762b..3d3bd944d630 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -102,6 +102,7 @@ struct arm_pmu {
void (*stop)(struct arm_pmu *);
void (*reset)(void *);
int (*map_event)(struct perf_event *event);
+ void (*sched_task)(struct perf_event_pmu_context *pmu_ctx, bool sched_in);
int num_events;
bool secure_access; /* 32-bit ARM only */
#define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40
--
2.25.1



2023-08-10 05:28:48

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH V13 - RESEND 01/10] drivers: perf: arm_pmu: Add new sched_task() callback

Hello Will,

I am just wondering - would it be possible for you to take this pre-requisite
patch stand alone for the upcoming merge window. This has been acked by Mark
earlier. Besides, I am also working on your other suggestions on the series,
and will respond soon. Thank you.

- Anshuman

On 7/11/23 13:54, Anshuman Khandual wrote:
> This adds armpmu_sched_task(), as generic pmu's sched_task() override which
> in turn can utilize a new arm_pmu.sched_task() callback when available from
> the arm_pmu instance. This new callback will be used while enabling BRBE in
> ARMV8 PMU.
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Tested-by: James Clark <[email protected]>
> Acked-by: Mark Rutland <[email protected]>
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> drivers/perf/arm_pmu.c | 9 +++++++++
> include/linux/perf/arm_pmu.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index f6ccb2cd4dfc..c0475a96c2a0 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -519,6 +519,14 @@ static int armpmu_event_init(struct perf_event *event)
> return __hw_perf_event_init(event);
> }
>
> +static void armpmu_sched_task(struct perf_event_pmu_context *pmu_ctx, bool sched_in)
> +{
> + struct arm_pmu *armpmu = to_arm_pmu(pmu_ctx->pmu);
> +
> + if (armpmu->sched_task)
> + armpmu->sched_task(pmu_ctx, sched_in);
> +}
> +
> static void armpmu_enable(struct pmu *pmu)
> {
> struct arm_pmu *armpmu = to_arm_pmu(pmu);
> @@ -865,6 +873,7 @@ struct arm_pmu *armpmu_alloc(void)
> }
>
> pmu->pmu = (struct pmu) {
> + .sched_task = armpmu_sched_task,
> .pmu_enable = armpmu_enable,
> .pmu_disable = armpmu_disable,
> .event_init = armpmu_event_init,
> diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
> index a0801f68762b..3d3bd944d630 100644
> --- a/include/linux/perf/arm_pmu.h
> +++ b/include/linux/perf/arm_pmu.h
> @@ -102,6 +102,7 @@ struct arm_pmu {
> void (*stop)(struct arm_pmu *);
> void (*reset)(void *);
> int (*map_event)(struct perf_event *event);
> + void (*sched_task)(struct perf_event_pmu_context *pmu_ctx, bool sched_in);
> int num_events;
> bool secure_access; /* 32-bit ARM only */
> #define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40

2023-08-10 10:13:08

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH V13 - RESEND 01/10] drivers: perf: arm_pmu: Add new sched_task() callback

On Thu, Aug 10, 2023 at 10:35:42AM +0530, Anshuman Khandual wrote:
> I am just wondering - would it be possible for you to take this pre-requisite
> patch stand alone for the upcoming merge window. This has been acked by Mark
> earlier. Besides, I am also working on your other suggestions on the series,
> and will respond soon. Thank you.

I can if it helps in some way, but I'm not seeing how it does. Can't you
just carry this along with the BRBE changes that use it? How does it benefit
anybody on its own?

Will

2023-08-10 12:35:00

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH V13 - RESEND 01/10] drivers: perf: arm_pmu: Add new sched_task() callback



On 8/10/23 15:11, Will Deacon wrote:
> On Thu, Aug 10, 2023 at 10:35:42AM +0530, Anshuman Khandual wrote:
>> I am just wondering - would it be possible for you to take this pre-requisite
>> patch stand alone for the upcoming merge window. This has been acked by Mark
>> earlier. Besides, I am also working on your other suggestions on the series,
>> and will respond soon. Thank you.
>
> I can if it helps in some way, but I'm not seeing how it does. Can't you
> just carry this along with the BRBE changes that use it? How does it benefit
> anybody on its own?
It might help de-clutter our conversation and focus on BRBE specific
implementation stuff next time around in V14. Also this change here
is quite harmless for any other thing, hence without much risk.