Messages in this thread Patch in this message | | | Date | Wed, 12 Apr 2023 12:27:20 +0100 | From | Conor Dooley <> | Subject | Re: [PATCH v2 1/3] cacheinfo: Check sib_leaf in cache_leaves_are_shared() |
| |
On Wed, Apr 12, 2023 at 09:18:04AM +0200, Pierre Gondois wrote: > If 'this_leaf' is a L2 cache (or higher) and 'sib_leaf' is a L1 cache, > the caches are detected as shared. Indeed, cache_leaves_are_shared() > only checks the cache level of 'this_leaf' when 'sib_leaf''s cache > level should also be checked.
I have to say, I'm a wee bit confused reading this patch - although it's likely that I have just confused myself here.
The comment reads "For non DT/ACPI systems, assume unique level 1 caches, system-wide shared caches for all other levels". Does this mean all level 1 caches are unique & all level N caches are shared with all other level N caches, but not with level M caches? (M != N; M, N > 1)
Is this patches goal to make sure that if this_leaf is level 2 and sib_leaf is level 1 that these are not detected as shared, since level one caches are meant to be unique?
The previous logic checked only this_leaf's level, and declared things shared if this_leaf is not a level 1 cache. What happens here if this_leaf->level == 1 and sib_leaf->level == 2? That'll be detected as shared, in a contradiction of the comment above it, no?
As you never state the actual problem with the current code, I'm not entirely sure if I am making a fool of myself or not here.
Probably making a fool, that's par for the course ;)
Thanks, Conor.
> > Check 'sib_leaf->level'. Also update the comment as the function is > called when populating 'shared_cpu_map'. > > Fixes: f16d1becf96f ("cacheinfo: Use cache identifiers to check if the caches are shared if available") > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/base/cacheinfo.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c > index f3903d002819..e7ad6aba5f97 100644 > --- a/drivers/base/cacheinfo.c > +++ b/drivers/base/cacheinfo.c > @@ -38,11 +38,10 @@ static inline bool cache_leaves_are_shared(struct cacheinfo *this_leaf, > { > /* > * For non DT/ACPI systems, assume unique level 1 caches, > - * system-wide shared caches for all other levels. This will be used > - * only if arch specific code has not populated shared_cpu_map > + * system-wide shared caches for all other levels. > */ > if (!(IS_ENABLED(CONFIG_OF) || IS_ENABLED(CONFIG_ACPI))) > - return !(this_leaf->level == 1); > + return (this_leaf->level != 1) || (sib_leaf->level != 1); > > if ((sib_leaf->attributes & CACHE_ID) && > (this_leaf->attributes & CACHE_ID)) > -- > 2.25.1 > [unhandled content-type:application/pgp-signature] | |