* [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3
@ 2023-11-02 15:09 Peter Zijlstra
2023-11-02 15:09 ` [PATCH 01/13] perf: Simplify perf_event_alloc() error path Peter Zijlstra
` (12 more replies)
0 siblings, 13 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Hi,
These few patches start the process of using Scope Based Resource Management
(SBRM) for the perf core.
As per the subject, I have cut all the patches I have for this into 3 separate
parts so as to not overload you all with some 35+ patches.
I plan to post the next batch in about a week or so, we'll see. I'm hoping we
can get them all merged in time for 6.8.
The full pile can be admired here:
git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git perf/guards
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 01/13] perf: Simplify perf_event_alloc() error path
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-03 12:38 ` Jiri Olsa
2023-11-02 15:09 ` [PATCH 02/13] perf: Simplify perf_pmu_register() " Peter Zijlstra
` (11 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
The error cleanup sequence in perf_event_alloc() is a subset of the
existing _free_event() function (it must of course be).
Split this out into __free_event() and simplify the error path.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/perf_event.h | 1
kernel/events/core.c | 129 ++++++++++++++++++++++-----------------------
2 files changed, 66 insertions(+), 64 deletions(-)
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -634,6 +634,7 @@ struct swevent_hlist {
#define PERF_ATTACH_ITRACE 0x10
#define PERF_ATTACH_SCHED_CB 0x20
#define PERF_ATTACH_CHILD 0x40
+#define PERF_ATTACH_EXCLUSIVE 0x80
struct bpf_prog;
struct perf_cgroup;
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5094,6 +5094,8 @@ static int exclusive_event_init(struct p
return -EBUSY;
}
+ event->attach_state |= PERF_ATTACH_EXCLUSIVE;
+
return 0;
}
@@ -5101,14 +5103,13 @@ static void exclusive_event_destroy(stru
{
struct pmu *pmu = event->pmu;
- if (!is_exclusive_pmu(pmu))
- return;
-
/* see comment in exclusive_event_init() */
if (event->attach_state & PERF_ATTACH_TASK)
atomic_dec(&pmu->exclusive_cnt);
else
atomic_inc(&pmu->exclusive_cnt);
+
+ event->attach_state &= ~PERF_ATTACH_EXCLUSIVE;
}
static bool exclusive_event_match(struct perf_event *e1, struct perf_event *e2)
@@ -5143,38 +5144,22 @@ static bool exclusive_event_installable(
static void perf_addr_filters_splice(struct perf_event *event,
struct list_head *head);
-static void _free_event(struct perf_event *event)
+/* vs perf_event_alloc() error */
+static void __free_event(struct perf_event *event)
{
- irq_work_sync(&event->pending_irq);
-
- unaccount_event(event);
-
- security_perf_event_free(event);
-
- if (event->rb) {
- /*
- * Can happen when we close an event with re-directed output.
- *
- * Since we have a 0 refcount, perf_mmap_close() will skip
- * over us; possibly making our ring_buffer_put() the last.
- */
- mutex_lock(&event->mmap_mutex);
- ring_buffer_attach(event, NULL);
- mutex_unlock(&event->mmap_mutex);
- }
-
- if (is_cgroup_event(event))
- perf_detach_cgroup(event);
-
if (!event->parent) {
if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
put_callchain_buffers();
}
- perf_event_free_bpf_prog(event);
- perf_addr_filters_splice(event, NULL);
kfree(event->addr_filter_ranges);
+ if (event->attach_state & PERF_ATTACH_EXCLUSIVE)
+ exclusive_event_destroy(event);
+
+ if (is_cgroup_event(event))
+ perf_detach_cgroup(event);
+
if (event->destroy)
event->destroy(event);
@@ -5185,22 +5170,56 @@ static void _free_event(struct perf_even
if (event->hw.target)
put_task_struct(event->hw.target);
- if (event->pmu_ctx)
+ if (event->pmu_ctx) {
+ /*
+ * put_pmu_ctx() needs an event->ctx reference, because of
+ * epc->ctx.
+ */
+ WARN_ON_ONCE(!event->ctx);
+ WARN_ON_ONCE(event->pmu_ctx->ctx != event->ctx);
put_pmu_ctx(event->pmu_ctx);
+ }
/*
- * perf_event_free_task() relies on put_ctx() being 'last', in particular
- * all task references must be cleaned up.
+ * perf_event_free_task() relies on put_ctx() being 'last', in
+ * particular all task references must be cleaned up.
*/
if (event->ctx)
put_ctx(event->ctx);
- exclusive_event_destroy(event);
- module_put(event->pmu->module);
+ if (event->pmu)
+ module_put(event->pmu->module);
call_rcu(&event->rcu_head, free_event_rcu);
}
+/* vs perf_event_alloc() success */
+static void _free_event(struct perf_event *event)
+{
+ irq_work_sync(&event->pending_irq);
+
+ unaccount_event(event);
+
+ security_perf_event_free(event);
+
+ if (event->rb) {
+ /*
+ * Can happen when we close an event with re-directed output.
+ *
+ * Since we have a 0 refcount, perf_mmap_close() will skip
+ * over us; possibly making our ring_buffer_put() the last.
+ */
+ mutex_lock(&event->mmap_mutex);
+ ring_buffer_attach(event, NULL);
+ mutex_unlock(&event->mmap_mutex);
+ }
+
+ perf_event_free_bpf_prog(event);
+ perf_addr_filters_splice(event, NULL);
+
+ __free_event(event);
+}
+
/*
* Used to free events which have a known refcount of 1, such as in error paths
* where the event isn't exposed yet and inherited events.
@@ -11591,8 +11610,10 @@ static int perf_try_init_event(struct pm
event->destroy(event);
}
- if (ret)
+ if (ret) {
+ event->pmu = NULL;
module_put(pmu->module);
+ }
return ret;
}
@@ -11918,7 +11939,7 @@ perf_event_alloc(struct perf_event_attr
* See perf_output_read().
*/
if (attr->inherit && (attr->sample_type & PERF_SAMPLE_READ))
- goto err_ns;
+ goto err;
if (!has_branch_stack(event))
event->attr.branch_sample_type = 0;
@@ -11926,7 +11947,7 @@ perf_event_alloc(struct perf_event_attr
pmu = perf_init_event(event);
if (IS_ERR(pmu)) {
err = PTR_ERR(pmu);
- goto err_ns;
+ goto err;
}
/*
@@ -11936,24 +11957,24 @@ perf_event_alloc(struct perf_event_attr
*/
if (pmu->task_ctx_nr == perf_invalid_context && (task || cgroup_fd != -1)) {
err = -EINVAL;
- goto err_pmu;
+ goto err;
}
if (event->attr.aux_output &&
!(pmu->capabilities & PERF_PMU_CAP_AUX_OUTPUT)) {
err = -EOPNOTSUPP;
- goto err_pmu;
+ goto err;
}
if (cgroup_fd != -1) {
err = perf_cgroup_connect(cgroup_fd, event, attr, group_leader);
if (err)
- goto err_pmu;
+ goto err;
}
err = exclusive_event_init(event);
if (err)
- goto err_pmu;
+ goto err;
if (has_addr_filter(event)) {
event->addr_filter_ranges = kcalloc(pmu->nr_addr_filters,
@@ -11961,7 +11982,7 @@ perf_event_alloc(struct perf_event_attr
GFP_KERNEL);
if (!event->addr_filter_ranges) {
err = -ENOMEM;
- goto err_per_task;
+ goto err;
}
/*
@@ -11986,41 +12007,21 @@ perf_event_alloc(struct perf_event_attr
if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN) {
err = get_callchain_buffers(attr->sample_max_stack);
if (err)
- goto err_addr_filters;
+ goto err;
}
}
err = security_perf_event_alloc(event);
if (err)
- goto err_callchain_buffer;
+ goto err;
/* symmetric to unaccount_event() in _free_event() */
account_event(event);
return event;
-err_callchain_buffer:
- if (!event->parent) {
- if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
- put_callchain_buffers();
- }
-err_addr_filters:
- kfree(event->addr_filter_ranges);
-
-err_per_task:
- exclusive_event_destroy(event);
-
-err_pmu:
- if (is_cgroup_event(event))
- perf_detach_cgroup(event);
- if (event->destroy)
- event->destroy(event);
- module_put(pmu->module);
-err_ns:
- if (event->hw.target)
- put_task_struct(event->hw.target);
- call_rcu(&event->rcu_head, free_event_rcu);
-
+err:
+ __free_event(event);
return ERR_PTR(err);
}
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 02/13] perf: Simplify perf_pmu_register() error path
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
2023-11-02 15:09 ` [PATCH 01/13] perf: Simplify perf_event_alloc() error path Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 03/13] perf: Simplify perf_fget_light() Peter Zijlstra
` (10 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
The error path of perf_pmu_register() is of course very similar to a
subset of perf_pmu_unregister(). Extract this common part in
perf_pmu_free() and simplify things.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 51 ++++++++++++++++++++++++---------------------------
1 file changed, 24 insertions(+), 27 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11426,20 +11426,35 @@ static int pmu_dev_alloc(struct pmu *pmu
static struct lock_class_key cpuctx_mutex;
static struct lock_class_key cpuctx_lock;
+static void perf_pmu_free(struct pmu *pmu)
+{
+ free_percpu(pmu->pmu_disable_count);
+ if (pmu->type >= 0)
+ idr_remove(&pmu_idr, pmu->type);
+ if (pmu_bus_running && pmu->dev && pmu->dev != PMU_NULL_DEV) {
+ if (pmu->nr_addr_filters)
+ device_remove_file(pmu->dev, &dev_attr_nr_addr_filters);
+ device_del(pmu->dev);
+ put_device(pmu->dev);
+ }
+ free_pmu_context(pmu);
+}
+
int perf_pmu_register(struct pmu *pmu, const char *name, int type)
{
int cpu, ret, max = PERF_TYPE_MAX;
+ pmu->type = -1;
+
mutex_lock(&pmus_lock);
ret = -ENOMEM;
pmu->pmu_disable_count = alloc_percpu(int);
if (!pmu->pmu_disable_count)
goto unlock;
- pmu->type = -1;
if (WARN_ONCE(!name, "Can not register anonymous pmu.\n")) {
ret = -EINVAL;
- goto free_pdc;
+ goto free;
}
pmu->name = name;
@@ -11449,23 +11464,22 @@ int perf_pmu_register(struct pmu *pmu, c
ret = idr_alloc(&pmu_idr, pmu, max, 0, GFP_KERNEL);
if (ret < 0)
- goto free_pdc;
+ goto free;
WARN_ON(type >= 0 && ret != type);
- type = ret;
- pmu->type = type;
+ pmu->type = ret;
if (pmu_bus_running && !pmu->dev) {
ret = pmu_dev_alloc(pmu);
if (ret)
- goto free_idr;
+ goto free;
}
ret = -ENOMEM;
pmu->cpu_pmu_context = alloc_percpu(struct perf_cpu_pmu_context);
if (!pmu->cpu_pmu_context)
- goto free_dev;
+ goto free;
for_each_possible_cpu(cpu) {
struct perf_cpu_pmu_context *cpc;
@@ -11511,17 +11525,8 @@ int perf_pmu_register(struct pmu *pmu, c
return ret;
-free_dev:
- if (pmu->dev && pmu->dev != PMU_NULL_DEV) {
- device_del(pmu->dev);
- put_device(pmu->dev);
- }
-
-free_idr:
- idr_remove(&pmu_idr, pmu->type);
-
-free_pdc:
- free_percpu(pmu->pmu_disable_count);
+free:
+ perf_pmu_free(pmu);
goto unlock;
}
EXPORT_SYMBOL_GPL(perf_pmu_register);
@@ -11538,15 +11543,7 @@ void perf_pmu_unregister(struct pmu *pmu
synchronize_srcu(&pmus_srcu);
synchronize_rcu();
- free_percpu(pmu->pmu_disable_count);
- idr_remove(&pmu_idr, pmu->type);
- if (pmu_bus_running && pmu->dev && pmu->dev != PMU_NULL_DEV) {
- if (pmu->nr_addr_filters)
- device_remove_file(pmu->dev, &dev_attr_nr_addr_filters);
- device_del(pmu->dev);
- put_device(pmu->dev);
- }
- free_pmu_context(pmu);
+ perf_pmu_free(pmu);
mutex_unlock(&pmus_lock);
}
EXPORT_SYMBOL_GPL(perf_pmu_unregister);
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 03/13] perf: Simplify perf_fget_light()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
2023-11-02 15:09 ` [PATCH 01/13] perf: Simplify perf_event_alloc() error path Peter Zijlstra
2023-11-02 15:09 ` [PATCH 02/13] perf: Simplify perf_pmu_register() " Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 04/13] perf: Simplify event_function*() Peter Zijlstra
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Introduce fdnull and use it to simplify perf_fget_light() to either
return a valid struct fd or not -- much like fdget() itself.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/file.h | 7 ++++++-
kernel/events/core.c | 22 +++++++++++-----------
2 files changed, 17 insertions(+), 12 deletions(-)
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -59,6 +59,8 @@ static inline struct fd __to_fd(unsigned
return (struct fd){(struct file *)(v & ~3),v & 3};
}
+#define fdnull __to_fd(0)
+
static inline struct fd fdget(unsigned int fd)
{
return __to_fd(__fdget(fd));
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5802,18 +5802,17 @@ EXPORT_SYMBOL_GPL(perf_event_period);
static const struct file_operations perf_fops;
-static inline int perf_fget_light(int fd, struct fd *p)
+static inline struct fd perf_fdget(int fd)
{
struct fd f = fdget(fd);
if (!f.file)
- return -EBADF;
+ return fdnull;
if (f.file->f_op != &perf_fops) {
fdput(f);
- return -EBADF;
+ return fdnull;
}
- *p = f;
- return 0;
+ return f;
}
static int perf_event_set_output(struct perf_event *event,
@@ -5864,10 +5863,9 @@ static long _perf_ioctl(struct perf_even
int ret;
if (arg != -1) {
struct perf_event *output_event;
- struct fd output;
- ret = perf_fget_light(arg, &output);
- if (ret)
- return ret;
+ struct fd output = perf_fdget(arg);
+ if (!output.file)
+ return -EBADF;
output_event = output.file->private_data;
ret = perf_event_set_output(event, output_event);
fdput(output);
@@ -12401,9 +12399,11 @@ SYSCALL_DEFINE5(perf_event_open,
return event_fd;
if (group_fd != -1) {
- err = perf_fget_light(group_fd, &group);
- if (err)
+ group = perf_fdget(group_fd);
+ if (!group.file) {
+ err = -EBADF;
goto err_fd;
+ }
group_leader = group.file->private_data;
if (flags & PERF_FLAG_FD_OUTPUT)
output_event = group_leader;
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 04/13] perf: Simplify event_function*()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (2 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 03/13] perf: Simplify perf_fget_light() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 05/13] perf: Simplify perf_cgroup_connect() Peter Zijlstra
` (8 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Use guards to reduce gotos and simplify control flow.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -214,6 +214,19 @@ struct event_function_struct {
void *data;
};
+typedef struct {
+ struct perf_cpu_context *cpuctx;
+ struct perf_event_context *ctx;
+} class_perf_ctx_lock_t;
+
+static inline void class_perf_ctx_lock_destructor(class_perf_ctx_lock_t *_T)
+{ perf_ctx_unlock(_T->cpuctx, _T->ctx); }
+
+static inline class_perf_ctx_lock_t
+class_perf_ctx_lock_constructor(struct perf_cpu_context *cpuctx,
+ struct perf_event_context *ctx)
+{ perf_ctx_lock(cpuctx, ctx); return (class_perf_ctx_lock_t){ cpuctx, ctx }; }
+
static int event_function(void *info)
{
struct event_function_struct *efs = info;
@@ -221,20 +234,17 @@ static int event_function(void *info)
struct perf_event_context *ctx = event->ctx;
struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
struct perf_event_context *task_ctx = cpuctx->task_ctx;
- int ret = 0;
lockdep_assert_irqs_disabled();
+ guard(perf_ctx_lock)(cpuctx, task_ctx);
- perf_ctx_lock(cpuctx, task_ctx);
/*
* Since we do the IPI call without holding ctx->lock things can have
* changed, double check we hit the task we set out to hit.
*/
if (ctx->task) {
- if (ctx->task != current) {
- ret = -ESRCH;
- goto unlock;
- }
+ if (ctx->task != current)
+ return -ESRCH;
/*
* We only use event_function_call() on established contexts,
@@ -254,10 +264,8 @@ static int event_function(void *info)
}
efs->func(event, cpuctx, ctx, efs->data);
-unlock:
- perf_ctx_unlock(cpuctx, task_ctx);
- return ret;
+ return 0;
}
static void event_function_call(struct perf_event *event, event_f func, void *data)
@@ -329,11 +337,11 @@ static void event_function_local(struct
task_ctx = ctx;
}
- perf_ctx_lock(cpuctx, task_ctx);
+ guard(perf_ctx_lock)(cpuctx, task_ctx);
task = ctx->task;
if (task == TASK_TOMBSTONE)
- goto unlock;
+ return;
if (task) {
/*
@@ -343,18 +351,16 @@ static void event_function_local(struct
*/
if (ctx->is_active) {
if (WARN_ON_ONCE(task != current))
- goto unlock;
+ return;
if (WARN_ON_ONCE(cpuctx->task_ctx != ctx))
- goto unlock;
+ return;
}
} else {
WARN_ON_ONCE(&cpuctx->ctx != ctx);
}
func(event, cpuctx, ctx, data);
-unlock:
- perf_ctx_unlock(cpuctx, task_ctx);
}
#define PERF_FLAG_ALL (PERF_FLAG_FD_NO_GROUP |\
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 05/13] perf: Simplify perf_cgroup_connect()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (3 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 04/13] perf: Simplify event_function*() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 06/13] perf; Simplify event_sched_in() Peter Zijlstra
` (7 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Use CLASS to reduce gotos and simplify control flow.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/file.h | 2 +-
kernel/events/core.c | 19 ++++++++-----------
2 files changed, 9 insertions(+), 12 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -936,22 +936,20 @@ static inline int perf_cgroup_connect(in
{
struct perf_cgroup *cgrp;
struct cgroup_subsys_state *css;
- struct fd f = fdget(fd);
- int ret = 0;
+ int ret;
+ CLASS(fd, f)(fd);
if (!f.file)
return -EBADF;
css = css_tryget_online_from_dir(f.file->f_path.dentry,
&perf_event_cgrp_subsys);
- if (IS_ERR(css)) {
- ret = PTR_ERR(css);
- goto out;
- }
+ if (IS_ERR(css))
+ return PTR_ERR(css);
ret = perf_cgroup_ensure_storage(event, css);
if (ret)
- goto out;
+ return ret;
cgrp = container_of(css, struct perf_cgroup, css);
event->cgrp = cgrp;
@@ -963,11 +961,10 @@ static inline int perf_cgroup_connect(in
*/
if (group_leader && group_leader->cgrp != cgrp) {
perf_detach_cgroup(event);
- ret = -EINVAL;
+ return -EINVAL;
}
-out:
- fdput(f);
- return ret;
+
+ return 0;
}
static inline void
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 06/13] perf; Simplify event_sched_in()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (4 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 05/13] perf: Simplify perf_cgroup_connect() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 07/13] perf: Simplify: __perf_install_in_context() Peter Zijlstra
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Use guards to reduce gotos and simplify control flow.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1153,6 +1153,8 @@ void perf_pmu_enable(struct pmu *pmu)
pmu->pmu_enable(pmu);
}
+DEFINE_GUARD(perf_pmu_disable, struct pmu *, perf_pmu_disable(_T), perf_pmu_enable(_T))
+
static void perf_assert_pmu_disabled(struct pmu *pmu)
{
WARN_ON_ONCE(*this_cpu_ptr(pmu->pmu_disable_count) == 0);
@@ -2489,7 +2491,6 @@ event_sched_in(struct perf_event *event,
{
struct perf_event_pmu_context *epc = event->pmu_ctx;
struct perf_cpu_pmu_context *cpc = this_cpu_ptr(epc->pmu->cpu_pmu_context);
- int ret = 0;
WARN_ON_ONCE(event->ctx != ctx);
@@ -2517,15 +2518,14 @@ event_sched_in(struct perf_event *event,
event->hw.interrupts = 0;
}
- perf_pmu_disable(event->pmu);
+ guard(perf_pmu_disable)(event->pmu);
perf_log_itrace_start(event);
if (event->pmu->add(event, PERF_EF_START)) {
perf_event_set_state(event, PERF_EVENT_STATE_INACTIVE);
event->oncpu = -1;
- ret = -EAGAIN;
- goto out;
+ return -EAGAIN;
}
if (!is_software_event(event))
@@ -2536,10 +2536,7 @@ event_sched_in(struct perf_event *event,
if (event->attr.exclusive)
cpc->exclusive = 1;
-out:
- perf_pmu_enable(event->pmu);
-
- return ret;
+ return 0;
}
static int
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 07/13] perf: Simplify: __perf_install_in_context()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (5 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 06/13] perf; Simplify event_sched_in() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-04 1:08 ` Namhyung Kim
2023-11-02 15:09 ` [PATCH 08/13] perf: Simplify: *perf_event_{dis,en}able*() Peter Zijlstra
` (5 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2732,13 +2732,13 @@ static int __perf_install_in_context(vo
struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
struct perf_event_context *task_ctx = cpuctx->task_ctx;
bool reprogram = true;
- int ret = 0;
- raw_spin_lock(&cpuctx->ctx.lock);
- if (ctx->task) {
- raw_spin_lock(&ctx->lock);
+ if (ctx->task)
task_ctx = ctx;
+ guard(perf_ctx_lock)(cpuctx, task_ctx);
+
+ if (ctx->task) {
reprogram = (ctx->task == current);
/*
@@ -2748,14 +2748,10 @@ static int __perf_install_in_context(vo
* If its not running, we don't care, ctx->lock will
* serialize against it becoming runnable.
*/
- if (task_curr(ctx->task) && !reprogram) {
- ret = -ESRCH;
- goto unlock;
- }
+ if (task_curr(ctx->task) && !reprogram)
+ return -ESRCH;
WARN_ON_ONCE(reprogram && cpuctx->task_ctx && cpuctx->task_ctx != ctx);
- } else if (task_ctx) {
- raw_spin_lock(&task_ctx->lock);
}
#ifdef CONFIG_CGROUP_PERF
@@ -2778,10 +2774,7 @@ static int __perf_install_in_context(vo
add_event_to_ctx(event, ctx);
}
-unlock:
- perf_ctx_unlock(cpuctx, task_ctx);
-
- return ret;
+ return 0;
}
static bool exclusive_event_installable(struct perf_event *event,
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 08/13] perf: Simplify: *perf_event_{dis,en}able*()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (6 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 07/13] perf: Simplify: __perf_install_in_context() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 09/13] perf: Simplify perf_event_modify_attr() Peter Zijlstra
` (4 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 51 ++++++++++++++++++++++-----------------------------
1 file changed, 22 insertions(+), 29 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2415,7 +2415,7 @@ static void __perf_event_disable(struct
update_cgrp_time_from_event(event);
}
- perf_pmu_disable(event->pmu_ctx->pmu);
+ guard(perf_pmu_disable)(event->pmu_ctx->pmu);
if (event == event->group_leader)
group_sched_out(event, ctx);
@@ -2424,8 +2424,6 @@ static void __perf_event_disable(struct
perf_event_set_state(event, PERF_EVENT_STATE_OFF);
perf_cgroup_event_disable(event, ctx);
-
- perf_pmu_enable(event->pmu_ctx->pmu);
}
/*
@@ -2446,12 +2444,10 @@ static void _perf_event_disable(struct p
{
struct perf_event_context *ctx = event->ctx;
- raw_spin_lock_irq(&ctx->lock);
- if (event->state <= PERF_EVENT_STATE_OFF) {
- raw_spin_unlock_irq(&ctx->lock);
- return;
+ scoped_guard (raw_spinlock_irq, &ctx->lock) {
+ if (event->state <= PERF_EVENT_STATE_OFF)
+ return;
}
- raw_spin_unlock_irq(&ctx->lock);
event_function_call(event, __perf_event_disable, NULL);
}
@@ -2955,32 +2951,29 @@ static void _perf_event_enable(struct pe
{
struct perf_event_context *ctx = event->ctx;
- raw_spin_lock_irq(&ctx->lock);
- if (event->state >= PERF_EVENT_STATE_INACTIVE ||
- event->state < PERF_EVENT_STATE_ERROR) {
-out:
- raw_spin_unlock_irq(&ctx->lock);
- return;
- }
+ scoped_guard (raw_spinlock_irq, &ctx->lock) {
+ if (event->state >= PERF_EVENT_STATE_INACTIVE ||
+ event->state < PERF_EVENT_STATE_ERROR)
+ return;
- /*
- * If the event is in error state, clear that first.
- *
- * That way, if we see the event in error state below, we know that it
- * has gone back into error state, as distinct from the task having
- * been scheduled away before the cross-call arrived.
- */
- if (event->state == PERF_EVENT_STATE_ERROR) {
/*
- * Detached SIBLING events cannot leave ERROR state.
+ * If the event is in error state, clear that first.
+ *
+ * That way, if we see the event in error state below, we know that it
+ * has gone back into error state, as distinct from the task having
+ * been scheduled away before the cross-call arrived.
*/
- if (event->event_caps & PERF_EV_CAP_SIBLING &&
- event->group_leader == event)
- goto out;
+ if (event->state == PERF_EVENT_STATE_ERROR) {
+ /*
+ * Detached SIBLING events cannot leave ERROR state.
+ */
+ if (event->event_caps & PERF_EV_CAP_SIBLING &&
+ event->group_leader == event)
+ return;
- event->state = PERF_EVENT_STATE_OFF;
+ event->state = PERF_EVENT_STATE_OFF;
+ }
}
- raw_spin_unlock_irq(&ctx->lock);
event_function_call(event, __perf_event_enable, NULL);
}
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 09/13] perf: Simplify perf_event_modify_attr()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (7 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 08/13] perf: Simplify: *perf_event_{dis,en}able*() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 10/13] perf: Simplify perf_event_context_sched_in() Peter Zijlstra
` (3 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3172,7 +3172,7 @@ static int perf_event_modify_attr(struct
WARN_ON_ONCE(event->ctx->parent_ctx);
- mutex_lock(&event->child_mutex);
+ guard(mutex)(&event->child_mutex);
/*
* Event-type-independent attributes must be copied before event-type
* modification, which will validate that final attributes match the
@@ -3181,16 +3181,16 @@ static int perf_event_modify_attr(struct
perf_event_modify_copy_attr(&event->attr, attr);
err = func(event, attr);
if (err)
- goto out;
+ return err;
+
list_for_each_entry(child, &event->child_list, child_list) {
perf_event_modify_copy_attr(&child->attr, attr);
err = func(child, attr);
if (err)
- goto out;
+ return err;
}
-out:
- mutex_unlock(&event->child_mutex);
- return err;
+
+ return 0;
}
static void __pmu_ctx_sched_out(struct perf_event_pmu_context *pmu_ctx,
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 10/13] perf: Simplify perf_event_context_sched_in()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (8 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 09/13] perf: Simplify perf_event_modify_attr() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context() Peter Zijlstra
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 38 +++++++++++++++-----------------------
1 file changed, 15 insertions(+), 23 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -713,6 +713,9 @@ static void perf_ctx_enable(struct perf_
}
}
+DEFINE_GUARD(perf_ctx_disable, struct perf_event_context *,
+ perf_ctx_disable(_T, false), perf_ctx_enable(_T, false))
+
static void ctx_sched_out(struct perf_event_context *ctx, enum event_type_t event_type);
static void ctx_sched_in(struct perf_event_context *ctx, enum event_type_t event_type);
@@ -3903,31 +3906,27 @@ static void perf_event_context_sched_in(
struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
struct perf_event_context *ctx;
- rcu_read_lock();
+ guard(rcu)();
+
ctx = rcu_dereference(task->perf_event_ctxp);
if (!ctx)
- goto rcu_unlock;
-
- if (cpuctx->task_ctx == ctx) {
- perf_ctx_lock(cpuctx, ctx);
- perf_ctx_disable(ctx, false);
-
- perf_ctx_sched_task_cb(ctx, true);
-
- perf_ctx_enable(ctx, false);
- perf_ctx_unlock(cpuctx, ctx);
- goto rcu_unlock;
- }
+ return;
- perf_ctx_lock(cpuctx, ctx);
+ guard(perf_ctx_lock)(cpuctx, ctx);
/*
* We must check ctx->nr_events while holding ctx->lock, such
* that we serialize against perf_install_in_context().
*/
if (!ctx->nr_events)
- goto unlock;
+ return;
+
+ guard(perf_ctx_disable)(ctx);
+
+ if (cpuctx->task_ctx == ctx) {
+ perf_ctx_sched_task_cb(ctx, true);
+ return;
+ }
- perf_ctx_disable(ctx, false);
/*
* We want to keep the following priority order:
* cpu pinned (that don't need to move), task pinned,
@@ -3947,13 +3946,6 @@ static void perf_event_context_sched_in(
if (!RB_EMPTY_ROOT(&ctx->pinned_groups.tree))
perf_ctx_enable(&cpuctx->ctx, false);
-
- perf_ctx_enable(ctx, false);
-
-unlock:
- perf_ctx_unlock(cpuctx, ctx);
-rcu_unlock:
- rcu_read_unlock();
}
/*
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (9 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 10/13] perf: Simplify perf_event_context_sched_in() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-04 1:14 ` Namhyung Kim
2023-11-02 15:09 ` [PATCH 12/13] perf: Simplify perf_event_*_on_exec() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 13/13] perf: Simplify *perf_event_read*() Peter Zijlstra
12 siblings, 1 reply; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 51 +++++++++++++++++++++++----------------------------
1 file changed, 23 insertions(+), 28 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4090,7 +4090,7 @@ perf_adjust_freq_unthr_context(struct pe
if (!(ctx->nr_freq || unthrottle))
return;
- raw_spin_lock(&ctx->lock);
+ guard(raw_spinlock)(&ctx->lock);
list_for_each_entry_rcu(event, &ctx->event_list, event_entry) {
if (event->state != PERF_EVENT_STATE_ACTIVE)
@@ -4100,7 +4100,7 @@ perf_adjust_freq_unthr_context(struct pe
if (!event_filter_match(event))
continue;
- perf_pmu_disable(event->pmu);
+ guard(perf_pmu_disable)(event->pmu);
hwc = &event->hw;
@@ -4110,34 +4110,29 @@ perf_adjust_freq_unthr_context(struct pe
event->pmu->start(event, 0);
}
- if (!event->attr.freq || !event->attr.sample_freq)
- goto next;
+ if (event->attr.freq && event->attr.sample_freq) {
+ /*
+ * stop the event and update event->count
+ */
+ event->pmu->stop(event, PERF_EF_UPDATE);
+
+ now = local64_read(&event->count);
+ delta = now - hwc->freq_count_stamp;
+ hwc->freq_count_stamp = now;
+
+ /*
+ * restart the event
+ * reload only if value has changed
+ * we have stopped the event so tell that
+ * to perf_adjust_period() to avoid stopping it
+ * twice.
+ */
+ if (delta > 0)
+ perf_adjust_period(event, period, delta, false);
- /*
- * stop the event and update event->count
- */
- event->pmu->stop(event, PERF_EF_UPDATE);
-
- now = local64_read(&event->count);
- delta = now - hwc->freq_count_stamp;
- hwc->freq_count_stamp = now;
-
- /*
- * restart the event
- * reload only if value has changed
- * we have stopped the event so tell that
- * to perf_adjust_period() to avoid stopping it
- * twice.
- */
- if (delta > 0)
- perf_adjust_period(event, period, delta, false);
-
- event->pmu->start(event, delta > 0 ? PERF_EF_RELOAD : 0);
- next:
- perf_pmu_enable(event->pmu);
+ event->pmu->start(event, delta > 0 ? PERF_EF_RELOAD : 0);
+ }
}
-
- raw_spin_unlock(&ctx->lock);
}
/*
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 12/13] perf: Simplify perf_event_*_on_exec()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (10 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 13/13] perf: Simplify *perf_event_read*() Peter Zijlstra
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 88 +++++++++++++++++++++++----------------------------
1 file changed, 40 insertions(+), 48 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4318,39 +4318,36 @@ static void perf_event_enable_on_exec(st
enum event_type_t event_type = 0;
struct perf_cpu_context *cpuctx;
struct perf_event *event;
- unsigned long flags;
int enabled = 0;
- local_irq_save(flags);
- if (WARN_ON_ONCE(current->perf_event_ctxp != ctx))
- goto out;
-
- if (!ctx->nr_events)
- goto out;
-
- cpuctx = this_cpu_ptr(&perf_cpu_context);
- perf_ctx_lock(cpuctx, ctx);
- ctx_sched_out(ctx, EVENT_TIME);
-
- list_for_each_entry(event, &ctx->event_list, event_entry) {
- enabled |= event_enable_on_exec(event, ctx);
- event_type |= get_event_type(event);
+ scoped_guard (irqsave) {
+ if (WARN_ON_ONCE(current->perf_event_ctxp != ctx))
+ return;
+
+ if (!ctx->nr_events)
+ return;
+
+ cpuctx = this_cpu_ptr(&perf_cpu_context);
+ guard(perf_ctx_lock)(cpuctx, ctx);
+
+ ctx_sched_out(ctx, EVENT_TIME);
+
+ list_for_each_entry(event, &ctx->event_list, event_entry) {
+ enabled |= event_enable_on_exec(event, ctx);
+ event_type |= get_event_type(event);
+ }
+
+ /*
+ * Unclone and reschedule this context if we enabled any event.
+ */
+ if (enabled) {
+ clone_ctx = unclone_ctx(ctx);
+ ctx_resched(cpuctx, ctx, event_type);
+ } else {
+ ctx_sched_in(ctx, EVENT_TIME);
+ }
}
- /*
- * Unclone and reschedule this context if we enabled any event.
- */
- if (enabled) {
- clone_ctx = unclone_ctx(ctx);
- ctx_resched(cpuctx, ctx, event_type);
- } else {
- ctx_sched_in(ctx, EVENT_TIME);
- }
- perf_ctx_unlock(cpuctx, ctx);
-
-out:
- local_irq_restore(flags);
-
if (clone_ctx)
put_ctx(clone_ctx);
}
@@ -4367,34 +4364,29 @@ static void perf_event_remove_on_exec(st
{
struct perf_event_context *clone_ctx = NULL;
struct perf_event *event, *next;
- unsigned long flags;
bool modified = false;
- mutex_lock(&ctx->mutex);
+ scoped_guard (mutex, &ctx->mutex) {
+ if (WARN_ON_ONCE(ctx->task != current))
+ return;
- if (WARN_ON_ONCE(ctx->task != current))
- goto unlock;
+ list_for_each_entry_safe(event, next, &ctx->event_list, event_entry) {
+ if (!event->attr.remove_on_exec)
+ continue;
- list_for_each_entry_safe(event, next, &ctx->event_list, event_entry) {
- if (!event->attr.remove_on_exec)
- continue;
+ if (!is_kernel_event(event))
+ perf_remove_from_owner(event);
- if (!is_kernel_event(event))
- perf_remove_from_owner(event);
+ modified = true;
- modified = true;
+ perf_event_exit_event(event, ctx);
+ }
- perf_event_exit_event(event, ctx);
+ guard(raw_spinlock_irqsave)(&ctx->lock);
+ if (modified)
+ clone_ctx = unclone_ctx(ctx);
}
- raw_spin_lock_irqsave(&ctx->lock, flags);
- if (modified)
- clone_ctx = unclone_ctx(ctx);
- raw_spin_unlock_irqrestore(&ctx->lock, flags);
-
-unlock:
- mutex_unlock(&ctx->mutex);
-
if (clone_ctx)
put_ctx(clone_ctx);
}
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 13/13] perf: Simplify *perf_event_read*()
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
` (11 preceding siblings ...)
2023-11-02 15:09 ` [PATCH 12/13] perf: Simplify perf_event_*_on_exec() Peter Zijlstra
@ 2023-11-02 15:09 ` Peter Zijlstra
12 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-02 15:09 UTC (permalink / raw)
To: mingo
Cc: linux-kernel, peterz, acme, mark.rutland, alexander.shishkin,
jolsa, namhyung, irogers, adrian.hunter
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 54 ++++++++++++++++-----------------------------------
1 file changed, 17 insertions(+), 37 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4434,7 +4434,8 @@ static void __perf_event_read(void *info
if (ctx->task && cpuctx->task_ctx != ctx)
return;
- raw_spin_lock(&ctx->lock);
+ guard(raw_spinlock)(&ctx->lock);
+
if (ctx->is_active & EVENT_TIME) {
update_context_time(ctx);
update_cgrp_time_from_event(event);
@@ -4445,12 +4446,12 @@ static void __perf_event_read(void *info
perf_event_update_sibling_time(event);
if (event->state != PERF_EVENT_STATE_ACTIVE)
- goto unlock;
+ return;
if (!data->group) {
pmu->read(event);
data->ret = 0;
- goto unlock;
+ return;
}
pmu->start_txn(pmu, PERF_PMU_TXN_READ);
@@ -4468,9 +4469,6 @@ static void __perf_event_read(void *info
}
data->ret = pmu->commit_txn(pmu);
-
-unlock:
- raw_spin_unlock(&ctx->lock);
}
static inline u64 perf_event_count(struct perf_event *event)
@@ -4501,32 +4499,25 @@ static void calc_timer_values(struct per
int perf_event_read_local(struct perf_event *event, u64 *value,
u64 *enabled, u64 *running)
{
- unsigned long flags;
int event_oncpu;
int event_cpu;
- int ret = 0;
-
/*
* Disabling interrupts avoids all counter scheduling (context
* switches, timer based rotation and IPIs).
*/
- local_irq_save(flags);
+ guard(irqsave)();
/*
* It must not be an event with inherit set, we cannot read
* all child counters from atomic context.
*/
- if (event->attr.inherit) {
- ret = -EOPNOTSUPP;
- goto out;
- }
+ if (event->attr.inherit)
+ return -EOPNOTSUPP;
/* If this is a per-task event, it must be for current */
if ((event->attach_state & PERF_ATTACH_TASK) &&
- event->hw.target != current) {
- ret = -EINVAL;
- goto out;
- }
+ event->hw.target != current)
+ return -EINVAL;
/*
* Get the event CPU numbers, and adjust them to local if the event is
@@ -4537,16 +4528,12 @@ int perf_event_read_local(struct perf_ev
/* If this is a per-CPU event, it must be for this CPU */
if (!(event->attach_state & PERF_ATTACH_TASK) &&
- event_cpu != smp_processor_id()) {
- ret = -EINVAL;
- goto out;
- }
+ event_cpu != smp_processor_id())
+ return -EINVAL;
/* If this is a pinned event it must be running on this CPU */
- if (event->attr.pinned && event_oncpu != smp_processor_id()) {
- ret = -EBUSY;
- goto out;
- }
+ if (event->attr.pinned && event_oncpu != smp_processor_id())
+ return -EBUSY;
/*
* If the event is currently on this CPU, its either a per-task event,
@@ -4566,10 +4553,8 @@ int perf_event_read_local(struct perf_ev
if (running)
*running = __running;
}
-out:
- local_irq_restore(flags);
- return ret;
+ return 0;
}
static int perf_event_read(struct perf_event *event, bool group)
@@ -4603,7 +4588,7 @@ static int perf_event_read(struct perf_e
.ret = 0,
};
- preempt_disable();
+ guard(preempt)();
event_cpu = __perf_event_read_cpu(event, event_cpu);
/*
@@ -4617,19 +4602,15 @@ static int perf_event_read(struct perf_e
* after this.
*/
(void)smp_call_function_single(event_cpu, __perf_event_read, &data, 1);
- preempt_enable();
ret = data.ret;
} else if (state == PERF_EVENT_STATE_INACTIVE) {
struct perf_event_context *ctx = event->ctx;
- unsigned long flags;
- raw_spin_lock_irqsave(&ctx->lock, flags);
+ guard(raw_spinlock_irqsave)(&ctx->lock);
state = event->state;
- if (state != PERF_EVENT_STATE_INACTIVE) {
- raw_spin_unlock_irqrestore(&ctx->lock, flags);
+ if (state != PERF_EVENT_STATE_INACTIVE)
goto again;
- }
/*
* May read while context is not active (e.g., thread is
@@ -4643,7 +4624,6 @@ static int perf_event_read(struct perf_e
perf_event_update_time(event);
if (group)
perf_event_update_sibling_time(event);
- raw_spin_unlock_irqrestore(&ctx->lock, flags);
}
return ret;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 01/13] perf: Simplify perf_event_alloc() error path
2023-11-02 15:09 ` [PATCH 01/13] perf: Simplify perf_event_alloc() error path Peter Zijlstra
@ 2023-11-03 12:38 ` Jiri Olsa
2023-11-03 19:50 ` Namhyung Kim
2023-11-15 9:31 ` Peter Zijlstra
0 siblings, 2 replies; 23+ messages in thread
From: Jiri Olsa @ 2023-11-03 12:38 UTC (permalink / raw)
To: Peter Zijlstra
Cc: mingo, linux-kernel, acme, mark.rutland, alexander.shishkin,
namhyung, irogers, adrian.hunter
On Thu, Nov 02, 2023 at 04:09:20PM +0100, Peter Zijlstra wrote:
SNIP
> @@ -11936,24 +11957,24 @@ perf_event_alloc(struct perf_event_attr
> */
> if (pmu->task_ctx_nr == perf_invalid_context && (task || cgroup_fd != -1)) {
> err = -EINVAL;
> - goto err_pmu;
> + goto err;
> }
>
> if (event->attr.aux_output &&
> !(pmu->capabilities & PERF_PMU_CAP_AUX_OUTPUT)) {
> err = -EOPNOTSUPP;
> - goto err_pmu;
> + goto err;
> }
>
> if (cgroup_fd != -1) {
> err = perf_cgroup_connect(cgroup_fd, event, attr, group_leader);
> if (err)
> - goto err_pmu;
> + goto err;
> }
>
> err = exclusive_event_init(event);
> if (err)
> - goto err_pmu;
> + goto err;
>
> if (has_addr_filter(event)) {
> event->addr_filter_ranges = kcalloc(pmu->nr_addr_filters,
> @@ -11961,7 +11982,7 @@ perf_event_alloc(struct perf_event_attr
> GFP_KERNEL);
> if (!event->addr_filter_ranges) {
> err = -ENOMEM;
> - goto err_per_task;
> + goto err;
> }
>
> /*
> @@ -11986,41 +12007,21 @@ perf_event_alloc(struct perf_event_attr
> if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN) {
> err = get_callchain_buffers(attr->sample_max_stack);
> if (err)
> - goto err_addr_filters;
> + goto err;
> }
> }
>
> err = security_perf_event_alloc(event);
> if (err)
> - goto err_callchain_buffer;
> + goto err;
>
> /* symmetric to unaccount_event() in _free_event() */
> account_event(event);
>
> return event;
>
> -err_callchain_buffer:
> - if (!event->parent) {
> - if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
> - put_callchain_buffers();
> - }
hum, so this is now called all the time via __free_event, but it should
be called only if we passed get_callchain_buffers call.. this could screw
up nr_callchain_events number eventually no?
jirka
> -err_addr_filters:
> - kfree(event->addr_filter_ranges);
> -
> -err_per_task:
> - exclusive_event_destroy(event);
> -
> -err_pmu:
> - if (is_cgroup_event(event))
> - perf_detach_cgroup(event);
> - if (event->destroy)
> - event->destroy(event);
> - module_put(pmu->module);
> -err_ns:
> - if (event->hw.target)
> - put_task_struct(event->hw.target);
> - call_rcu(&event->rcu_head, free_event_rcu);
> -
> +err:
> + __free_event(event);
> return ERR_PTR(err);
> }
>
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 01/13] perf: Simplify perf_event_alloc() error path
2023-11-03 12:38 ` Jiri Olsa
@ 2023-11-03 19:50 ` Namhyung Kim
2023-11-15 9:58 ` Peter Zijlstra
2023-11-15 9:31 ` Peter Zijlstra
1 sibling, 1 reply; 23+ messages in thread
From: Namhyung Kim @ 2023-11-03 19:50 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, mingo, linux-kernel, acme, mark.rutland,
alexander.shishkin, irogers, adrian.hunter
Hi Jiri and Peter,
On Fri, Nov 3, 2023 at 5:38 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Nov 02, 2023 at 04:09:20PM +0100, Peter Zijlstra wrote:
>
> SNIP
>
> > @@ -11936,24 +11957,24 @@ perf_event_alloc(struct perf_event_attr
> > */
> > if (pmu->task_ctx_nr == perf_invalid_context && (task || cgroup_fd != -1)) {
> > err = -EINVAL;
> > - goto err_pmu;
> > + goto err;
> > }
> >
> > if (event->attr.aux_output &&
> > !(pmu->capabilities & PERF_PMU_CAP_AUX_OUTPUT)) {
> > err = -EOPNOTSUPP;
> > - goto err_pmu;
> > + goto err;
> > }
> >
> > if (cgroup_fd != -1) {
> > err = perf_cgroup_connect(cgroup_fd, event, attr, group_leader);
> > if (err)
> > - goto err_pmu;
> > + goto err;
> > }
> >
> > err = exclusive_event_init(event);
> > if (err)
> > - goto err_pmu;
> > + goto err;
> >
> > if (has_addr_filter(event)) {
> > event->addr_filter_ranges = kcalloc(pmu->nr_addr_filters,
> > @@ -11961,7 +11982,7 @@ perf_event_alloc(struct perf_event_attr
> > GFP_KERNEL);
> > if (!event->addr_filter_ranges) {
> > err = -ENOMEM;
> > - goto err_per_task;
> > + goto err;
> > }
> >
> > /*
> > @@ -11986,41 +12007,21 @@ perf_event_alloc(struct perf_event_attr
> > if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN) {
> > err = get_callchain_buffers(attr->sample_max_stack);
> > if (err)
> > - goto err_addr_filters;
> > + goto err;
> > }
> > }
> >
> > err = security_perf_event_alloc(event);
> > if (err)
> > - goto err_callchain_buffer;
> > + goto err;
> >
> > /* symmetric to unaccount_event() in _free_event() */
> > account_event(event);
> >
> > return event;
> >
> > -err_callchain_buffer:
> > - if (!event->parent) {
> > - if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
> > - put_callchain_buffers();
> > - }
>
> hum, so this is now called all the time via __free_event, but it should
> be called only if we passed get_callchain_buffers call.. this could screw
> up nr_callchain_events number eventually no?
Looks like so.
>
> jirka
>
> > -err_addr_filters:
> > - kfree(event->addr_filter_ranges);
> > -
> > -err_per_task:
> > - exclusive_event_destroy(event);
> > -
> > -err_pmu:
> > - if (is_cgroup_event(event))
> > - perf_detach_cgroup(event);
> > - if (event->destroy)
> > - event->destroy(event);
> > - module_put(pmu->module);
I'm afraid of this part. If it failed at perf_init_event(), it might
call event->destroy() already. I saw you cleared event->pmu
in the failure path. Maybe we need the same thing for the
event->destroy.
Thanks,
Namhyung
> > -err_ns:
> > - if (event->hw.target)
> > - put_task_struct(event->hw.target);
> > - call_rcu(&event->rcu_head, free_event_rcu);
> > -
> > +err:
> > + __free_event(event);
> > return ERR_PTR(err);
> > }
> >
> >
> >
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 07/13] perf: Simplify: __perf_install_in_context()
2023-11-02 15:09 ` [PATCH 07/13] perf: Simplify: __perf_install_in_context() Peter Zijlstra
@ 2023-11-04 1:08 ` Namhyung Kim
2023-11-15 9:24 ` Peter Zijlstra
0 siblings, 1 reply; 23+ messages in thread
From: Namhyung Kim @ 2023-11-04 1:08 UTC (permalink / raw)
To: Peter Zijlstra
Cc: mingo, linux-kernel, acme, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter
You may want to remove ":" after Simplify in the subject line
here and the next patch.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context()
2023-11-02 15:09 ` [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context() Peter Zijlstra
@ 2023-11-04 1:14 ` Namhyung Kim
2023-11-15 10:31 ` Peter Zijlstra
0 siblings, 1 reply; 23+ messages in thread
From: Namhyung Kim @ 2023-11-04 1:14 UTC (permalink / raw)
To: Peter Zijlstra
Cc: mingo, linux-kernel, acme, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter
On Thu, Nov 2, 2023 at 8:32 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> kernel/events/core.c | 51 +++++++++++++++++++++++----------------------------
> 1 file changed, 23 insertions(+), 28 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4090,7 +4090,7 @@ perf_adjust_freq_unthr_context(struct pe
> if (!(ctx->nr_freq || unthrottle))
> return;
>
> - raw_spin_lock(&ctx->lock);
> + guard(raw_spinlock)(&ctx->lock);
>
> list_for_each_entry_rcu(event, &ctx->event_list, event_entry) {
> if (event->state != PERF_EVENT_STATE_ACTIVE)
> @@ -4100,7 +4100,7 @@ perf_adjust_freq_unthr_context(struct pe
> if (!event_filter_match(event))
> continue;
>
> - perf_pmu_disable(event->pmu);
> + guard(perf_pmu_disable)(event->pmu);
>
> hwc = &event->hw;
>
> @@ -4110,34 +4110,29 @@ perf_adjust_freq_unthr_context(struct pe
> event->pmu->start(event, 0);
> }
>
> - if (!event->attr.freq || !event->attr.sample_freq)
> - goto next;
> + if (event->attr.freq && event->attr.sample_freq) {
Any reason for this change? I think we can just change the
'goto next' to 'continue', no?
Also I think this code needs changes to optimize the access.
A similar reason for the cgroup switch, it accesses all the
pmu/events in the context even before checking the sampling
frequency. This is bad for uncore PMUs (and KVM too).
But this is a different issue..
Thanks,
Namhyung
> + /*
> + * stop the event and update event->count
> + */
> + event->pmu->stop(event, PERF_EF_UPDATE);
> +
> + now = local64_read(&event->count);
> + delta = now - hwc->freq_count_stamp;
> + hwc->freq_count_stamp = now;
> +
> + /*
> + * restart the event
> + * reload only if value has changed
> + * we have stopped the event so tell that
> + * to perf_adjust_period() to avoid stopping it
> + * twice.
> + */
> + if (delta > 0)
> + perf_adjust_period(event, period, delta, false);
>
> - /*
> - * stop the event and update event->count
> - */
> - event->pmu->stop(event, PERF_EF_UPDATE);
> -
> - now = local64_read(&event->count);
> - delta = now - hwc->freq_count_stamp;
> - hwc->freq_count_stamp = now;
> -
> - /*
> - * restart the event
> - * reload only if value has changed
> - * we have stopped the event so tell that
> - * to perf_adjust_period() to avoid stopping it
> - * twice.
> - */
> - if (delta > 0)
> - perf_adjust_period(event, period, delta, false);
> -
> - event->pmu->start(event, delta > 0 ? PERF_EF_RELOAD : 0);
> - next:
> - perf_pmu_enable(event->pmu);
> + event->pmu->start(event, delta > 0 ? PERF_EF_RELOAD : 0);
> + }
> }
> -
> - raw_spin_unlock(&ctx->lock);
> }
>
> /*
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 07/13] perf: Simplify: __perf_install_in_context()
2023-11-04 1:08 ` Namhyung Kim
@ 2023-11-15 9:24 ` Peter Zijlstra
0 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-15 9:24 UTC (permalink / raw)
To: Namhyung Kim
Cc: mingo, linux-kernel, acme, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter
On Fri, Nov 03, 2023 at 06:08:06PM -0700, Namhyung Kim wrote:
> You may want to remove ":" after Simplify in the subject line
> here and the next patch.
Duh, typing hard. Thanks!
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 01/13] perf: Simplify perf_event_alloc() error path
2023-11-03 12:38 ` Jiri Olsa
2023-11-03 19:50 ` Namhyung Kim
@ 2023-11-15 9:31 ` Peter Zijlstra
1 sibling, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-15 9:31 UTC (permalink / raw)
To: Jiri Olsa
Cc: mingo, linux-kernel, acme, mark.rutland, alexander.shishkin,
namhyung, irogers, adrian.hunter
On Fri, Nov 03, 2023 at 01:38:05PM +0100, Jiri Olsa wrote:
> > -err_callchain_buffer:
> > - if (!event->parent) {
> > - if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
> > - put_callchain_buffers();
> > - }
>
> hum, so this is now called all the time via __free_event, but it should
> be called only if we passed get_callchain_buffers call.. this could screw
> up nr_callchain_events number eventually no?
Yes, good catch, thanks!
Something like the below should handle that, no?
---
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -628,14 +628,15 @@ 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_CHILD 0x40
-#define PERF_ATTACH_EXCLUSIVE 0x80
+#define PERF_ATTACH_CONTEXT 0x0001
+#define PERF_ATTACH_GROUP 0x0002
+#define PERF_ATTACH_TASK 0x0004
+#define PERF_ATTACH_TASK_DATA 0x0008
+#define PERF_ATTACH_ITRACE 0x0010
+#define PERF_ATTACH_SCHED_CB 0x0020
+#define PERF_ATTACH_CHILD 0x0040
+#define PERF_ATTACH_EXCLUSIVE 0x0080
+#define PERF_ATTACH_CALLCHAIN 0x0100
struct bpf_prog;
struct perf_cgroup;
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5166,10 +5166,8 @@ static void perf_addr_filters_splice(str
/* vs perf_event_alloc() error */
static void __free_event(struct perf_event *event)
{
- if (!event->parent) {
- if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
- put_callchain_buffers();
- }
+ if (event->attach_state & PERF_ATTACH_CALLCHAIN)
+ put_callchain_buffers();
kfree(event->addr_filter_ranges);
@@ -12065,6 +12063,7 @@ perf_event_alloc(struct perf_event_attr
err = get_callchain_buffers(attr->sample_max_stack);
if (err)
goto err;
+ event->attach_state |= PERF_ATTACH_CALLCHAIN;
}
}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 01/13] perf: Simplify perf_event_alloc() error path
2023-11-03 19:50 ` Namhyung Kim
@ 2023-11-15 9:58 ` Peter Zijlstra
2023-11-15 15:12 ` Namhyung Kim
0 siblings, 1 reply; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-15 9:58 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, mingo, linux-kernel, acme, mark.rutland,
alexander.shishkin, irogers, adrian.hunter
On Fri, Nov 03, 2023 at 12:50:19PM -0700, Namhyung Kim wrote:
> > > -err_pmu:
> > > - if (is_cgroup_event(event))
> > > - perf_detach_cgroup(event);
> > > - if (event->destroy)
> > > - event->destroy(event);
> > > - module_put(pmu->module);
>
> I'm afraid of this part. If it failed at perf_init_event(), it might
> call event->destroy() already. I saw you cleared event->pmu
> in the failure path. Maybe we need the same thing for the
> event->destroy.
In that case event->destroy will not yet have been set, no?
And once it is set, we should call it.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context()
2023-11-04 1:14 ` Namhyung Kim
@ 2023-11-15 10:31 ` Peter Zijlstra
0 siblings, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2023-11-15 10:31 UTC (permalink / raw)
To: Namhyung Kim
Cc: mingo, linux-kernel, acme, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter
On Fri, Nov 03, 2023 at 06:14:32PM -0700, Namhyung Kim wrote:
> On Thu, Nov 2, 2023 at 8:32 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> > kernel/events/core.c | 51 +++++++++++++++++++++++----------------------------
> > 1 file changed, 23 insertions(+), 28 deletions(-)
> >
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -4090,7 +4090,7 @@ perf_adjust_freq_unthr_context(struct pe
> > if (!(ctx->nr_freq || unthrottle))
> > return;
> >
> > - raw_spin_lock(&ctx->lock);
> > + guard(raw_spinlock)(&ctx->lock);
> >
> > list_for_each_entry_rcu(event, &ctx->event_list, event_entry) {
> > if (event->state != PERF_EVENT_STATE_ACTIVE)
> > @@ -4100,7 +4100,7 @@ perf_adjust_freq_unthr_context(struct pe
> > if (!event_filter_match(event))
> > continue;
> >
> > - perf_pmu_disable(event->pmu);
> > + guard(perf_pmu_disable)(event->pmu);
> >
> > hwc = &event->hw;
> >
> > @@ -4110,34 +4110,29 @@ perf_adjust_freq_unthr_context(struct pe
> > event->pmu->start(event, 0);
> > }
> >
> > - if (!event->attr.freq || !event->attr.sample_freq)
> > - goto next;
> > + if (event->attr.freq && event->attr.sample_freq) {
>
> Any reason for this change? I think we can just change the
> 'goto next' to 'continue', no?
Linus initially got confused about the life-time of for-loop scopes, but
yeah, this could be continue just fine.
> Also I think this code needs changes to optimize the access.
> A similar reason for the cgroup switch, it accesses all the
> pmu/events in the context even before checking the sampling
> frequency. This is bad for uncore PMUs (and KVM too).
>
> But this is a different issue..
Right, lets do that in another patch. Also, there seems to be a problem
with the cgroup thing :/
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 01/13] perf: Simplify perf_event_alloc() error path
2023-11-15 9:58 ` Peter Zijlstra
@ 2023-11-15 15:12 ` Namhyung Kim
0 siblings, 0 replies; 23+ messages in thread
From: Namhyung Kim @ 2023-11-15 15:12 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Jiri Olsa, mingo, linux-kernel, acme, mark.rutland,
alexander.shishkin, irogers, adrian.hunter
On Wed, Nov 15, 2023 at 1:58 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Nov 03, 2023 at 12:50:19PM -0700, Namhyung Kim wrote:
>
> > > > -err_pmu:
> > > > - if (is_cgroup_event(event))
> > > > - perf_detach_cgroup(event);
> > > > - if (event->destroy)
> > > > - event->destroy(event);
> > > > - module_put(pmu->module);
> >
> > I'm afraid of this part. If it failed at perf_init_event(), it might
> > call event->destroy() already. I saw you cleared event->pmu
> > in the failure path. Maybe we need the same thing for the
> > event->destroy.
>
> In that case event->destroy will not yet have been set, no?
In perf_try_init_event(), it calls event->destroy() if set when it
has EXTENDED_REGS or NO_EXCLUDE capabilities and returns an error.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2023-11-15 15:13 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-02 15:09 [PATCH 00/13] perf: Employ SBRM to simplify error handling -- batch 1/3 Peter Zijlstra
2023-11-02 15:09 ` [PATCH 01/13] perf: Simplify perf_event_alloc() error path Peter Zijlstra
2023-11-03 12:38 ` Jiri Olsa
2023-11-03 19:50 ` Namhyung Kim
2023-11-15 9:58 ` Peter Zijlstra
2023-11-15 15:12 ` Namhyung Kim
2023-11-15 9:31 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 02/13] perf: Simplify perf_pmu_register() " Peter Zijlstra
2023-11-02 15:09 ` [PATCH 03/13] perf: Simplify perf_fget_light() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 04/13] perf: Simplify event_function*() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 05/13] perf: Simplify perf_cgroup_connect() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 06/13] perf; Simplify event_sched_in() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 07/13] perf: Simplify: __perf_install_in_context() Peter Zijlstra
2023-11-04 1:08 ` Namhyung Kim
2023-11-15 9:24 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 08/13] perf: Simplify: *perf_event_{dis,en}able*() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 09/13] perf: Simplify perf_event_modify_attr() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 10/13] perf: Simplify perf_event_context_sched_in() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context() Peter Zijlstra
2023-11-04 1:14 ` Namhyung Kim
2023-11-15 10:31 ` Peter Zijlstra
2023-11-02 15:09 ` [PATCH 12/13] perf: Simplify perf_event_*_on_exec() Peter Zijlstra
2023-11-02 15:09 ` [PATCH 13/13] perf: Simplify *perf_event_read*() Peter Zijlstra
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.