lkml.org 
[lkml]   [2023]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] cacheinfo: Fix LLC is not exported through sysfs
From
Date
Hi Pierre and Sudeep,

On 2023/3/24 19:35, Sudeep Holla wrote:
> 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.
>

yes it's right, the LLC information is not provided by the CPU register and can
only be retrieved from PPTT on my machine. Maybe fix the issue first, I don't
know how to make arch callback handle this since arch_topology is also used
other than arm64 which I'm not familiar with.

> [...]
>
>>> @@ -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.
>

ok, will modify as below with a little refactor to get rid of the
'populate_leaves:' label as suggested.

Thanks,
Yicong

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index f6573c335f4c..e34e6b77e81a 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -462,24 +462,28 @@ int detect_cache_attributes(unsigned int cpu)
* as it will happen only once (the cacheinfo memory is never freed).
* Just populate the cacheinfo.
*/
- if (per_cpu_cacheinfo(cpu))
- goto populate_leaves;
-
- if (init_cache_level(cpu) || !cache_leaves(cpu))
- return -ENOENT;
+ if (!per_cpu_cacheinfo(cpu)) {
+ if (init_cache_level(cpu) || !cache_leaves(cpu))
+ return -ENOENT;

- ret = allocate_cache_info(cpu);
- if (ret)
- return ret;
+ ret = allocate_cache_info(cpu);
+ if (ret)
+ return ret;
+ }

-populate_leaves:
/*
- * populate_cache_leaves() may completely setup the cache leaves and
- * shared_cpu_map or it may leave it partially setup.
+ * If LLC is valid the cache leaves were already populated so just go to
+ * update the cpu map.
*/
- ret = populate_cache_leaves(cpu);
- if (ret)
- goto free_ci;
+ if (!last_level_cache_is_valid(cpu)) {
+ /*
+ * populate_cache_leaves() may completely setup the cache leaves and
+ * shared_cpu_map or it may leave it partially setup.
+ */
+ ret = populate_cache_leaves(cpu);
+ if (ret)
+ goto free_ci;
+ }

/*
* For systems using DT for cache hierarchy, fw_token
\
 
 \ /
  Last update: 2023-03-27 08:58    [W:0.073 / U:0.892 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site