perf/core: Flush PMU internal buffers for per-CPU events
Subsystems (3)
PATCH v2 1/3 perf/core: Flush PMU internal buffers for per-CPU events
From: Kan Liang <[email protected]>
Sometimes the PMU internal buffers have to be flushed for per-CPU events during a context switch, e.g., large PEBS. Otherwise, the perf tool may report samples in locations that do not belong to the process where the samples are processed in, because PEBS does not tag samples with PID/TID.
The current code only flush the buffers for a per-task event. It doesn't check a per-CPU event.
Add a new event state flag, PERF_ATTACH_SCHED_CB, to indicate that the PMU internal buffers have to be flushed for this event during a context switch.
Add sched_cb_entry and perf_sched_cb_usages back to track the PMU/cpuctx which is required to be flushed.
Only need to invoke the sched_task() for per-CPU events in this patch. The per-task events have been handled in perf_event_context_sched_in/out already.
---
No changes since V1
include/linux/perf_event.h | 2 ++ kernel/events/core.c | 42 ++++++++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-)
include/linux/perf_event.h +2 -0
@@ -606,6 +606,7 @@ struct swevent_hlist {#define PERF_ATTACH_TASK 0x04#define PERF_ATTACH_TASK_DATA 0x08#define PERF_ATTACH_ITRACE 0x10 +#define PERF_ATTACH_SCHED_CB 0x20struct perf_cgroup ;struct perf_buffer ;
@@ -872,6 +873,7 @@ struct perf_cpu_context {struct list_head cgrp_cpuctx_entry;#endif +struct list_head sched_cb_entry;int sched_cb_usage;int online;
kernel/events/core.c +38 -4
100 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v2 2/3 perf/x86/intel: Set PERF_ATTACH_SCHED_CB for large PEBS and LBR
From: Kan Liang <[email protected]>
To supply a PID/TID for large PEBS, it requires flushing the PEBS buffer in a context switch.
For normal LBRs, a context switch can flip the address space and LBR entries are not tagged with an identifier, we need to wipe the LBR, even for per-cpu events.
For LBR callstack, save/restore the stack is required during a context switch.
Set PERF_ATTACH_SCHED_CB for the event with large PEBS & LBR.
---
Changes since V1
- Set PERF_ATTACH_SCHED_CB for a LBR event as well
arch/x86/events/intel/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
arch/x86/events/intel/core.c +4 -1
@@ -3564,8 +3564,10 @@ static int intel_pmu_hw_config(struct perf_event *event)if (!(event->attr.freq || (event->attr.wakeup_events && !event->attr.watermark))) { event->hw.flags |=PERF_X86_EVENT_AUTO_RELOAD ;if (!(event->attr.sample_type & - ~intel_pmu_large_pebs_flags (event))) + ~intel_pmu_large_pebs_flags (event))) { event->hw.flags |=PERF_X86_EVENT_LARGE_PEBS ; + event->attach_state |=PERF_ATTACH_SCHED_CB ; + } }if (x86_pmu.pebs_aliases) x86_pmu.pebs_aliases (event);
@@ -3578,6 +3580,7 @@ static int intel_pmu_hw_config(struct perf_event *event) ret =intel_pmu_setup_lbr_filter (event);if (ret)return ret; + event->attach_state |=PERF_ATTACH_SCHED_CB ;/* * BTS is set up earlier in this path, so don't account twice
Signature · 1 line
2.17.1
[tip: perf/urgent] perf/x86/intel: Set PERF_ATTACH_SCHED_CB for large PEBS and LBR
The following commit has been merged into the perf/urgent branch of tip:
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: a8abc881981762631a22568d5e4b2c0ce4aeb15c Gitweb: https://git.kernel.org/tip/a8abc881981762631a22568d5e4b2c0ce4aeb15c Author: Kan Liang <[email protected]> AuthorDate: Mon, 30 Nov 2020 11:38:41 -08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Mon, 01 Mar 2021 11:02:19 +01:00
perf/x86/intel: Set PERF_ATTACH_SCHED_CB for large PEBS and LBR
To supply a PID/TID for large PEBS, it requires flushing the PEBS buffer in a context switch.
For normal LBRs, a context switch can flip the address space and LBR entries are not tagged with an identifier, we need to wipe the LBR, even for per-cpu events.
For LBR callstack, save/restore the stack is required during a context switch.
Set PERF_ATTACH_SCHED_CB for the event with large PEBS & LBR.
--- arch/x86/events/intel/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
arch/x86/events/intel/core.c +4 -1
@@ -3662,8 +3662,10 @@ static int intel_pmu_hw_config(struct perf_event *event)if (!(event->attr.freq || (event->attr.wakeup_events && !event->attr.watermark))) { event->hw.flags |=PERF_X86_EVENT_AUTO_RELOAD ;if (!(event->attr.sample_type & - ~intel_pmu_large_pebs_flags (event))) + ~intel_pmu_large_pebs_flags (event))) { event->hw.flags |=PERF_X86_EVENT_LARGE_PEBS ; + event->attach_state |=PERF_ATTACH_SCHED_CB ; + } }if (x86_pmu.pebs_aliases) x86_pmu.pebs_aliases (event);
@@ -3676,6 +3678,7 @@ static int intel_pmu_hw_config(struct perf_event *event) ret =intel_pmu_setup_lbr_filter (event);if (ret)return ret; + event->attach_state |=PERF_ATTACH_SCHED_CB ;/* * BTS is set up earlier in this path, so don't account twice
[tip: perf/urgent] perf/x86/intel: Set PERF_ATTACH_SCHED_CB for large PEBS and LBR
In reply to [email protected]The following commit has been merged into the perf/urgent branch of tip:
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: afbef30149587ad46f4780b1e0cc5e219745ce90 Gitweb: https://git.kernel.org/tip/afbef30149587ad46f4780b1e0cc5e219745ce90 Author: Kan Liang <[email protected]> AuthorDate: Mon, 30 Nov 2020 11:38:41 -08:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Sat, 06 Mar 2021 12:52:44 +01:00
perf/x86/intel: Set PERF_ATTACH_SCHED_CB for large PEBS and LBR
To supply a PID/TID for large PEBS, it requires flushing the PEBS buffer in a context switch.
For normal LBRs, a context switch can flip the address space and LBR entries are not tagged with an identifier, we need to wipe the LBR, even for per-cpu events.
For LBR callstack, save/restore the stack is required during a context switch.
Set PERF_ATTACH_SCHED_CB for the event with large PEBS & LBR.
--- arch/x86/events/intel/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
arch/x86/events/intel/core.c +4 -1
@@ -3662,8 +3662,10 @@ static int intel_pmu_hw_config(struct perf_event *event)if (!(event->attr.freq || (event->attr.wakeup_events && !event->attr.watermark))) { event->hw.flags |=PERF_X86_EVENT_AUTO_RELOAD ;if (!(event->attr.sample_type & - ~intel_pmu_large_pebs_flags (event))) + ~intel_pmu_large_pebs_flags (event))) { event->hw.flags |=PERF_X86_EVENT_LARGE_PEBS ; + event->attach_state |=PERF_ATTACH_SCHED_CB ; + } }if (x86_pmu.pebs_aliases) x86_pmu.pebs_aliases (event);
@@ -3676,6 +3678,7 @@ static int intel_pmu_hw_config(struct perf_event *event) ret =intel_pmu_setup_lbr_filter (event);if (ret)return ret; + event->attach_state |=PERF_ATTACH_SCHED_CB ;/* * BTS is set up earlier in this path, so don't account twice
PATCH v2 3/3 perf: Optimize sched_task() in a context switch
In reply to [email protected]From: Kan Liang <[email protected]>
Some calls to sched_task() in a context switch can be avoided. For example, large PEBS only requires flushing the buffer in context switch out. The current code still invokes the sched_task() for large PEBS in context switch in.
The current code doesn't know and check the states of an event. It has no idea which calls is unnecessary.
Add a new variable, sched_cb_state, to indicate the states of an event. Add sched_cb_state to track the states per CPU context. Only invokes the sched_task() when the relative states are set.
Split sched_cb_usage into sched_cb_task_usage and sched_cb_cpu_usage to track per-task and per-CPU events respectively. Avoid going through the sched_cb_list for pure per-task events. Avoid invoking sched_task() in perf_event_context_sched_in/out for pure perf-CPU events.
Since the interface functions are changed, the specific codes are also changed here.
- For power, the BHRB entries are reset in context switch in.
- For X86 large PEBS, the PEBS buffer are flushed in context switch out.
- For X86 normal LBR, the LBR registers are reset in context switch in.
- For X86 LBR call stack, the LBR registers are saved in context switch out, and restored in context switch in.
---
Changes since V1:
- Fix the missed changes in the power_pmu_bhrb_disable()
This patch still not split the ppc change to a separate patch, because both PPC and X86 invokes the perf_sched_cb_inc() directly. The patch changes the parameters of the perf_sched_cb_inc(). We have to update the PPC and X86 codes together.
arch/powerpc/perf/core-book3s.c | 14 ++++++-- arch/x86/events/intel/ds.c | 8 +++-- arch/x86/events/intel/lbr.c | 15 ++++++-- include/linux/perf_event.h | 12 +++++-- kernel/events/core.c | 62 +++++++++++++++++++++++++++------ 5 files changed, 92 insertions(+), 19 deletions(-)
arch/powerpc/perf/core-book3s.c +12 -2
@@ -370,6 +370,7 @@ static void power_pmu_bhrb_reset(void)static void power_pmu_bhrb_enable (struct perf_event *event) {struct cpu_hw_events *cpuhw =this_cpu_ptr (&cpu_hw_events); +int state =PERF_SCHED_CB_SW_IN ;if (!ppmu->bhrb_nr)return ;
@@ -380,19 +381,28 @@ static void power_pmu_bhrb_enable(struct perf_event *event) cpuhw->bhrb_context = event->ctx; } cpuhw->bhrb_users++; -perf_sched_cb_inc (event->ctx->pmu); + +if (!(event->attach_state &PERF_ATTACH_TASK )) + state |=PERF_SCHED_CB_CPU ; + +perf_sched_cb_inc (event->ctx->pmu, state); }static void power_pmu_bhrb_disable (struct perf_event *event) {struct cpu_hw_events *cpuhw =this_cpu_ptr (&cpu_hw_events); +int state =PERF_SCHED_CB_SW_IN ;if (!ppmu->bhrb_nr)return ;WARN_ON_ONCE (!cpuhw->bhrb_users); cpuhw->bhrb_users--; -perf_sched_cb_dec (event->ctx->pmu); + +if (!(event->attach_state &PERF_ATTACH_TASK )) + state |=PERF_SCHED_CB_CPU ; + +perf_sched_cb_dec (event->ctx->pmu, state);if (!cpuhw->disabled && !cpuhw->bhrb_users) {/* BHRB cannot be turned off when other
arch/x86/events/intel/ds.c +6 -2
@@ -1018,12 +1018,16 @@ pebs_update_state(bool needed_cb, struct cpu_hw_events *cpuc,* that does not hurt: */ bool update = cpuc->n_pebs ==1 ; +int state =PERF_SCHED_CB_SW_OUT ;if (needed_cb !=pebs_needs_sched_cb (cpuc)) { +if (!(event->attach_state &PERF_ATTACH_TASK )) + state |=PERF_SCHED_CB_CPU ; +if (!needed_cb) -perf_sched_cb_inc (pmu); +perf_sched_cb_inc (pmu, state);else -perf_sched_cb_dec (pmu); +perf_sched_cb_dec (pmu, state); update = true; }
arch/x86/events/intel/lbr.c +13 -2
@@ -660,6 +660,7 @@ void intel_pmu_lbr_add(struct perf_event *event) {struct kmem_cache *kmem_cache = event->pmu->task_ctx_cache;struct cpu_hw_events *cpuc =this_cpu_ptr (&cpu_hw_events); +int state =PERF_SCHED_CB_SW_IN ;if (!x86_pmu.lbr_nr)return ;
@@ -693,7 +694,13 @@ void intel_pmu_lbr_add(struct perf_event *event)*/ if (x86_pmu.intel_cap.pebs_baseline && event->attr.precise_ip >0 ) cpuc->lbr_pebs_users++; -perf_sched_cb_inc (event->ctx->pmu); + +if (!(event->attach_state &PERF_ATTACH_TASK )) + state |=PERF_SCHED_CB_CPU ; +if (event->attach_state &PERF_ATTACH_TASK_DATA ) + state |=PERF_SCHED_CB_SW_OUT ; + +perf_sched_cb_inc (event->ctx->pmu, state);if (!cpuc->lbr_users++ && !event->total_time_running)intel_pmu_lbr_reset ();
@@ -724,6 +731,7 @@ void release_lbr_buffers(void)void intel_pmu_lbr_del (struct perf_event *event) {struct cpu_hw_events *cpuc =this_cpu_ptr (&cpu_hw_events); +int state =0 ;if (!x86_pmu.lbr_nr)return ;
@@ -740,7 +748,10 @@ void intel_pmu_lbr_del(struct perf_event *event) cpuc->lbr_users--;WARN_ON_ONCE (cpuc->lbr_users <0 );WARN_ON_ONCE (cpuc->lbr_pebs_users <0 ); -perf_sched_cb_dec (event->ctx->pmu); + +if (!(event->attach_state &PERF_ATTACH_TASK )) + state |=PERF_SCHED_CB_CPU ; +perf_sched_cb_dec (event->ctx->pmu, state); }static inline bool vlbr_exclude_host (void )
include/linux/perf_event.h +9 -3
@@ -854,6 +854,10 @@ struct perf_event_context {*/ #define PERF_NR_CONTEXTS 4 +#define PERF_SCHED_CB_CPU 0x1 +#define PERF_SCHED_CB_SW_IN 0x2 +#define PERF_SCHED_CB_SW_OUT 0x4 +/** * struct perf_event_cpu_context - per cpu event context structure */
@@ -874,7 +878,9 @@ struct perf_cpu_context {#endif struct list_head sched_cb_entry; -int sched_cb_usage; +int sched_cb_task_usage; +int sched_cb_cpu_usage; +int sched_cb_state;int online;/*
@@ -967,8 +973,8 @@ extern const struct perf_event_attr *perf_event_attrs(struct perf_event *event);extern void perf_event_print_debug (void );extern void perf_pmu_disable (struct pmu *pmu);extern void perf_pmu_enable (struct pmu *pmu); -extern void perf_sched_cb_dec (struct pmu *pmu); -extern void perf_sched_cb_inc (struct pmu *pmu); +extern void perf_sched_cb_dec (struct pmu *pmu,int state); +extern void perf_sched_cb_inc (struct pmu *pmu,int state);extern int perf_event_task_disable (void );extern int perf_event_task_enable (void );
kernel/events/core.c +52 -10
116 lines not on this page · raw message
Signature · 1 line
2.17.1
Peter Zijlstra
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
Quoted 6 lines · On Mon, Nov 30, 2020 at 11:38:42AM -0800, [email protected] wrote:
From: Kan Liang <[email protected]>
Some calls to
sched_task()in a context switch can be avoided. For example, large PEBS only requires flushing the buffer in context switch out. The current code still invokes thesched_task()for large PEBS in context switch in.
I still hate this one, how's something like this then? Which I still don't really like.. but at least its simpler.
(completely untested, may contain spurious edits, might ICE the compiler and set your pets on fire if it doesn't)
And given this is an optimization, can we actually measure it to improve matters?
---
arch/powerpc/perf/core-book3s.c +2 -2
@@ -380,7 +380,7 @@ static void power_pmu_bhrb_enable(struct cpuhw->bhrb_context = event->ctx; } cpuhw->bhrb_users++; -perf_sched_cb_inc (event->ctx->pmu); +perf_sched_cb_inc (event); }static void power_pmu_bhrb_disable (struct perf_event *event)
@@ -392,7 +392,7 @@ static void power_pmu_bhrb_disable(strucWARN_ON_ONCE (!cpuhw->bhrb_users); cpuhw->bhrb_users--; -perf_sched_cb_dec (event->ctx->pmu); +perf_sched_cb_dec (event);if (!cpuhw->disabled && !cpuhw->bhrb_users) {/* BHRB cannot be turned off when other
arch/x86/events/intel/core.c +1 -1
@@ -3567,7 +3567,7 @@ static int intel_pmu_hw_config(struct peif (!(event->attr.sample_type & ~intel_pmu_large_pebs_flags (event))) { event->hw.flags |=PERF_X86_EVENT_LARGE_PEBS ; - event->attach_state |=PERF_ATTACH_SCHED_CB ; + event->attach_state |=PERF_ATTACH_SCHED_CB_OUT ; } }if (x86_pmu.pebs_aliases)
arch/x86/events/intel/ds.c +2 -3
@@ -1014,7 +1014,6 @@ static void pebs_update_state(bool needed_cb, struct cpu_hw_events *cpuc, struct perf_event *event, bool add) { - struct pmu *pmu = event->ctx->pmu;/* * Make sure we get updated with the first PEBS * event. It will trigger also during removal, but
@@ -1024,9 +1023,9 @@ pebs_update_state(bool needed_cb, structif (needed_cb !=pebs_needs_sched_cb (cpuc)) {if (!needed_cb) -perf_sched_cb_inc (pmu); +perf_sched_cb_inc (event);else -perf_sched_cb_dec (pmu); +perf_sched_cb_dec (event); update = true; }
arch/x86/events/intel/lbr.c +2 -2
@@ -693,7 +693,7 @@ void intel_pmu_lbr_add(struct perf_event*/ if (x86_pmu.intel_cap.pebs_baseline && event->attr.precise_ip >0 ) cpuc->lbr_pebs_users++; -perf_sched_cb_inc (event->ctx->pmu); +perf_sched_cb_inc (event);if (!cpuc->lbr_users++ && !event->total_time_running)intel_pmu_lbr_reset ();
@@ -740,7 +740,7 @@ void intel_pmu_lbr_del(struct perf_event cpuc->lbr_users--;WARN_ON_ONCE (cpuc->lbr_users <0 );WARN_ON_ONCE (cpuc->lbr_pebs_users <0 ); -perf_sched_cb_dec (event->ctx->pmu); +perf_sched_cb_dec (event); }static inline bool vlbr_exclude_host (void )
include/linux/perf_event.h +11 -8
@@ -601,12 +601,14 @@ struct swevent_hlist {struct rcu_head rcu_head; }; -#define PERF_ATTACH_CONTEXT 0x01 -#define PERF_ATTACH_GROUP 0x02 -#define PERF_ATTACH_TASK 0x04 -#define PERF_ATTACH_TASK_DATA 0x08 -#define PERF_ATTACH_ITRACE 0x10 -#define PERF_ATTACH_SCHED_CB 0x20 +#define PERF_ATTACH_CONTEXT 0x01 +#define PERF_ATTACH_GROUP 0x02 +#define PERF_ATTACH_TASK 0x04 +#define PERF_ATTACH_TASK_DATA 0x08 +#define PERF_ATTACH_ITRACE 0x10 +#define PERF_ATTACH_SCHED_CB_IN 0x20 +#define PERF_ATTACH_SCHED_CB_OUT 0x40 +#define PERF_ATTACH_SCHED_CB 0x60struct perf_cgroup ;struct perf_buffer ;
@@ -875,6 +877,7 @@ struct perf_cpu_context {struct list_head sched_cb_entry;int sched_cb_usage; +int sched_cb_dir[2 ];int online;/*
@@ -967,8 +970,8 @@ extern const struct perf_event_attr *perextern void perf_event_print_debug (void );extern void perf_pmu_disable (struct pmu *pmu);extern void perf_pmu_enable (struct pmu *pmu); -extern void perf_sched_cb_dec (struct pmu *pmu); -extern void perf_sched_cb_inc (struct pmu *pmu); +extern void perf_sched_cb_dec (struct perf_event *event); +extern void perf_sched_cb_inc (struct perf_event *event);extern int perf_event_task_disable (void );extern int perf_event_task_enable (void );
kernel/events/core.c +38 -22
126 lines not on this page · raw message
Peter Zijlstra
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
kernel/events/core.c static void perf_event_sync_stat(struct · line 3386
Quoted 9 lines · Peter Zijlstra
kernel/events/core.c +9
-0+static void context_sched_task (struct perf_cpu_context *cpuctx, +struct perf_event_context *ctx, +bool sched_in) +{ +struct pmu *pmu = ctx->pmu; + +if (cpuctx->sched_cb_dir[sched_in] && pmu->sched_task) + pmu->sched_task (ctx, false); +}
s/false/sched_in/
Namhyung Kim
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
In reply to Peter ZijlstraHi Peter and Kan,
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines · On Mon, Nov 30, 2020 at 11:38:42AM -0800, [email protected] wrote:
From: Kan Liang <[email protected]>
Some calls to
sched_task()in a context switch can be avoided. For example, large PEBS only requires flushing the buffer in context switch out. The current code still invokes thesched_task()for large PEBS in context switch in.
Quoted 5 lines
I still hate this one, how's something like this then? Which I still don't really like.. but at least its simpler.
(completely untested, may contain spurious edits, might ICE the compiler and set your pets on fire if it doesn't)
I've tested Kan's v2 patches and it worked well. Will test your version (with the fix in the other email) too.
Quoted 3 lines
And given this is an optimization, can we actually measure it to improve matters?
I just checked perf bench sched pipe result. Without perf record running, it usually takes less than 7 seconds. Note that this (and below) is a median value of 10 runs.
# perf bench sched pipe # Running 'sched/pipe' benchmark: # Executed 1000000 pipe operations between two processes
Total time: 6.875 [sec]
6.875700 usecs/op
145439 ops/secAnd I ran it again with perf record like below. This is a result when I applied the patch 1 and 2 only.
# perf record -aB -c 100001 -e cycles:pp perf bench sched pipe # Running 'sched/pipe' benchmark: # Executed 1000000 pipe operations between two processes
Total time: 8.198 [sec]
8.198952 usecs/op
121966 ops/sec[ perf record: Woken up 10 times to write data ] [ perf record: Captured and wrote 4.972 MB perf.data ]
With patch 3 applied, the total time went down a little bit.
# perf record -aB -c 100001 -e cycles:pp perf bench sched pipe # Running 'sched/pipe' benchmark: # Executed 1000000 pipe operations between two processes
Total time: 7.785 [sec]
7.785119 usecs/op
128450 ops/sec[ perf record: Woken up 12 times to write data ] [ perf record: Captured and wrote 4.622 MB perf.data ]
Thanks,
Namhyung
Re: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
In reply to Peter ZijlstraHi Peter,
Quoted 9 lines · Peter Zijlstra
Quoted 6 lines · On Mon, Nov 30, 2020 at 11:38:42AM -0800, [email protected] wrote:
From: Kan Liang <[email protected]>
Some calls to
sched_task()in a context switch can be avoided. For example, large PEBS only requires flushing the buffer in context switch out. The current code still invokes thesched_task()for large PEBS in context switch in.
Quoted 5 lines
I still hate this one, how's something like this then? Which I still don't really like.. but at least its simpler.
(completely untested, may contain spurious edits, might ICE the compiler and set your pets on fire if it doesn't)
I've tested this version... and it worked well besides the optimization.. :)
[SNIP]
Review
Quoted 8 lines
(quoted diff) +8
-0+static void context_sched_task(struct perf_cpu_context *cpuctx, + struct perf_event_context *ctx, + bool sched_in) +{ + struct pmu *pmu = ctx->pmu; + + if (cpuctx->sched_cb_dir[sched_in] && pmu->sched_task) + pmu->sched_task(ctx, false);
applied: s/false/sched_in/
Quoted 26 lines
(quoted diff) +4
-5+} + static void perf_event_context_sched_out(struct task_struct *task, int ctxn, struct task_struct *next) {@@ -3424,9 +3433,7 @@ static void perf_event_context_sched_out WRITE_ONCE(next_ctx->task, task); perf_pmu_disable(pmu); - - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(ctx, false); + context_sched_task(cpuctx, ctx, false); /* * PMU specific parts of task perf context can require@@ -3465,8 +3472,7 @@ static void perf_event_context_sched_out raw_spin_lock(&ctx->lock); perf_pmu_disable(pmu); - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(ctx, false); + context_sched_task(cpuctx, ctx, false); task_ctx_sched_out(cpuctx, ctx, EVENT_ALL); perf_pmu_enable(pmu);
[SNIP]
Review void __perf_event_task_sched_out(struct · line 3585
Quoted 7 lines
(quoted diff) +1
-2@@ -3563,8 +3582,7 @@ void __perf_event_task_sched_out(struct { int ctxn; - if (__this_cpu_read(perf_sched_cb_usage)) - perf_pmu_sched_task(task, next, false); + perf_pmu_sched_task(task, next, false);
I think the reason is this change. It now calls perf_pmu_sched_task() without checking the counter. And this is for per-cpu events.
Quoted 14 lines
(quoted diff) +1
-2if (atomic_read(&nr_switch_events)) perf_event_switch(task, next, false);@@ -3828,8 +3846,7 @@ static void perf_event_context_sched_in( cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE); perf_event_sched_in(cpuctx, ctx, task); - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(cpuctx->task_ctx, true); + context_sched_task(cpuctx, ctx, true); perf_pmu_enable(pmu);
Review void __perf_event_task_sched_in(struct t · line 3895
Quoted 6 lines
(quoted diff) +1
-2@@ -3875,8 +3892,7 @@ void __perf_event_task_sched_in(struct t if (atomic_read(&nr_switch_events)) perf_event_switch(task, prev, true); - if (__this_cpu_read(perf_sched_cb_usage)) - perf_pmu_sched_task(prev, task, true); + perf_pmu_sched_task(prev, task, true);
Ditto.
Review void __perf_event_task_sched_in(struct t · line 3898
Quoted 3 lines
(quoted diff) +0
-0} static u64 perf_calculate_period(struct perf_event *event, u64 nsec, u64 count)
So I made a change like below.. and it could bring the optimization back.
Thanks,
Namhyung
kernel/events/core.c +0 -0
@@ -3528,6 +3528,9 @@ static void __perf_pmu_sched_task(struct
perf_cpu_context *cpuctx, bool sched_in
{
struct pmu *pmu;+ if (!cpuctx->sched_cb_dir[sched_in])
+ return;
+
pmu = cpuctx->ctx.pmu; /* software PMUs will not have sched_task */
if (WARN_ON_ONCE(!pmu->sched_task))Namhyung Kim
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
Hi Peter and Kan,
How can we move this forward?
Thanks,
Namhyung
Quoted 121 lines · Namhyung Kim
Hi Peter,
Quoted 9 lines · Peter Zijlstra
Quoted 6 lines · On Mon, Nov 30, 2020 at 11:38:42AM -0800, [email protected] wrote:
From: Kan Liang <[email protected]>
Some calls to
sched_task()in a context switch can be avoided. For example, large PEBS only requires flushing the buffer in context switch out. The current code still invokes thesched_task()for large PEBS in context switch in.Quoted 5 lines
I still hate this one, how's something like this then? Which I still don't really like.. but at least its simpler.
(completely untested, may contain spurious edits, might ICE the compiler and set your pets on fire if it doesn't)
I've tested this version... and it worked well besides the optimization.. :)
[SNIP]
Review
Quoted 8 lines
(quoted diff) +8
-0+static void context_sched_task(struct perf_cpu_context *cpuctx, + struct perf_event_context *ctx, + bool sched_in) +{ + struct pmu *pmu = ctx->pmu; + + if (cpuctx->sched_cb_dir[sched_in] && pmu->sched_task) + pmu->sched_task(ctx, false);applied: s/false/sched_in/
Quoted 26 lines
(quoted diff) +4
-5+} + static void perf_event_context_sched_out(struct task_struct *task, int ctxn, struct task_struct *next) {@@ -3424,9 +3433,7 @@ static void perf_event_context_sched_out WRITE_ONCE(next_ctx->task, task); perf_pmu_disable(pmu); - - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(ctx, false); + context_sched_task(cpuctx, ctx, false); /* * PMU specific parts of task perf context can require@@ -3465,8 +3472,7 @@ static void perf_event_context_sched_out raw_spin_lock(&ctx->lock); perf_pmu_disable(pmu); - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(ctx, false); + context_sched_task(cpuctx, ctx, false); task_ctx_sched_out(cpuctx, ctx, EVENT_ALL); perf_pmu_enable(pmu);[SNIP]
Review void __perf_event_task_sched_out(struct · line 3585
Quoted 7 lines
(quoted diff) +1
-2@@ -3563,8 +3582,7 @@ void __perf_event_task_sched_out(struct { int ctxn; - if (__this_cpu_read(perf_sched_cb_usage)) - perf_pmu_sched_task(task, next, false); + perf_pmu_sched_task(task, next, false);I think the reason is this change. It now calls
perf_pmu_sched_task()without checking the counter. And this is for per-cpu events.Quoted 14 lines
(quoted diff) +1
-2if (atomic_read(&nr_switch_events)) perf_event_switch(task, next, false);@@ -3828,8 +3846,7 @@ static void perf_event_context_sched_in( cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE); perf_event_sched_in(cpuctx, ctx, task); - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(cpuctx->task_ctx, true); + context_sched_task(cpuctx, ctx, true); perf_pmu_enable(pmu);Review void __perf_event_task_sched_in(struct t · line 3895
Quoted 6 lines
(quoted diff) +1
-2@@ -3875,8 +3892,7 @@ void __perf_event_task_sched_in(struct t if (atomic_read(&nr_switch_events)) perf_event_switch(task, prev, true); - if (__this_cpu_read(perf_sched_cb_usage)) - perf_pmu_sched_task(prev, task, true); + perf_pmu_sched_task(prev, task, true);Ditto.
Review void __perf_event_task_sched_in(struct t · line 3898
Quoted 3 lines
(quoted diff) +0
-0} static u64 perf_calculate_period(struct perf_event *event, u64 nsec, u64 count)So I made a change like below.. and it could bring the optimization back.
Thanks,
Namhyungkernel/events/core.c +0
-0@@ -3528,6 +3528,9 @@ static void __perf_pmu_sched_task(struct
perf_cpu_context*cpuctx, boolsched_in{ struct pmu *pmu;kernel/events/core.c +3
-0+if (!cpuctx->sched_cb_dir[sched_in]) +return ; + pmu = cpuctx->ctx.pmu;/* software PMUs will not have sched_task */ if (WARN_ON_ONCE (!pmu->sched_task))
Re: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
Quoted 3 lines · Namhyung Kim
Hi Peter and Kan,
How can we move this forward?
Hi Namhyung,
Thanks for the test. The changes look good to me.
Hi Peter,
Should we resend the patch set for further review?
Thanks,
Kan
Quoted 126 lines
Thanks,
NamhyungQuoted 121 lines · Namhyung Kim
Hi Peter,
Quoted 9 lines · Peter Zijlstra
Quoted 6 lines · On Mon, Nov 30, 2020 at 11:38:42AM -0800, [email protected] wrote:
From: Kan Liang <[email protected]>
Some calls to
sched_task()in a context switch can be avoided. For example, large PEBS only requires flushing the buffer in context switch out. The current code still invokes thesched_task()for large PEBS in context switch in.Quoted 5 lines
I still hate this one, how's something like this then? Which I still don't really like.. but at least its simpler.
(completely untested, may contain spurious edits, might ICE the compiler and set your pets on fire if it doesn't)
I've tested this version... and it worked well besides the optimization.. :)
[SNIP]
Review
Quoted 8 lines
(quoted diff) +8
-0+static void context_sched_task(struct perf_cpu_context *cpuctx, + struct perf_event_context *ctx, + bool sched_in) +{ + struct pmu *pmu = ctx->pmu; + + if (cpuctx->sched_cb_dir[sched_in] && pmu->sched_task) + pmu->sched_task(ctx, false);applied: s/false/sched_in/
Quoted 26 lines
(quoted diff) +4
-5+} + static void perf_event_context_sched_out(struct task_struct *task, int ctxn, struct task_struct *next) {@@ -3424,9 +3433,7 @@ static void perf_event_context_sched_out WRITE_ONCE(next_ctx->task, task); perf_pmu_disable(pmu); - - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(ctx, false); + context_sched_task(cpuctx, ctx, false); /* * PMU specific parts of task perf context can require@@ -3465,8 +3472,7 @@ static void perf_event_context_sched_out raw_spin_lock(&ctx->lock); perf_pmu_disable(pmu); - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(ctx, false); + context_sched_task(cpuctx, ctx, false); task_ctx_sched_out(cpuctx, ctx, EVENT_ALL); perf_pmu_enable(pmu);[SNIP]
Review void __perf_event_task_sched_out(struct · line 3585
Quoted 7 lines
(quoted diff) +1
-2@@ -3563,8 +3582,7 @@ void __perf_event_task_sched_out(struct { int ctxn; - if (__this_cpu_read(perf_sched_cb_usage)) - perf_pmu_sched_task(task, next, false); + perf_pmu_sched_task(task, next, false);I think the reason is this change. It now calls
perf_pmu_sched_task()without checking the counter. And this is for per-cpu events.Quoted 14 lines
(quoted diff) +1
-2if (atomic_read(&nr_switch_events)) perf_event_switch(task, next, false);@@ -3828,8 +3846,7 @@ static void perf_event_context_sched_in( cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE); perf_event_sched_in(cpuctx, ctx, task); - if (cpuctx->sched_cb_usage && pmu->sched_task) - pmu->sched_task(cpuctx->task_ctx, true); + context_sched_task(cpuctx, ctx, true); perf_pmu_enable(pmu);Review void __perf_event_task_sched_in(struct t · line 3895
Quoted 6 lines
(quoted diff) +1
-2@@ -3875,8 +3892,7 @@ void __perf_event_task_sched_in(struct t if (atomic_read(&nr_switch_events)) perf_event_switch(task, prev, true); - if (__this_cpu_read(perf_sched_cb_usage)) - perf_pmu_sched_task(prev, task, true); + perf_pmu_sched_task(prev, task, true);Ditto.
Review void __perf_event_task_sched_in(struct t · line 3898
Quoted 3 lines
(quoted diff) +0
-0} static u64 perf_calculate_period(struct perf_event *event, u64 nsec, u64 count)So I made a change like below.. and it could bring the optimization back.
Thanks,
Namhyungkernel/events/core.c +0
-0@@ -3528,6 +3528,9 @@ static void __perf_pmu_sched_task(struct
perf_cpu_context*cpuctx, boolsched_in{ struct pmu *pmu;kernel/events/core.c +3
-0+if (!cpuctx->sched_cb_dir[sched_in]) +return ; + pmu = cpuctx->ctx.pmu;/* software PMUs will not have sched_task */ if (WARN_ON_ONCE (!pmu->sched_task))
Peter Zijlstra
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
Quoted 7 lines · Kan
Quoted 3 lines · Namhyung Kim
Hi Peter and Kan,
How can we move this forward?
Quoted 7 lines
Hi Namhyung,
Thanks for the test. The changes look good to me.
Hi Peter,
Should we resend the patch set for further review?
I've not yet seen a coherent replacement of #3, what I send was just a PoC.
Namhyung Kim
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
Hi Peter and Kan,
Quoted 11 lines · Peter Zijlstra
Quoted 9 lines · Kan
Quoted 3 lines · Namhyung Kim
Hi Peter and Kan,
How can we move this forward?
Hi Namhyung,
Quoted 8 lines
Quoted 5 lines
Thanks for the test. The changes look good to me.
Hi Peter,
Should we resend the patch set for further review?
I've not yet seen a coherent replacement of #3, what I send was just a PoC.
Any updates?
Thanks,
Namhyung
Namhyung Kim
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
Hi,
Quoted 23 lines · Namhyung Kim
Hi Peter and Kan,
Quoted 19 lines · Peter Zijlstra
Quoted 7 lines · Kan
Quoted 3 lines · Namhyung Kim
Hi Peter and Kan,
How can we move this forward?
Quoted 7 lines
Hi Namhyung,
Thanks for the test. The changes look good to me.
Hi Peter,
Should we resend the patch set for further review?
I've not yet seen a coherent replacement of #3, what I send was just a PoC.
If it's the only problem of #3 which is an optimization, can we merge the actual fixes in #1 and #2 first?
I know some people waiting for the fix..
Thanks,
Namhyung
Namhyung Kim
maintainerRe: [PATCH V2 3/3] perf: Optimize sched_task() in a context switch
Quoted 25 lines · Namhyung Kim
Hi,
Quoted 21 lines · Namhyung Kim
Hi Peter and Kan,
Quoted 17 lines · Peter Zijlstra
Quoted 14 lines · Kan
Quoted 3 lines · Namhyung Kim
Hi Peter and Kan,
How can we move this forward?
Hi Namhyung,
Thanks for the test. The changes look good to me.
Hi Peter,
Should we resend the patch set for further review?
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
I've not yet seen a coherent replacement of #3, what I send was just a PoC.
If it's the only problem of #3 which is an optimization, can we merge the actual fixes in #1 and #2 first?
I know some people waiting for the fix..
Ping again...
Thanks,
Namhyung
Peter Zijlstra
maintainerQuoted 1 line · On Mon, Nov 30, 2020 at 11:38:40AM -0800, [email protected] wrote:
+static
DEFINE_PER_CPU(int, perf_sched_cb_usages);
% s/perf_sched_cb_usages/perf_sched_cb_usage/g
The following commit has been merged into the perf/urgent branch of tip:
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: e748d3716e0e581401630d36d3ef0fc8fa8f830d Gitweb: https://git.kernel.org/tip/e748d3716e0e581401630d36d3ef0fc8fa8f830d Author: Kan Liang <[email protected]> AuthorDate: Mon, 30 Nov 2020 11:38:40 -08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Mon, 01 Mar 2021 11:02:18 +01:00
perf/core: Flush PMU internal buffers for per-CPU events
Sometimes the PMU internal buffers have to be flushed for per-CPU events during a context switch, e.g., large PEBS. Otherwise, the perf tool may report samples in locations that do not belong to the process where the samples are processed in, because PEBS does not tag samples with PID/TID.
The current code only flush the buffers for a per-task event. It doesn't check a per-CPU event.
Add a new event state flag, PERF_ATTACH_SCHED_CB, to indicate that the PMU internal buffers have to be flushed for this event during a context switch.
Add sched_cb_entry and perf_sched_cb_usages back to track the PMU/cpuctx which is required to be flushed.
Only need to invoke the sched_task() for per-CPU events in this patch. The per-task events have been handled in perf_event_context_sched_in/out already.
--- include/linux/perf_event.h | 2 ++- kernel/events/core.c | 42 +++++++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-)
include/linux/perf_event.h +2 -0
@@ -606,6 +606,7 @@ struct swevent_hlist {#define PERF_ATTACH_TASK 0x04#define PERF_ATTACH_TASK_DATA 0x08#define PERF_ATTACH_ITRACE 0x10 +#define PERF_ATTACH_SCHED_CB 0x20struct perf_cgroup ;struct perf_buffer ;
@@ -872,6 +873,7 @@ struct perf_cpu_context {struct list_head cgrp_cpuctx_entry;#endif +struct list_head sched_cb_entry;int sched_cb_usage;int online;
kernel/events/core.c +38 -4
100 lines not on this page · raw message
The following commit has been merged into the perf/urgent branch of tip:
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: a5398bffc01fe044848c5024e5e867e407f239b8 Gitweb: https://git.kernel.org/tip/a5398bffc01fe044848c5024e5e867e407f239b8 Author: Kan Liang <[email protected]> AuthorDate: Mon, 30 Nov 2020 11:38:40 -08:00 Committer: Ingo Molnar <[email protected]> CommitterDate: Sat, 06 Mar 2021 12:52:39 +01:00
perf/core: Flush PMU internal buffers for per-CPU events
Sometimes the PMU internal buffers have to be flushed for per-CPU events during a context switch, e.g., large PEBS. Otherwise, the perf tool may report samples in locations that do not belong to the process where the samples are processed in, because PEBS does not tag samples with PID/TID.
The current code only flush the buffers for a per-task event. It doesn't check a per-CPU event.
Add a new event state flag, PERF_ATTACH_SCHED_CB, to indicate that the PMU internal buffers have to be flushed for this event during a context switch.
Add sched_cb_entry and perf_sched_cb_usages back to track the PMU/cpuctx which is required to be flushed.
Only need to invoke the sched_task() for per-CPU events in this patch. The per-task events have been handled in perf_event_context_sched_in/out already.
--- include/linux/perf_event.h | 2 ++- kernel/events/core.c | 42 +++++++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-)
include/linux/perf_event.h +2 -0
@@ -606,6 +606,7 @@ struct swevent_hlist {#define PERF_ATTACH_TASK 0x04#define PERF_ATTACH_TASK_DATA 0x08#define PERF_ATTACH_ITRACE 0x10 +#define PERF_ATTACH_SCHED_CB 0x20struct perf_cgroup ;struct perf_buffer ;
@@ -872,6 +873,7 @@ struct perf_cpu_context {struct list_head cgrp_cpuctx_entry;#endif +struct list_head sched_cb_entry;int sched_cb_usage;int online;
kernel/events/core.c +38 -4
100 lines not on this page · raw message