Messages in this thread | | | Date | Fri, 24 Mar 2023 11:35:08 +0000 | From | Sudeep Holla <> | Subject | Re: [PATCH] cacheinfo: Fix LLC is not exported through sysfs |
| |
On Thu, Mar 23, 2023 at 06:58:53PM +0100, Pierre Gondois wrote: > Hello Yicong, > > FWIW, I think the patch is correct and I could reproduce the issue. > > On 3/23/23 13:25, Yicong Yang wrote: > > From: Yicong Yang <yangyicong@hisilicon.com> > > > > After entering 6.3-rc1 the LLC cacheinfo is not exported on our ACPI > > based arm64 server. This is because the LLC cacheinfo is partly reset > > when secondary CPUs boot up. On arm64 the primary cpu will allocate > > and setup cacheinfo: > > init_cpu_topology() > > for_each_possible_cpu() > > fetch_cache_info() // Allocate cacheinfo and init levels > > detect_cache_attributes() > > cache_shared_cpu_map_setup() > > if (!last_level_cache_is_valid()) // not valid, setup LLC > > cache_setup_properties() // setup LLC > > > > On secondary CPU boot up: > > detect_cache_attributes() > > populate_cache_leaves() > > get_cache_type() // Get cache type from clidr_el1, > > // for LLC type=CACHE_TYPE_NOCACHE > > cache_shared_cpu_map_setup() > > if (!last_level_cache_is_valid()) // Valid and won't go to this branch, > > // leave LLC's type=CACHE_TYPE_NOCACHE > > > > The last_level_cache_is_valid() use cacheinfo->{attributes, fw_token} to > > test it's valid or not, but populate_cache_leaves() will only reset > > LLC's type, so we won't try to re-setup LLC's type and leave it > > CACHE_TYPE_NOCACHE and won't export it through sysfs. > >
IIUC this is for the case where arch register doesn't report the system level cache. I wonder if it makes sense to fix the arch callback to deal with that instead of here. I am fine either way, just checking as ideally it is something populate_cache_leaves() is messing up.
[...]
> > @@ -481,6 +488,7 @@ int detect_cache_attributes(unsigned int cpu) > > if (ret) > > goto free_ci; > > +update_cpu_map: > > Maybe just a suggestion about the code itself, > it should be possible to replace the 'goto' by an 'if' condition. > (Similarly, the 'populate_leaves:' label could have been avoided.) >
Agreed, I prefer that as well.
-- Regards, Sudeep
| |