linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V5 1/6] perf: Save PMU specific data in task_struct
@ 2021-07-13 19:45 kan.liang
  2021-07-13 19:45 ` [PATCH V5 2/6] perf: attach/detach PMU specific data kan.liang
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: kan.liang @ 2021-07-13 19:45 UTC (permalink / raw)
  To: peterz, mingo, acme, tglx, bp, linux-kernel
  Cc: eranian, namhyung, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

Some PMU specific data has to be saved/restored during context switch,
e.g. LBR call stack data. Currently, the data is saved in event context
structure, but only for per-process event. For system-wide event,
because of missing the LBR call stack data after context switch, LBR
callstacks are always shorter in comparison to per-process mode.

For example,
  Per-process mode:
  $perf record --call-graph lbr -- taskset -c 0 ./tchain_edit

  -   99.90%    99.86%  tchain_edit  tchain_edit       [.] f3
       99.86% _start
          __libc_start_main
          generic_start_main
          main
          f1
        - f2
             f3

  System-wide mode:
  $perf record --call-graph lbr -a -- taskset -c 0 ./tchain_edit

  -   99.88%    99.82%  tchain_edit  tchain_edit        [.] f3
   - 62.02% main
        f1
        f2
        f3
   - 28.83% f1
      - f2
        f3
   - 28.83% f1
      - f2
           f3
   - 8.88% generic_start_main
        main
        f1
        f2
        f3

It isn't practical to simply allocate the data for system-wide event in
CPU context structure for all tasks. We have no idea which CPU a task
will be scheduled to. The duplicated LBR data has to be maintained on
every CPU context structure. That's a huge waste. Otherwise, the LBR
data still lost if the task is scheduled to another CPU.

Save the pmu specific data in task_struct. The size of pmu specific data
is 788 bytes for LBR call stack. Usually, the overall amount of threads
doesn't exceed a few thousands. For 10K threads, keeping LBR data would
consume additional ~8MB. The additional space will only be allocated
during LBR call stack monitoring. It will be released when the
monitoring is finished.

Furthermore, moving task_ctx_data from perf_event_context to task_struct
can reduce complexity and make things clearer. E.g. perf doesn't need to
swap task_ctx_data on optimized context switch path.
This patch set is just the first step. There could be other
optimization/extension on top of this patch set. E.g. for cgroup
profiling, perf just needs to save/store the LBR call stack information
for tasks in specific cgroup. That could reduce the additional space.
Also, the LBR call stack can be available for software events, or allow
even debugging use cases, like LBRs on crash later.

The Kmem cache of pmu specific data is saved in struct perf_ctx_data.
It's required when child task allocates the space.
The refcount in struct perf_ctx_data is used to track the users of pmu
specific data.

Reviewed-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

The V4 can be found here.
https://lore.kernel.org/lkml/1621436766-112801-1-git-send-email-kan.liang@linux.intel.com/

Changes since V4:
- Add global to track system-wide users
- Remove spinlock perf_ctx_data_lock

The V3 can be found here.
https://lore.kernel.org/lkml/1578495789-95006-1-git-send-email-kan.liang@linux.intel.com/

Changes since V3:
- Rebase for the Arch LBR. Use Kmem cache to replace the data_size.

Changes since V2:
- Cannot use mutex inside rcu_read_lock().
  Restore the pin lock perf_ctx_data_lock

 include/linux/perf_event.h | 30 ++++++++++++++++++++++++++++++
 include/linux/sched.h      |  2 ++
 kernel/events/core.c       |  1 +
 3 files changed, 33 insertions(+)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f5a6a2f..ece4035d 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -851,6 +851,36 @@ struct perf_event_context {
 	struct rcu_head			rcu_head;
 };
 
+/**
+ * struct perf_ctx_data - PMU specific data for a task
+ * @rcu_head:  To avoid the race on free PMU specific data
+ * @refcount:  To track users
+ * @global:    To track system-wide users
+ * @ctx_cache: Kmem cache of PMU specific data
+ * @data:      PMU specific data
+ *
+ * Currently, the struct is only used in Intel LBR call stack mode to
+ * save/restore the call stack of a task on context switches.
+ * The data only be allocated when Intel LBR call stack mode is enabled.
+ * The data will be freed when the mode is disabled. The rcu_head is
+ * used to prevent the race on free the data.
+ * The content of the data will only be accessed in context switch, which
+ * should be protected by rcu_read_lock().
+ *
+ * Careful: Struct perf_ctx_data is added as a pointor in struct task_struct.
+ * When system-wide Intel LBR call stack mode is enabled, a buffer with
+ * constant size will be allocated for each task.
+ * Also, system memory consumption can further grow when the size of
+ * struct perf_ctx_data enlarges.
+ */
+struct perf_ctx_data {
+	struct rcu_head			rcu_head;
+	refcount_t			refcount;
+	int				global;
+	struct kmem_cache		*ctx_cache;
+	void				*data;
+};
+
 /*
  * Number of contexts where an event can trigger:
  *	task, softirq, hardirq, nmi.
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d2c8813..4b4d746b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -52,6 +52,7 @@ struct mempolicy;
 struct nameidata;
 struct nsproxy;
 struct perf_event_context;
+struct perf_ctx_data;
 struct pid_namespace;
 struct pipe_inode_info;
 struct rcu_node;
@@ -1135,6 +1136,7 @@ struct task_struct {
 	struct perf_event_context	*perf_event_ctxp[perf_nr_task_contexts];
 	struct mutex			perf_event_mutex;
 	struct list_head		perf_event_list;
+	struct perf_ctx_data __rcu	*perf_ctx_data;
 #endif
 #ifdef CONFIG_DEBUG_PREEMPT
 	unsigned long			preempt_disable_ip;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 0e125ae..dcdd164 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -13142,6 +13142,7 @@ int perf_event_init_task(struct task_struct *child, u64 clone_flags)
 	memset(child->perf_event_ctxp, 0, sizeof(child->perf_event_ctxp));
 	mutex_init(&child->perf_event_mutex);
 	INIT_LIST_HEAD(&child->perf_event_list);
+	child->perf_ctx_data = NULL;
 
 	for_each_task_context_nr(ctxn) {
 		ret = perf_event_init_context(child, ctxn, clone_flags);
-- 
2.7.4


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

* [PATCH V5 2/6] perf: attach/detach PMU specific data
  2021-07-13 19:45 [PATCH V5 1/6] perf: Save PMU specific data in task_struct kan.liang
@ 2021-07-13 19:45 ` kan.liang
  2021-07-14 19:18   ` kernel test robot
  2021-07-13 19:45 ` [PATCH V5 3/6] perf: Supply task information to sched_task() kan.liang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: kan.liang @ 2021-07-13 19:45 UTC (permalink / raw)
  To: peterz, mingo, acme, tglx, bp, linux-kernel
  Cc: eranian, namhyung, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

The LBR call stack data has to be saved/restored during context switch
to fix the shorter LBRs call stacks issue in the  system-wide mode.
Allocate PMU specific data and attach them to the corresponding
task_struct during LBR call stack monitoring.

When a LBR call stack event is accounted, the perf_ctx_data for the
related tasks will be allocated/attached by attach_perf_ctx_data().
When a LBR call stack event is unaccounted, the perf_ctx_data for
related tasks will be detached/freed by detach_perf_ctx_data().

The LBR call stack event could be a per-task event or a system-wide
event.
- For a per-task event, perf only allocates the perf_ctx_data for the
  current task. If the allocation fails, perf will error out.
- For a system-wide event, perf has to allocate the perf_ctx_data for
  both the existing tasks and the upcoming tasks.
  The allocation for the existing tasks is done in perf_event_alloc().
  If any allocation fails, perf will error out.
  The allocation for the new tasks will be done in perf_event_fork().
  A global reader/writer semaphore, global_ctx_data_rwsem, is added to
  address the global race.
- The perf_ctx_data only be freed by the last LBR call stack event.
  The number of the per-task events is tracked by refcount of each task.
  Since the system-wide events impact all tasks, it's not practical to
  go through the whole task list to update the refcount for each
  system-wide event. The number of system-wide events is tracked by a
  global variable global_ctx_data_ref.

Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

Changes since V4:
- Remove the per-task lock and simplify all the attach_* and detach_*
  functions. (Peter)
- Add a global RWSEM to protect the system-wide allocation and free.
  Add a refcount to track the global users. (Peter)
- Modify the perf_event_alloc_task_data() to use the global RWSEM
  (Peter)

Changes since V3:
- Rebase for the Arch LBR
- Use kvcalloc to replace kcalloc (Andi)

Changes since V2:
- Remove global spin lock task_data_sys_wide_events_lock
  Since the global spin lock has been removed, we cannot guarantee
  that the allocation/assignments for existing threads and free are
  serialized.
  To fix it, in V3, we go through the task list when accounting for
  each system-wide event, and assign the perf_ctx_data pointer if needed.
  (In V2, we only do the assignment for the first system-wide event).
  In V3, we also add a breaker in free process for system-wide event.
  If there is new system-wide event accounted, stop the free process
  immediately.
- Add a macro TASK_DATA_SYS_WIDE to indicate the PMU specific data
  is used by system-wide events.

 kernel/events/core.c | 286 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 286 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index dcdd164..e7e9abe 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -54,6 +54,7 @@
 #include <linux/highmem.h>
 #include <linux/pgtable.h>
 #include <linux/buildid.h>
+#include <linux/percpu-rwsem.h>
 
 #include "internal.h"
 
@@ -4776,6 +4777,224 @@ static void unaccount_freq_event(void)
 		atomic_dec(&nr_freq_events);
 }
 
+
+static struct perf_ctx_data *
+alloc_perf_ctx_data(struct kmem_cache *ctx_cache, bool global)
+{
+	struct perf_ctx_data *cd;
+
+	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+	if (!cd)
+		return NULL;
+
+	cd->data = kmem_cache_zalloc(ctx_cache, GFP_KERNEL);
+	if (!cd->data) {
+		kfree(cd);
+		return NULL;
+	}
+
+	cd->global = global;
+	cd->ctx_cache = ctx_cache;
+	refcount_set(&cd->refcount, 1);
+
+	return cd;
+}
+
+static void free_perf_ctx_data(struct perf_ctx_data *cd)
+{
+	kmem_cache_free(cd->ctx_cache, cd->data);
+	kfree(cd);
+}
+
+static void __free_perf_ctx_data_rcu(struct rcu_head *rcu_head)
+{
+	struct perf_ctx_data *cd;
+
+	cd = container_of(rcu_head, struct perf_ctx_data, rcu_head);
+	free_perf_ctx_data(cd);
+}
+
+static inline void perf_free_ctx_data_rcu(struct perf_ctx_data *cd)
+{
+	call_rcu(&cd->rcu_head, __free_perf_ctx_data_rcu);
+}
+
+static int
+attach_task_ctx_data(struct task_struct *task, struct kmem_cache *ctx_cache,
+		     bool global)
+{
+	struct perf_ctx_data *cd, *old = NULL;
+
+	cd = alloc_perf_ctx_data(ctx_cache, global);
+	if (!cd)
+		return -ENOMEM;
+
+	for (;;) {
+		if (try_cmpxchg(&task->perf_ctx_data,
+		    (struct perf_ctx_data __rcu **)&old,
+		    (struct perf_ctx_data __rcu *)cd)) {
+			if (old)
+				perf_free_ctx_data_rcu(old);
+			return 0;
+		}
+
+		if (!old) {
+			/*
+			 * After seeing a dead @old, we raced with
+			 * removal and lost, try again to install @cd.
+			 */
+			continue;
+		}
+
+		if (refcount_inc_not_zero(&old->refcount)) {
+			free_perf_ctx_data(cd); /* unused */
+			return 0;
+		}
+
+		/*
+		 * @old is a dead object, refcount==0 is stable, try and
+		 * replace it with @cd.
+		 */
+	}
+	return 0;
+}
+
+static void __detach_global_ctx_data(void);
+DEFINE_STATIC_PERCPU_RWSEM(global_ctx_data_rwsem);
+static refcount_t global_ctx_data_ref;
+
+static int
+attach_global_ctx_data(struct kmem_cache *ctx_cache)
+{
+	if (refcount_inc_not_zero(&global_ctx_data_ref))
+		return 0;
+
+	percpu_down_write(&global_ctx_data_rwsem);
+	if (!refcount_inc_not_zero(&global_ctx_data_ref)) {
+		struct task_struct *g, *p;
+		struct perf_ctx_data *cd;
+		int ret;
+
+again:
+		/* Allocate everything */
+		rcu_read_lock();
+		for_each_process_thread(g, p) {
+			cd = rcu_dereference(p->perf_ctx_data);
+			if (cd && !cd->global) {
+				cd->global = 1;
+				if (!refcount_inc_not_zero(&cd->refcount))
+					cd = NULL;
+			}
+			if (!cd) {
+				get_task_struct(p);
+				rcu_read_unlock();
+
+				ret = attach_task_ctx_data(p, ctx_cache, true);
+				put_task_struct(p);
+				if (ret) {
+					__detach_global_ctx_data();
+					return ret;
+				}
+				goto again;
+			}
+		}
+		rcu_read_unlock();
+
+		refcount_set(&global_ctx_data_ref, 1);
+	}
+	percpu_up_write(&global_ctx_data_rwsem);
+
+	return 0;
+}
+
+static int
+attach_perf_ctx_data(struct perf_event *event)
+{
+	struct task_struct *task = event->hw.target;
+	struct kmem_cache *ctx_cache = event->pmu->task_ctx_cache;
+
+	if (!ctx_cache)
+		return -ENOMEM;
+
+	if (task)
+		return attach_task_ctx_data(task, ctx_cache, false);
+	else
+		return attach_global_ctx_data(ctx_cache);
+}
+
+static void
+detach_task_ctx_data(struct task_struct *p)
+{
+	struct perf_ctx_data *cd;
+
+	rcu_read_lock();
+	cd = rcu_dereference(p->perf_ctx_data);
+	if (!cd || !refcount_dec_and_test(&cd->refcount)) {
+		rcu_read_unlock();
+		return;
+	}
+	rcu_read_unlock();
+
+	/*
+	 * The old ctx_data may be lost because of the race.
+	 * Nothing is required to do for the case.
+	 * See attach_task_ctx_data().
+	 */
+	if (try_cmpxchg(&p->perf_ctx_data, (struct perf_ctx_data __rcu **)&cd, NULL))
+		perf_free_ctx_data_rcu(cd);
+}
+
+static void __detach_global_ctx_data(void)
+{
+	struct task_struct *g, *p;
+	struct perf_ctx_data *cd;
+
+again:
+	rcu_read_lock();
+	for_each_process_thread(g, p) {
+		cd = rcu_dereference(p->perf_ctx_data);
+		if (!cd || !cd->global)
+			continue;
+		cd->global = 0;
+		get_task_struct(p);
+		rcu_read_unlock();
+
+		detach_task_ctx_data(p);
+		put_task_struct(p);
+		goto again;
+	}
+	rcu_read_unlock();
+}
+
+static void detach_global_ctx_data(void)
+{
+	if (refcount_dec_not_one(&global_ctx_data_ref))
+		return;
+
+	percpu_down_write(&global_ctx_data_rwsem);
+	if (!refcount_dec_and_test(&global_ctx_data_ref))
+		goto unlock;
+
+	/* remove everything */
+	__detach_global_ctx_data();
+
+unlock:
+	percpu_up_write(&global_ctx_data_rwsem);
+}
+
+static void detach_perf_ctx_data(struct perf_event *event)
+{
+	struct task_struct *task = event->hw.target;
+
+	if (!event->pmu->task_ctx_cache)
+		return;
+
+	if (task)
+		detach_task_ctx_data(task);
+	else
+		detach_global_ctx_data();
+}
+
 static void unaccount_event(struct perf_event *event)
 {
 	bool dec = false;
@@ -4813,6 +5032,8 @@ static void unaccount_event(struct perf_event *event)
 		atomic_dec(&nr_bpf_events);
 	if (event->attr.text_poke)
 		atomic_dec(&nr_text_poke_events);
+	if (event->attach_state & PERF_ATTACH_TASK_DATA)
+		detach_perf_ctx_data(event);
 
 	if (dec) {
 		if (!atomic_add_unless(&perf_sched_count, -1, 1))
@@ -7849,10 +8070,62 @@ static void perf_event_task(struct task_struct *task,
 		       task_ctx);
 }
 
+/*
+ * Allocate data for a new task when profiling system-wide
+ * events which require PMU specific data
+ */
+static void
+perf_event_alloc_task_data(struct task_struct *child,
+			   struct task_struct *parent)
+{
+	struct kmem_cache *ctx_cache = NULL;
+	struct perf_ctx_data *cd;
+
+	if (!refcount_read(&global_ctx_data_ref))
+		return;
+
+	rcu_read_lock();
+	cd = rcu_dereference(parent->perf_ctx_data);
+	if (cd)
+		ctx_cache = cd->ctx_cache;
+	rcu_read_unlock();
+
+	if (!ctx_cache)
+		return;
+
+	percpu_down_read(&global_ctx_data_rwsem);
+
+	rcu_read_lock();
+	cd = rcu_dereference(child->perf_ctx_data);
+
+	if (!cd) {
+		/*
+		 * A system-wide event may be unaccount,
+		 * when attaching the perf_ctx_data.
+		 */
+		if (!refcount_read(&global_ctx_data_ref))
+			goto rcu_unlock;
+		rcu_read_unlock();
+		attach_task_ctx_data(child, ctx_cache, true);
+		goto up_rwsem;
+	}
+
+	if (!cd->global) {
+		cd->global = 1;
+		refcount_inc(&cd->refcount);
+	}
+
+rcu_unlock:
+	rcu_read_unlock();
+up_rwsem:
+	percpu_up_read(&global_ctx_data_rwsem);
+}
+
 void perf_event_fork(struct task_struct *task)
 {
 	perf_event_task(task, NULL, 1);
 	perf_event_namespaces(task);
+	perf_event_alloc_task_data(task, current);
 }
 
 /*
@@ -11622,11 +11895,17 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 	if (err)
 		goto err_callchain_buffer;
 
+	if ((event->attach_state & PERF_ATTACH_TASK_DATA) &&
+	    attach_perf_ctx_data(event))
+		goto err_task_ctx_data;
+
 	/* symmetric to unaccount_event() in _free_event() */
 	account_event(event);
 
 	return event;
 
+err_task_ctx_data:
+	security_perf_event_free(event);
 err_callchain_buffer:
 	if (!event->parent) {
 		if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
@@ -12707,6 +12986,13 @@ void perf_event_exit_task(struct task_struct *child)
 	 * At this point we need to send EXIT events to cpu contexts.
 	 */
 	perf_event_task(child, NULL, 0);
+
+	/*
+	 * Detach the perf_ctx_data for the system-wide event.
+	 */
+	percpu_down_read(&global_ctx_data_rwsem);
+	detach_task_ctx_data(child);
+	percpu_up_read(&global_ctx_data_rwsem);
 }
 
 static void perf_free_event(struct perf_event *event,
-- 
2.7.4


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

* [PATCH V5 3/6] perf: Supply task information to sched_task()
  2021-07-13 19:45 [PATCH V5 1/6] perf: Save PMU specific data in task_struct kan.liang
  2021-07-13 19:45 ` [PATCH V5 2/6] perf: attach/detach PMU specific data kan.liang
@ 2021-07-13 19:45 ` kan.liang
  2021-07-13 19:45 ` [PATCH V5 4/6] perf/x86/lbr: Fix shorter LBRs call stacks for the system-wide mode kan.liang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: kan.liang @ 2021-07-13 19:45 UTC (permalink / raw)
  To: peterz, mingo, acme, tglx, bp, linux-kernel
  Cc: eranian, namhyung, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

To save/restore LBR call stack data in system-wide mode, the task_struct
information is required.

Extend the parameters of sched_task() to supply task_struct information.

When schedule in, the LBR call stack data for new task will be restored.
When schedule out, the LBR call stack data for old task will be saved.
Only need to pass the required task_struct information.

Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

No changes since V4:

Changes since V3:
- Rebase on top of the 5.13-rc2

 arch/powerpc/perf/core-book3s.c |  8 ++++++--
 arch/x86/events/core.c          |  5 +++--
 arch/x86/events/intel/core.c    |  4 ++--
 arch/x86/events/intel/lbr.c     |  3 ++-
 arch/x86/events/perf_event.h    |  5 +++--
 include/linux/perf_event.h      |  2 +-
 kernel/events/core.c            | 15 ++++++++-------
 7 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 16d4d1b..5c2cc4c 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -131,7 +131,10 @@ static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
 
 static inline void power_pmu_bhrb_enable(struct perf_event *event) {}
 static inline void power_pmu_bhrb_disable(struct perf_event *event) {}
-static void power_pmu_sched_task(struct perf_event_context *ctx, bool sched_in) {}
+static void power_pmu_sched_task(struct perf_event_context *ctx,
+				 struct task_struct *task, bool sched_in)
+{
+}
 static inline void power_pmu_bhrb_read(struct perf_event *event, struct cpu_hw_events *cpuhw) {}
 static void pmao_restore_workaround(bool ebb) { }
 #endif /* CONFIG_PPC32 */
@@ -441,7 +444,8 @@ static void power_pmu_bhrb_disable(struct perf_event *event)
 /* Called from ctxsw to prevent one process's branch entries to
  * mingle with the other process's entries during context switch.
  */
-static void power_pmu_sched_task(struct perf_event_context *ctx, bool sched_in)
+static void power_pmu_sched_task(struct perf_event_context *ctx,
+				 struct task_struct *task, bool sched_in)
 {
 	if (!ppmu->bhrb_nr)
 		return;
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index c0167d5..0d4dd78 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2627,9 +2627,10 @@ static const struct attribute_group *x86_pmu_attr_groups[] = {
 	NULL,
 };
 
-static void x86_pmu_sched_task(struct perf_event_context *ctx, bool sched_in)
+static void x86_pmu_sched_task(struct perf_event_context *ctx,
+			       struct task_struct *task, bool sched_in)
 {
-	static_call_cond(x86_pmu_sched_task)(ctx, sched_in);
+	static_call_cond(x86_pmu_sched_task)(ctx, task, sched_in);
 }
 
 static void x86_pmu_swap_task_ctx(struct perf_event_context *prev,
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 52fc444..4fe91d6 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4480,10 +4480,10 @@ static void intel_pmu_cpu_dead(int cpu)
 }
 
 static void intel_pmu_sched_task(struct perf_event_context *ctx,
-				 bool sched_in)
+				 struct task_struct *task, bool sched_in)
 {
 	intel_pmu_pebs_sched_task(ctx, sched_in);
-	intel_pmu_lbr_sched_task(ctx, sched_in);
+	intel_pmu_lbr_sched_task(ctx, task, sched_in);
 }
 
 static void intel_pmu_swap_task_ctx(struct perf_event_context *prev,
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 76dbab6..da3a91a 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -619,7 +619,8 @@ void intel_pmu_lbr_swap_task_ctx(struct perf_event_context *prev,
 	     task_context_opt(next_ctx_data)->lbr_callstack_users);
 }
 
-void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in)
+void intel_pmu_lbr_sched_task(struct perf_event_context *ctx,
+			      struct task_struct *task, bool sched_in)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 	void *task_ctx;
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index d6003e0..8489f50 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -778,7 +778,7 @@ struct x86_pmu {
 
 	void		(*check_microcode)(void);
 	void		(*sched_task)(struct perf_event_context *ctx,
-				      bool sched_in);
+				      struct task_struct *task, bool sched_in);
 
 	/*
 	 * Intel Arch Perfmon v2+
@@ -1309,7 +1309,8 @@ void intel_ds_init(void);
 void intel_pmu_lbr_swap_task_ctx(struct perf_event_context *prev,
 				 struct perf_event_context *next);
 
-void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in);
+void intel_pmu_lbr_sched_task(struct perf_event_context *ctx,
+			      struct task_struct *task, bool sched_in);
 
 u64 lbr_from_signext_quirk_wr(u64 val);
 
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index ece4035d..e90de20 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -428,7 +428,7 @@ struct pmu {
 	 * context-switches callback
 	 */
 	void (*sched_task)		(struct perf_event_context *ctx,
-					bool sched_in);
+					 struct task_struct *task, bool sched_in);
 
 	/*
 	 * Kmem cache of PMU specific data
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e7e9abe..5e22b24 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3451,7 +3451,7 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn,
 			perf_pmu_disable(pmu);
 
 			if (cpuctx->sched_cb_usage && pmu->sched_task)
-				pmu->sched_task(ctx, false);
+				pmu->sched_task(ctx, task, false);
 
 			/*
 			 * PMU specific parts of task perf context can require
@@ -3491,7 +3491,7 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn,
 		perf_pmu_disable(pmu);
 
 		if (cpuctx->sched_cb_usage && pmu->sched_task)
-			pmu->sched_task(ctx, false);
+			pmu->sched_task(ctx, task, false);
 		task_ctx_sched_out(cpuctx, ctx, EVENT_ALL);
 
 		perf_pmu_enable(pmu);
@@ -3530,7 +3530,8 @@ void perf_sched_cb_inc(struct pmu *pmu)
  * PEBS requires this to provide PID/TID information. This requires we flush
  * all queued PEBS records before we context switch to a new task.
  */
-static void __perf_pmu_sched_task(struct perf_cpu_context *cpuctx, bool sched_in)
+static void __perf_pmu_sched_task(struct perf_cpu_context *cpuctx,
+				  struct task_struct *task, bool sched_in)
 {
 	struct pmu *pmu;
 
@@ -3542,7 +3543,7 @@ static void __perf_pmu_sched_task(struct perf_cpu_context *cpuctx, bool sched_in
 	perf_ctx_lock(cpuctx, cpuctx->task_ctx);
 	perf_pmu_disable(pmu);
 
-	pmu->sched_task(cpuctx->task_ctx, sched_in);
+	pmu->sched_task(cpuctx->task_ctx, task, sched_in);
 
 	perf_pmu_enable(pmu);
 	perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
@@ -3562,7 +3563,7 @@ static void perf_pmu_sched_task(struct task_struct *prev,
 		if (cpuctx->task_ctx)
 			continue;
 
-		__perf_pmu_sched_task(cpuctx, sched_in);
+		__perf_pmu_sched_task(cpuctx, sched_in ? next : prev, sched_in);
 	}
 }
 
@@ -3835,7 +3836,7 @@ static void perf_event_context_sched_in(struct perf_event_context *ctx,
 
 	if (cpuctx->task_ctx == ctx) {
 		if (cpuctx->sched_cb_usage)
-			__perf_pmu_sched_task(cpuctx, true);
+			__perf_pmu_sched_task(cpuctx, task, true);
 		return;
 	}
 
@@ -3861,7 +3862,7 @@ static void perf_event_context_sched_in(struct perf_event_context *ctx,
 	perf_event_sched_in(cpuctx, ctx, task);
 
 	if (cpuctx->sched_cb_usage && pmu->sched_task)
-		pmu->sched_task(cpuctx->task_ctx, true);
+		pmu->sched_task(cpuctx->task_ctx, task, true);
 
 	perf_pmu_enable(pmu);
 
-- 
2.7.4


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

* [PATCH V5 4/6] perf/x86/lbr: Fix shorter LBRs call stacks for the system-wide mode
  2021-07-13 19:45 [PATCH V5 1/6] perf: Save PMU specific data in task_struct kan.liang
  2021-07-13 19:45 ` [PATCH V5 2/6] perf: attach/detach PMU specific data kan.liang
  2021-07-13 19:45 ` [PATCH V5 3/6] perf: Supply task information to sched_task() kan.liang
@ 2021-07-13 19:45 ` kan.liang
  2021-07-13 19:45 ` [PATCH V5 5/6] perf/x86: Remove swap_task_ctx() kan.liang
  2021-07-13 19:45 ` [PATCH V5 6/6] perf: Clean up pmu specific data kan.liang
  4 siblings, 0 replies; 7+ messages in thread
From: kan.liang @ 2021-07-13 19:45 UTC (permalink / raw)
  To: peterz, mingo, acme, tglx, bp, linux-kernel
  Cc: eranian, namhyung, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

In the system-wide mode, LBR callstacks are shorter in comparison to
the per-process mode.

LBR MSRs are reset during a context switch in the system-wide mode. For
the LBR call stack, the LBRs should be always saved/restored during a
context switch.

Use the space in task_struct to save/restore the LBR call stack data.

For a system-wide event, it's unnecessagy to update the
lbr_callstack_users for each threads. Add a variable in x86_pmu to
indicate whether the system-wide event is active.

Fixes: 76cb2c617f12 ("perf/x86/intel: Save/restore LBR stack during context switch")
Reported-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Debugged-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

No changes since V4

Changes since V3:
- Rebase on top of the 5.13-rc2

 arch/x86/events/intel/lbr.c  | 49 ++++++++++++++++++++++++++++++++++++--------
 arch/x86/events/perf_event.h |  1 +
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index da3a91a..8248ae1 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -502,11 +502,17 @@ static __always_inline bool lbr_is_reset_in_cstate(void *ctx)
 	return !rdlbr_from(((struct x86_perf_task_context *)ctx)->tos, NULL);
 }
 
+static inline bool has_lbr_callstack_users(void *ctx)
+{
+	return task_context_opt(ctx)->lbr_callstack_users ||
+	       x86_pmu.lbr_callstack_users;
+}
+
 static void __intel_pmu_lbr_restore(void *ctx)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 
-	if (task_context_opt(ctx)->lbr_callstack_users == 0 ||
+	if (!has_lbr_callstack_users(ctx) ||
 	    task_context_opt(ctx)->lbr_stack_state == LBR_NONE) {
 		intel_pmu_lbr_reset();
 		return;
@@ -583,7 +589,7 @@ static void __intel_pmu_lbr_save(void *ctx)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 
-	if (task_context_opt(ctx)->lbr_callstack_users == 0) {
+	if (!has_lbr_callstack_users(ctx)) {
 		task_context_opt(ctx)->lbr_stack_state = LBR_NONE;
 		return;
 	}
@@ -623,6 +629,7 @@ void intel_pmu_lbr_sched_task(struct perf_event_context *ctx,
 			      struct task_struct *task, bool sched_in)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+	struct perf_ctx_data *ctx_data;
 	void *task_ctx;
 
 	if (!cpuc->lbr_users)
@@ -633,15 +640,18 @@ void intel_pmu_lbr_sched_task(struct perf_event_context *ctx,
 	 * the task was scheduled out, restore the stack. Otherwise flush
 	 * the LBR stack.
 	 */
-	task_ctx = ctx ? ctx->task_ctx_data : NULL;
+	rcu_read_lock();
+	ctx_data = rcu_dereference(task->perf_ctx_data);
+	task_ctx = ctx_data ? ctx_data->data : NULL;
 	if (task_ctx) {
 		if (sched_in)
 			__intel_pmu_lbr_restore(task_ctx);
 		else
 			__intel_pmu_lbr_save(task_ctx);
+		rcu_read_unlock();
 		return;
 	}
-
+	rcu_read_unlock();
 	/*
 	 * Since 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
@@ -670,8 +680,19 @@ void intel_pmu_lbr_add(struct perf_event *event)
 
 	cpuc->br_sel = event->hw.branch_reg.reg;
 
-	if (branch_user_callstack(cpuc->br_sel) && event->ctx->task_ctx_data)
-		task_context_opt(event->ctx->task_ctx_data)->lbr_callstack_users++;
+	if (branch_user_callstack(cpuc->br_sel)) {
+		if (event->attach_state & PERF_ATTACH_TASK) {
+			struct task_struct *task = event->hw.target;
+			struct perf_ctx_data *ctx_data;
+
+			rcu_read_lock();
+			ctx_data = rcu_dereference(task->perf_ctx_data);
+			if (ctx_data)
+				task_context_opt(ctx_data->data)->lbr_callstack_users++;
+			rcu_read_unlock();
+		} else
+			x86_pmu.lbr_callstack_users++;
+	}
 
 	/*
 	 * Request pmu::sched_task() callback, which will fire inside the
@@ -730,9 +751,19 @@ void intel_pmu_lbr_del(struct perf_event *event)
 	if (!x86_pmu.lbr_nr)
 		return;
 
-	if (branch_user_callstack(cpuc->br_sel) &&
-	    event->ctx->task_ctx_data)
-		task_context_opt(event->ctx->task_ctx_data)->lbr_callstack_users--;
+	if (branch_user_callstack(cpuc->br_sel)) {
+		if (event->attach_state & PERF_ATTACH_TASK) {
+			struct task_struct *task = event->hw.target;
+			struct perf_ctx_data *ctx_data;
+
+			rcu_read_lock();
+			ctx_data = rcu_dereference(task->perf_ctx_data);
+			if (ctx_data)
+				task_context_opt(ctx_data->data)->lbr_callstack_users--;
+			rcu_read_unlock();
+		} else
+			x86_pmu.lbr_callstack_users--;
+	}
 
 	if (event->hw.flags & PERF_X86_EVENT_LBR_SELECT)
 		cpuc->lbr_select = 0;
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 8489f50..583c08d38 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -820,6 +820,7 @@ struct x86_pmu {
 		const int	*lbr_sel_map;	   /* lbr_select mappings */
 		int		*lbr_ctl_map;	   /* LBR_CTL mappings */
 	};
+	u64		lbr_callstack_users;	   /* lbr callstack system wide users */
 	bool		lbr_double_abort;	   /* duplicated lbr aborts */
 	bool		lbr_pt_coexist;		   /* (LBR|BTS) may coexist with PT */
 
-- 
2.7.4


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

* [PATCH V5 5/6] perf/x86: Remove swap_task_ctx()
  2021-07-13 19:45 [PATCH V5 1/6] perf: Save PMU specific data in task_struct kan.liang
                   ` (2 preceding siblings ...)
  2021-07-13 19:45 ` [PATCH V5 4/6] perf/x86/lbr: Fix shorter LBRs call stacks for the system-wide mode kan.liang
@ 2021-07-13 19:45 ` kan.liang
  2021-07-13 19:45 ` [PATCH V5 6/6] perf: Clean up pmu specific data kan.liang
  4 siblings, 0 replies; 7+ messages in thread
From: kan.liang @ 2021-07-13 19:45 UTC (permalink / raw)
  To: peterz, mingo, acme, tglx, bp, linux-kernel
  Cc: eranian, namhyung, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

The pmu specific data is saved in task_struct now. It doesn't need to
swap between context.

Remove swap_task_ctx() support.

Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

No changes since V4

Changes since V3:
- Rebase on top of the 5.13-rc2

 arch/x86/events/core.c       |  9 ---------
 arch/x86/events/intel/core.c |  7 -------
 arch/x86/events/intel/lbr.c  | 23 -----------------------
 arch/x86/events/perf_event.h | 11 -----------
 4 files changed, 50 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 0d4dd78..d59ce9c 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -79,7 +79,6 @@ DEFINE_STATIC_CALL_NULL(x86_pmu_commit_scheduling, *x86_pmu.commit_scheduling);
 DEFINE_STATIC_CALL_NULL(x86_pmu_stop_scheduling,   *x86_pmu.stop_scheduling);
 
 DEFINE_STATIC_CALL_NULL(x86_pmu_sched_task,    *x86_pmu.sched_task);
-DEFINE_STATIC_CALL_NULL(x86_pmu_swap_task_ctx, *x86_pmu.swap_task_ctx);
 
 DEFINE_STATIC_CALL_NULL(x86_pmu_drain_pebs,   *x86_pmu.drain_pebs);
 DEFINE_STATIC_CALL_NULL(x86_pmu_pebs_aliases, *x86_pmu.pebs_aliases);
@@ -2018,7 +2017,6 @@ static void x86_pmu_static_call_update(void)
 	static_call_update(x86_pmu_stop_scheduling, x86_pmu.stop_scheduling);
 
 	static_call_update(x86_pmu_sched_task, x86_pmu.sched_task);
-	static_call_update(x86_pmu_swap_task_ctx, x86_pmu.swap_task_ctx);
 
 	static_call_update(x86_pmu_drain_pebs, x86_pmu.drain_pebs);
 	static_call_update(x86_pmu_pebs_aliases, x86_pmu.pebs_aliases);
@@ -2633,12 +2631,6 @@ static void x86_pmu_sched_task(struct perf_event_context *ctx,
 	static_call_cond(x86_pmu_sched_task)(ctx, task, sched_in);
 }
 
-static void x86_pmu_swap_task_ctx(struct perf_event_context *prev,
-				  struct perf_event_context *next)
-{
-	static_call_cond(x86_pmu_swap_task_ctx)(prev, next);
-}
-
 void perf_check_microcode(void)
 {
 	if (x86_pmu.check_microcode)
@@ -2700,7 +2692,6 @@ static struct pmu pmu = {
 
 	.event_idx		= x86_pmu_event_idx,
 	.sched_task		= x86_pmu_sched_task,
-	.swap_task_ctx		= x86_pmu_swap_task_ctx,
 	.check_period		= x86_pmu_check_period,
 
 	.aux_output_match	= x86_pmu_aux_output_match,
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 4fe91d6..62f4004 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4486,12 +4486,6 @@ static void intel_pmu_sched_task(struct perf_event_context *ctx,
 	intel_pmu_lbr_sched_task(ctx, task, sched_in);
 }
 
-static void intel_pmu_swap_task_ctx(struct perf_event_context *prev,
-				    struct perf_event_context *next)
-{
-	intel_pmu_lbr_swap_task_ctx(prev, next);
-}
-
 static int intel_pmu_check_period(struct perf_event *event, u64 value)
 {
 	return intel_pmu_has_bts_period(event, value) ? -EINVAL : 0;
@@ -4640,7 +4634,6 @@ static __initconst const struct x86_pmu intel_pmu = {
 
 	.guest_get_msrs		= intel_guest_get_msrs,
 	.sched_task		= intel_pmu_sched_task,
-	.swap_task_ctx		= intel_pmu_swap_task_ctx,
 
 	.check_period		= intel_pmu_check_period,
 
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 8248ae1..a56068c 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -602,29 +602,6 @@ static void __intel_pmu_lbr_save(void *ctx)
 	cpuc->last_log_id = ++task_context_opt(ctx)->log_id;
 }
 
-void intel_pmu_lbr_swap_task_ctx(struct perf_event_context *prev,
-				 struct perf_event_context *next)
-{
-	void *prev_ctx_data, *next_ctx_data;
-
-	swap(prev->task_ctx_data, next->task_ctx_data);
-
-	/*
-	 * Architecture specific synchronization makes sense in
-	 * case both prev->task_ctx_data and next->task_ctx_data
-	 * pointers are allocated.
-	 */
-
-	prev_ctx_data = next->task_ctx_data;
-	next_ctx_data = prev->task_ctx_data;
-
-	if (!prev_ctx_data || !next_ctx_data)
-		return;
-
-	swap(task_context_opt(prev_ctx_data)->lbr_callstack_users,
-	     task_context_opt(next_ctx_data)->lbr_callstack_users);
-}
-
 void intel_pmu_lbr_sched_task(struct perf_event_context *ctx,
 			      struct task_struct *task, bool sched_in)
 {
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 583c08d38..bc8836b 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -855,14 +855,6 @@ struct x86_pmu {
 	int		(*set_topdown_event_period)(struct perf_event *event);
 
 	/*
-	 * perf task context (i.e. struct perf_event_context::task_ctx_data)
-	 * switch helper to bridge calls from perf/core to perf/x86.
-	 * See struct pmu::swap_task_ctx() usage for examples;
-	 */
-	void		(*swap_task_ctx)(struct perf_event_context *prev,
-					 struct perf_event_context *next);
-
-	/*
 	 * AMD bits
 	 */
 	unsigned int	amd_nb_constraints : 1;
@@ -1307,9 +1299,6 @@ void intel_pmu_store_pebs_lbrs(struct lbr_entry *lbr);
 
 void intel_ds_init(void);
 
-void intel_pmu_lbr_swap_task_ctx(struct perf_event_context *prev,
-				 struct perf_event_context *next);
-
 void intel_pmu_lbr_sched_task(struct perf_event_context *ctx,
 			      struct task_struct *task, bool sched_in);
 
-- 
2.7.4


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

* [PATCH V5 6/6] perf: Clean up pmu specific data
  2021-07-13 19:45 [PATCH V5 1/6] perf: Save PMU specific data in task_struct kan.liang
                   ` (3 preceding siblings ...)
  2021-07-13 19:45 ` [PATCH V5 5/6] perf/x86: Remove swap_task_ctx() kan.liang
@ 2021-07-13 19:45 ` kan.liang
  4 siblings, 0 replies; 7+ messages in thread
From: kan.liang @ 2021-07-13 19:45 UTC (permalink / raw)
  To: peterz, mingo, acme, tglx, bp, linux-kernel
  Cc: eranian, namhyung, ak, Kan Liang

From: Kan Liang <kan.liang@linux.intel.com>

The pmu specific data is saved in task_struct now. Remove it from event
context structure.

Remove swap_task_ctx() as well.

Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

No changes since V4

Changes since V3:
- Rebase on top of the 5.13-rc2

 include/linux/perf_event.h | 11 --------
 kernel/events/core.c       | 64 ++--------------------------------------------
 2 files changed, 2 insertions(+), 73 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index e90de20..6e96839 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -436,16 +436,6 @@ struct pmu {
 	struct kmem_cache		*task_ctx_cache;
 
 	/*
-	 * PMU specific parts of task perf event context (i.e. ctx->task_ctx_data)
-	 * can be synchronized using this function. See Intel LBR callstack support
-	 * implementation and Perf core context switch handling callbacks for usage
-	 * examples.
-	 */
-	void (*swap_task_ctx)		(struct perf_event_context *prev,
-					 struct perf_event_context *next);
-					/* optional */
-
-	/*
 	 * Set up pmu-private data structures for an AUX area
 	 */
 	void *(*setup_aux)		(struct perf_event *event, void **pages,
@@ -847,7 +837,6 @@ struct perf_event_context {
 #ifdef CONFIG_CGROUP_PERF
 	int				nr_cgroups;	 /* cgroup evts */
 #endif
-	void				*task_ctx_data; /* pmu specific data */
 	struct rcu_head			rcu_head;
 };
 
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5e22b24..c490246 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1241,26 +1241,11 @@ static void get_ctx(struct perf_event_context *ctx)
 	refcount_inc(&ctx->refcount);
 }
 
-static void *alloc_task_ctx_data(struct pmu *pmu)
-{
-	if (pmu->task_ctx_cache)
-		return kmem_cache_zalloc(pmu->task_ctx_cache, GFP_KERNEL);
-
-	return NULL;
-}
-
-static void free_task_ctx_data(struct pmu *pmu, void *task_ctx_data)
-{
-	if (pmu->task_ctx_cache && task_ctx_data)
-		kmem_cache_free(pmu->task_ctx_cache, task_ctx_data);
-}
-
 static void free_ctx(struct rcu_head *head)
 {
 	struct perf_event_context *ctx;
 
 	ctx = container_of(head, struct perf_event_context, rcu_head);
-	free_task_ctx_data(ctx->pmu, ctx->task_ctx_data);
 	kfree(ctx);
 }
 
@@ -3453,25 +3438,13 @@ static void perf_event_context_sched_out(struct task_struct *task, int ctxn,
 			if (cpuctx->sched_cb_usage && pmu->sched_task)
 				pmu->sched_task(ctx, task, false);
 
-			/*
-			 * PMU specific parts of task perf context can require
-			 * additional synchronization. As an example of such
-			 * synchronization see implementation details of Intel
-			 * LBR call stack data profiling;
-			 */
-			if (pmu->swap_task_ctx)
-				pmu->swap_task_ctx(ctx, next_ctx);
-			else
-				swap(ctx->task_ctx_data, next_ctx->task_ctx_data);
-
 			perf_pmu_enable(pmu);
 
 			/*
 			 * RCU_INIT_POINTER here is safe because we've not
 			 * modified the ctx and the above modification of
-			 * ctx->task and ctx->task_ctx_data are immaterial
-			 * since those values are always verified under
-			 * ctx->lock which we're now holding.
+			 * ctx->task is immaterial since this value is always
+			 * verified under ctx->lock which we're now holding.
 			 */
 			RCU_INIT_POINTER(task->perf_event_ctxp[ctxn], next_ctx);
 			RCU_INIT_POINTER(next->perf_event_ctxp[ctxn], ctx);
@@ -4605,7 +4578,6 @@ find_get_context(struct pmu *pmu, struct task_struct *task,
 {
 	struct perf_event_context *ctx, *clone_ctx = NULL;
 	struct perf_cpu_context *cpuctx;
-	void *task_ctx_data = NULL;
 	unsigned long flags;
 	int ctxn, err;
 	int cpu = event->cpu;
@@ -4629,24 +4601,12 @@ find_get_context(struct pmu *pmu, struct task_struct *task,
 	if (ctxn < 0)
 		goto errout;
 
-	if (event->attach_state & PERF_ATTACH_TASK_DATA) {
-		task_ctx_data = alloc_task_ctx_data(pmu);
-		if (!task_ctx_data) {
-			err = -ENOMEM;
-			goto errout;
-		}
-	}
-
 retry:
 	ctx = perf_lock_task_context(task, ctxn, &flags);
 	if (ctx) {
 		clone_ctx = unclone_ctx(ctx);
 		++ctx->pin_count;
 
-		if (task_ctx_data && !ctx->task_ctx_data) {
-			ctx->task_ctx_data = task_ctx_data;
-			task_ctx_data = NULL;
-		}
 		raw_spin_unlock_irqrestore(&ctx->lock, flags);
 
 		if (clone_ctx)
@@ -4657,11 +4617,6 @@ find_get_context(struct pmu *pmu, struct task_struct *task,
 		if (!ctx)
 			goto errout;
 
-		if (task_ctx_data) {
-			ctx->task_ctx_data = task_ctx_data;
-			task_ctx_data = NULL;
-		}
-
 		err = 0;
 		mutex_lock(&task->perf_event_mutex);
 		/*
@@ -4688,11 +4643,9 @@ find_get_context(struct pmu *pmu, struct task_struct *task,
 		}
 	}
 
-	free_task_ctx_data(pmu, task_ctx_data);
 	return ctx;
 
 errout:
-	free_task_ctx_data(pmu, task_ctx_data);
 	return ERR_PTR(err);
 }
 
@@ -13147,18 +13100,6 @@ inherit_event(struct perf_event *parent_event,
 	if (IS_ERR(child_event))
 		return child_event;
 
-
-	if ((child_event->attach_state & PERF_ATTACH_TASK_DATA) &&
-	    !child_ctx->task_ctx_data) {
-		struct pmu *pmu = child_event->pmu;
-
-		child_ctx->task_ctx_data = alloc_task_ctx_data(pmu);
-		if (!child_ctx->task_ctx_data) {
-			free_event(child_event);
-			return ERR_PTR(-ENOMEM);
-		}
-	}
-
 	/*
 	 * is_orphaned_event() and list_add_tail(&parent_event->child_list)
 	 * must be under the same lock in order to serialize against
@@ -13169,7 +13110,6 @@ inherit_event(struct perf_event *parent_event,
 	if (is_orphaned_event(parent_event) ||
 	    !atomic_long_inc_not_zero(&parent_event->refcount)) {
 		mutex_unlock(&parent_event->child_mutex);
-		/* task_ctx_data is freed with child_ctx */
 		free_event(child_event);
 		return NULL;
 	}
-- 
2.7.4


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

* Re: [PATCH V5 2/6] perf: attach/detach PMU specific data
  2021-07-13 19:45 ` [PATCH V5 2/6] perf: attach/detach PMU specific data kan.liang
@ 2021-07-14 19:18   ` kernel test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-07-14 19:18 UTC (permalink / raw)
  To: kan.liang, peterz, mingo, acme, tglx, bp, linux-kernel
  Cc: kbuild-all, eranian, namhyung, ak, Kan Liang

[-- Attachment #1: Type: text/plain, Size: 13176 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/perf/core]
[also build test WARNING on tip/sched/core powerpc/next v5.14-rc1 next-20210714]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/kan-liang-linux-intel-com/perf-Save-PMU-specific-data-in-task_struct/20210714-034829
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git c76826a65f50038f050424365dbf3f97203f8710
config: riscv-randconfig-s031-20210714 (attached as .config)
compiler: riscv32-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/5600461badd8a97324b24dde400e078bc7ee1cdd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review kan-liang-linux-intel-com/perf-Save-PMU-specific-data-in-task_struct/20210714-034829
        git checkout 5600461badd8a97324b24dde400e078bc7ee1cdd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=riscv SHELL=/bin/bash kernel/events/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   kernel/events/core.c:1464:15: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:1464:15: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:1464:15: sparse:    struct perf_event_context *
   kernel/events/core.c:1477:28: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:1477:28: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:1477:28: sparse:    struct perf_event_context *
   kernel/events/core.c:3427:18: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:3427:18: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:3427:18: sparse:    struct perf_event_context *
   kernel/events/core.c:3428:23: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:3428:23: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:3428:23: sparse:    struct perf_event_context *
   kernel/events/core.c:3476:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:3476:25: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:3476:25: sparse:    struct perf_event_context *
   kernel/events/core.c:3477:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:3477:25: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:3477:25: sparse:    struct perf_event_context *
   kernel/events/core.c:4677:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:4677:25: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:4677:25: sparse:    struct perf_event_context *
   kernel/events/core.c:6159:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:6159:9: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:6159:9: sparse:    struct perf_buffer *
>> kernel/events/core.c:4833:21: sparse: sparse: cast removes address space '__rcu' of expression
   kernel/events/core.c:4943:13: sparse: sparse: cast removes address space '__rcu' of expression
   kernel/events/core.c:5635:24: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __poll_t [usertype] events @@     got int @@
   kernel/events/core.c:5865:22: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:5865:22: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:5865:22: sparse:    struct perf_buffer *
   kernel/events/core.c:6001:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:6001:14: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:6001:14: sparse:    struct perf_buffer *
   kernel/events/core.c:6034:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:6034:14: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:6034:14: sparse:    struct perf_buffer *
   kernel/events/core.c:6091:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:6091:14: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:6091:14: sparse:    struct perf_buffer *
   kernel/events/core.c:6177:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:6177:14: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:6177:14: sparse:    struct perf_buffer *
   kernel/events/core.c:6190:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:6190:14: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:6190:14: sparse:    struct perf_buffer *
   kernel/events/core.c:7825:23: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:7825:23: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:7825:23: sparse:    struct perf_event_context *
   kernel/events/core.c:7877:23: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:7877:23: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:7877:23: sparse:    struct perf_event_context *
   kernel/events/core.c:7916:13: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:7916:13: sparse:    struct perf_buffer [noderef] __rcu *
   kernel/events/core.c:7916:13: sparse:    struct perf_buffer *
   kernel/events/core.c:8021:61: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected struct task_struct *p @@     got struct task_struct [noderef] __rcu *real_parent @@
   kernel/events/core.c:8021:61: sparse:     expected struct task_struct *p
   kernel/events/core.c:8021:61: sparse:     got struct task_struct [noderef] __rcu *real_parent
   kernel/events/core.c:8023:61: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected struct task_struct *p @@     got struct task_struct [noderef] __rcu *real_parent @@
   kernel/events/core.c:8023:61: sparse:     expected struct task_struct *p
   kernel/events/core.c:8023:61: sparse:     got struct task_struct [noderef] __rcu *real_parent
   kernel/events/core.c:8780:23: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:8780:23: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:8780:23: sparse:    struct perf_event_context *
   kernel/events/core.c:9745:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9745:9: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:9745:9: sparse:    struct swevent_hlist *
   kernel/events/core.c:9784:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9784:17: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:9784:17: sparse:    struct swevent_hlist *
   kernel/events/core.c:9965:23: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9965:23: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:9965:23: sparse:    struct perf_event_context *
   kernel/events/core.c:11150:1: sparse: sparse: symbol 'dev_attr_nr_addr_filters' was not declared. Should it be static?
   kernel/events/core.c:12928:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:12928:9: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:12928:9: sparse:    struct perf_event_context *
   kernel/events/core.c:13045:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:13045:17: sparse:    struct perf_event_context [noderef] __rcu *
   kernel/events/core.c:13045:17: sparse:    struct perf_event_context *
   kernel/events/core.c:13476:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:13476:17: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:13476:17: sparse:    struct swevent_hlist *
   kernel/events/core.c:168:9: sparse: sparse: context imbalance in 'perf_ctx_lock' - wrong count at exit
   kernel/events/core.c:176:17: sparse: sparse: context imbalance in 'perf_ctx_unlock' - unexpected unlock
   kernel/events/core.c: note: in included file (through include/linux/rculist.h, include/linux/dcache.h, include/linux/fs.h):
   include/linux/rcupdate.h:707:9: sparse: sparse: context imbalance in 'perf_lock_task_context' - different lock contexts for basic block
   kernel/events/core.c:1511:17: sparse: sparse: context imbalance in 'perf_pin_task_context' - unexpected unlock
   kernel/events/core.c:2824:9: sparse: sparse: context imbalance in '__perf_install_in_context' - wrong count at exit
   kernel/events/core.c:4649:17: sparse: sparse: context imbalance in 'find_get_context' - unexpected unlock
   kernel/events/core.c: note: in included file:
   kernel/events/internal.h:197:46: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const [noderef] __user *from @@     got void const *src @@
   kernel/events/core.c:9594:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9594:17: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:9594:17: sparse:    struct swevent_hlist *
   kernel/events/core.c:9614:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9614:17: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:9614:17: sparse:    struct swevent_hlist *
   kernel/events/core.c:9734:16: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9734:16: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:9734:16: sparse:    struct swevent_hlist *
   kernel/events/core.c:9734:16: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9734:16: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:9734:16: sparse:    struct swevent_hlist *
   kernel/events/core.c:9734:16: sparse: sparse: incompatible types in comparison expression (different address spaces):
   kernel/events/core.c:9734:16: sparse:    struct swevent_hlist [noderef] __rcu *
   kernel/events/core.c:9734:16: sparse:    struct swevent_hlist *

vim +/__rcu +4833 kernel/events/core.c

  4821	
  4822	static int
  4823	attach_task_ctx_data(struct task_struct *task, struct kmem_cache *ctx_cache,
  4824			     bool global)
  4825	{
  4826		struct perf_ctx_data *cd, *old = NULL;
  4827	
  4828		cd = alloc_perf_ctx_data(ctx_cache, global);
  4829		if (!cd)
  4830			return -ENOMEM;
  4831	
  4832		for (;;) {
> 4833			if (try_cmpxchg(&task->perf_ctx_data,
  4834			    (struct perf_ctx_data __rcu **)&old,
  4835			    (struct perf_ctx_data __rcu *)cd)) {
  4836				if (old)
  4837					perf_free_ctx_data_rcu(old);
  4838				return 0;
  4839			}
  4840	
  4841			if (!old) {
  4842				/*
  4843				 * After seeing a dead @old, we raced with
  4844				 * removal and lost, try again to install @cd.
  4845				 */
  4846				continue;
  4847			}
  4848	
  4849			if (refcount_inc_not_zero(&old->refcount)) {
  4850				free_perf_ctx_data(cd); /* unused */
  4851				return 0;
  4852			}
  4853	
  4854			/*
  4855			 * @old is a dead object, refcount==0 is stable, try and
  4856			 * replace it with @cd.
  4857			 */
  4858		}
  4859		return 0;
  4860	}
  4861	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34475 bytes --]

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

end of thread, other threads:[~2021-07-14 19:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 19:45 [PATCH V5 1/6] perf: Save PMU specific data in task_struct kan.liang
2021-07-13 19:45 ` [PATCH V5 2/6] perf: attach/detach PMU specific data kan.liang
2021-07-14 19:18   ` kernel test robot
2021-07-13 19:45 ` [PATCH V5 3/6] perf: Supply task information to sched_task() kan.liang
2021-07-13 19:45 ` [PATCH V5 4/6] perf/x86/lbr: Fix shorter LBRs call stacks for the system-wide mode kan.liang
2021-07-13 19:45 ` [PATCH V5 5/6] perf/x86: Remove swap_task_ctx() kan.liang
2021-07-13 19:45 ` [PATCH V5 6/6] perf: Clean up pmu specific data kan.liang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).