lkml.org 
[lkml]   [2020]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v6 1/6] perf/cgroup: Reorder perf_cgroup_connect()
    From
    Date
    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

    \
     
     \ /
      Last update: 2020-02-14 17:13    [W:2.373 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site