Messages in this thread | | | Subject | Re: [PATCH v6 1/6] perf/cgroup: Reorder perf_cgroup_connect() | From | Shuah Khan <> | Date | Fri, 14 Feb 2020 09:11:24 -0700 |
| |
On 2/14/20 12:51 AM, Ian Rogers wrote: > From: Peter Zijlstra <peterz@infradead.org> > > Move perf_cgroup_connect() after perf_event_alloc(), such that we can > find/use the PMU's cpu context.
Can you elaborate on this usage? It will helpful to know how this is used and what do we get from it. What were we missing with the way it was done before?
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Signed-off-by: Ian Rogers <irogers@google.com> > --- > kernel/events/core.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 3f1f77de7247..9bd2af954c54 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -10804,12 +10804,6 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, > if (!has_branch_stack(event)) > event->attr.branch_sample_type = 0; > > - if (cgroup_fd != -1) { > - err = perf_cgroup_connect(cgroup_fd, event, attr, group_leader); > - if (err) > - goto err_ns; > - } > - > pmu = perf_init_event(event); > if (IS_ERR(pmu)) { > err = PTR_ERR(pmu); > @@ -10831,6 +10825,12 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, > goto err_pmu;
Is this patch based on linux-next or linux 5.6-rc1. I am finding code path to be different in those. Also in https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git
goto err_ns makes more sense since if perf_init_event() doesn't return valid pmu, especially since err_pmu tries to do a put pmu->module.
Something doesn't look right.
> }
> > + if (cgroup_fd != -1) { > + err = perf_cgroup_connect(cgroup_fd, event, attr, group_leader); > + if (err) > + goto err_pmu; > + } > + > err = exclusive_event_init(event); > if (err) > goto err_pmu; > @@ -10891,12 +10891,12 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, > 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 (is_cgroup_event(event)) > - perf_detach_cgroup(event); > if (event->ns) > put_pid_ns(event->ns); > if (event->hw.target) >
thanks, -- Shuah
| |