Messages in this thread | | | Date | Tue, 12 Apr 2022 09:58:39 +0100 | From | Sudeep Holla <> | Subject | Re: [PATCH] arch_topology: Do not set llc_sibling if llc_id is invalid |
| |
On Mon, Apr 11, 2022 at 06:07:45PM +0300, Andy Shevchenko wrote: > On Mon, Apr 11, 2022 at 12:10 PM Qing Wang <wangqing@vivo.com> wrote: > > > > From: Wang Qing <wangqing@vivo.com> > > > > When ACPI is not enabled, cpuid_topo->llc_id = cpu_topo->llc_id = -1, which > > will set llc_sibling 0xff(...), this is misleading. > > Shouldn't it be a Fixes tag then? >
I thought about that. One the file has moved and lot of refactoring before the move after the code was first introduced. Since no one has seen any issues as the mask matches all the CPUs on a single chip SoC and this is not user visible, I didn't push for the fixes tag.
Anyways the original commit introducing the feature is Commit 37c3ec2d810f ("arm64: topology: divorce MC scheduling domain from core_siblings") which was merged in v4.18 if I read git log correctly.
I am happy to backport if needed.
> > Don't set llc_sibling(default 0) if we don't know the cache topology. > > ... > > > - if (cpuid_topo->llc_id == cpu_topo->llc_id) { > > + if (cpu_topo->llc_id != -1 && cpuid_topo->llc_id == cpu_topo->llc_id) { > > I'm wondering if more strict check is better here, i.e. > > if (cpu_topo->llc_id >= 0 && cpuid_topo->llc_id == > cpu_topo->llc_id) { >
Yes I would agree. But I think Qing is just following other similar checks in the file. All such ids are initialised to -1 and are assigned only valid values >= 0. I am OK to keep it as is to keep it aligned with other similar checks.
-- Regards, Sudeep
| |