Messages in this thread | | | From | Song Liu <> | Subject | Re: [PATCH v2 1/1] perf: Sharing PMU counters across compatible events | Date | Tue, 11 Sep 2018 13:29:32 +0000 |
| |
> On Sep 10, 2018, at 1:13 AM, Jiri Olsa <jolsa@redhat.com> wrote: > > On Thu, Aug 30, 2018 at 06:35:37PM +0000, Song Liu wrote: >> >> >>> On Aug 30, 2018, at 8:13 AM, Jiri Olsa <jolsa@redhat.com> wrote: >>> >>> On Wed, Aug 15, 2018 at 10:03:13AM -0700, Song Liu wrote: >>> >>> SNIP >>> >>>> >>>> + perf_event_remove_dup(event, ctx); >>>> /* >>>> * We can have double detach due to exit/hot-unplug + close. >>>> */ >>>> @@ -1982,6 +2123,92 @@ event_filter_match(struct perf_event *event) >>>> perf_cgroup_match(event) && pmu_filter_match(event); >>>> } >>>> >>>> +/* PMU sharing aware version of event->pmu->add() */ >>>> +static int event_pmu_add(struct perf_event *event, >>>> + struct perf_event_context *ctx) >>>> +{ >>>> + struct perf_event_dup *dup; >>>> + int ret; >>>> + >>>> + /* no sharing, just do event->pmu->add() */ >>>> + if (event->dup_id == -1) >>>> + return event->pmu->add(event, PERF_EF_START); >>>> + >>>> + dup = &ctx->dup_events[event->dup_id]; >>>> + >>>> + if (dup->active_event_count) { >>>> + /* already enabled */ >>>> + dup->active_event_count++; >>>> + dup->master->pmu->read(dup->master); >>>> + event->dup_base_count = dup_read_count(dup); >>>> + event->dup_base_child_count = dup_read_child_count(dup); >>>> + return 0; >>>> + } >>>> + >>>> + /* try add master */ >>>> + ret = event->pmu->add(dup->master, PERF_EF_START); >>>> + >>>> + if (!ret) { >>>> + dup->active_event_count = 1; >>>> + event->pmu->read(dup->master); >>>> + event->dup_base_count = dup_read_count(dup); >>>> + event->dup_base_child_count = dup_read_child_count(dup); >>> >>> should you read the base before calling pmu->add ? >>> should be same for any dup event not just master >>> >>> jirka >> >> I am not sure I am following. The pmu is disabled when we call >> event_pmu_add(). Why do we need to read before calling pmu->add()? >> And this is the first added dup event for this master, so we don't >> need to worry about others. >> >> Does this make sense? > > I was just thinking since the pmu is disable we could > we don't need to read the event on 2 places.. it's almost > identic code
How about something like:
+/* PMU sharing aware version of event->pmu->add() */ +static int event_pmu_add(struct perf_event *event, + struct perf_event_context *ctx) +{ + struct perf_event_dup *dup; + int ret; + + /* no sharing, just do event->pmu->add() */ + if (event->dup_id == -1) + return event->pmu->add(event, PERF_EF_START); + + dup = &ctx->dup_events[event->dup_id]; + + if (dup->active_event_count = 0) { + /* try add master */ + ret = event->pmu->add(dup->master, PERF_EF_START); + if (ret) + return ret; + } + + dup->active_event_count++; + event->pmu->read(dup->master); + event->dup_base_count = dup_read_count(dup); + event->dup_base_child_count = dup_read_child_count(dup); + + return 0; +}
| |