Messages in this thread | | | Subject | Re: [PATCH] perf pmu: Fix event list for uncore PMUs | From | John Garry <> | Date | Tue, 21 Dec 2021 10:14:42 +0000 |
| |
On 21/12/2021 09:35, Jiri Olsa wrote: > On Tue, Dec 21, 2021 at 09:10:37AM +0000, John Garry wrote: >> On 21/12/2021 07:58, Jiri Olsa wrote: >>>> + /* Different names -> never duplicates */ >>>> + if (strcmp(alias_a->name, alias_b->name)) >>>> + return false; >>>> + if (!alias_a->pmu) >>>> + return true; >>>> + if (!alias_b->pmu) >>>> + return true; >>> nit could be: >>> >>> if (!alias_a->pmu || !alias_b->pmu) >>> return true; >>> >>> would be great to have more comments explaining the check >>> >> >> This is just a sanity check that both strings are non-NULL as we do a >> strcmp() next. So would this be better: >> >> if (!alias_a->pmu || !alias_b->pmu || !strcmp(alias_a->pmu, alias_b->pmu)) >> return true >> >> ? >> >> It will spill a line. > > sure, it cought my eye because the is_cpu check later is done on > the same line, so I started wondering what's the difference ;-) >
Now thinking a bit more I am not confident that this patch is a full fix.
arm have heterogeneous CPU systems as well - which are not "hybrid" - and I need to ensure that aliasing is still working properly there, as I think that this following check would stop removing duplicates there:
+ /* uncore PMUs */ + if (!alias_a->is_cpu && !alias_b->is_cpu) + return true; + return false;
Thanks, John
| |