On Thu, Aug 30, 2018 at 06:35:37PM +0000, Song Liu wrote:
>
>
> > On Aug 30, 2018, at 8:13 AM, Jiri Olsa <jo...@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
jirka
