Re: [PATCH 2/5] libperf: Propagate maps only if necessary

[Namhyung Kim]

Hi Adrian,

On Tue, Sep 27, 2022 at 12:06 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>
> On 24/09/22 19:57, Namhyung Kim wrote:
> > The current code propagate evsel's cpu map settings to evlist when it's
> > added to an evlist.  But the evlist->all_cpus and each evsel's cpus will
> > be updated in perf_evlist__set_maps() later.  No need to do it before
> > evlist's cpus are set actually.
> >
> > Actually we discarded this intermediate all_cpus maps at the beginning
> > of perf_evlist__set_maps().  Let's not do this.  It's only needed when
> > an evsel is added after the evlist cpu maps are set.
>
> That might not be true.  Consider evlist__fix_hybrid_cpus() which fiddles
> with evsel->core.cpus and evsel->core.own_cpus after the evsel has been
> added to the evlist.  It can also remove an evsel from the evlist.

Thanks for your review.  I think it's fine to change evsel cpus or to remove
an evsel from evlist before calling evlist__create_maps().  The function
will take care of setting evlist's all_cpus from the evsels in the evlist.
So previous changes in evsel/cpus wouldn't be any special.

After this point, adding a new evsel needs to update evlist all cpus by
propagating cpu maps.  So I think hybrid cpus should be fine.
Did I miss something?

>
> There might be other cases like that, but that was just one that stuck
> out.

Thanks for sharing your concern.  Please let me know if you could
come up with another.

Namhyung