Re: [PATCH RFC v2 04/10] cacheinfo: Expose the code to generate a cache-id from a device_node
From: Yin Li
Date: Tue Sep 15 2026 - 22:29:57 EST
On 9/15/2026 3:59 PM, Andre Przywara wrote:
Hi Yin,
thanks for the reply!
On 9/15/26 08:49, Yin Li wrote:
On 9/14/2026 8:26 PM, Andre Przywara wrote:
Hi,
On 9/14/26 11:37, Yin Li wrote:
From: James Morse <james.morse@xxxxxxx>
The MPAM driver identifies caches by id for use with resctrl. It
needs to know the cache-id when probe-ing, but the value isn't set
in cacheinfo until device_initcall(). Even after device_initcall(),
the cache-id is only available if at least one CPU associated with
the cache is online.
Instead of making the driver wait, expose the code that generates the
cache-id. The parts of the MPAM driver that run early can use this to
set up the resctrl structures before cacheinfo is ready in
device_initcall().
Signed-off-by: James Morse <james.morse@xxxxxxx>
[ Yin Li: fix context conflicts in cacheinfo.c and cacheinfo.h; guard the
cache_of_calculate_id() declaration with CONFIG_OF to prevent build
errors when CONFIG_OF is not set ]
You can shorten that part in square brackets: doing adjustments due to rebasing is surely implied, and you can shorten the rest, like:
[ Yin Li: guard cache_of_calculate_id() prototype ]
Speaking of which ...
Signed-off-by: Yin Li <yin.li@xxxxxxxxxxxxxxxx>
---
drivers/base/cacheinfo.c | 17 ++++++++++++-----
include/linux/cacheinfo.h | 3 +++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 9f9c72727a05..f75e7f64038b 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -226,8 +226,7 @@ static bool match_cache_node(struct device_node *cpu,
#define arch_compact_of_hwid(_x) (_x)
#endif
-static void cache_of_set_id(struct cacheinfo *this_leaf,
- struct device_node *cache_node)
+u32 cache_of_calculate_id(struct device_node *cache_node)
{
struct device_node *cpu;
u32 min_id = ~0;
@@ -238,15 +237,23 @@ static void cache_of_set_id(struct cacheinfo *this_leaf,
id = arch_compact_of_hwid(id);
if (FIELD_GET(GENMASK_ULL(63, 32), id)) {
of_node_put(cpu);
- return;
+ return ~0;
}
if (match_cache_node(cpu, cache_node))
min_id = min(min_id, id);
}
- if (min_id != ~0) {
- this_leaf->id = min_id;
+ return min_id;
+}
+
+static void cache_of_set_id(struct cacheinfo *this_leaf,
+ struct device_node *cache_node)
+{
+ u32 id = cache_of_calculate_id(cache_node);
+
+ if (id != ~0) {
+ this_leaf->id = id;
this_leaf->attributes |= CACHE_ID;
}
}
diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h
index fc879ac4cc4f..c33bb3c8bd63 100644
--- a/include/linux/cacheinfo.h
+++ b/include/linux/cacheinfo.h
@@ -113,6 +113,9 @@ int acpi_get_cache_info(unsigned int cpu,
#endif
const struct attribute_group *cache_get_priv_group(struct cacheinfo *this_leaf);
+#ifdef CONFIG_OF
Why is that, exactly? First IIUC it's quite uncommon to use #ifdef guards around prototypes (unless they are stubbed without the symbol defined). Using types protected by those symbols if certainly another reason, and it looks like this would be the case here, but I had no trouble building the kernel for x86, where CONFIG_OF is not defined.
So can you share a .config example (or give a hint) as to where this fails building?
And if it does, wouldn't it be better to always include <linux/of.h> in that file instead? I think I see a similar pattern elsewhere (rfkill- gpio.c, sound/ac97/bus.c).
Hi Andre,
On the #ifdef CONFIG_OF: I also verified that removing the guard doesn't
break the build. However, since the implementation in cacheinfo.c is
itself guarded by #ifdef CONFIG_OF, exposing the prototype
unconditionally could cause a link error on CONFIG_OF=n builds if called
How so? Just exposing a prototype wouldn't be a problem, as long as you don't try to call that function. And that would be caught by the linker, and then you have a different problem anyway (the caller).
The only reason to protect the prototype would be if a type used in the parameters is not defined. And on the face of it "struct device_node" is an OF specific type, declared in include/linux/of.h, but as mentioned, I can't produce a compiler error, and even if so, would prefer to include of.h instead.
Hi Andre,
Thanks for the review.
I'll drop the #ifdef guard in the next version.
Just to clarify: MPAM depends on ARM64, which selects OF, so CONFIG_OF is always set in any configuration that enables MPAM. I had kept the guard as a precaution for potential callers outside that dependency chain, though I take your point that this would be a caller-side issue rather than something to address in the header.
from outside the ARM64/MPAM path. A more idiomatic approach might be to
use a stub to keep the header consistent with the implementation:
#ifdef CONFIG_OF
u32 cache_of_calculate_id(struct device_node *np);
#else
static inline u32 cache_of_calculate_id(struct device_node *np)
{
return ~0U;
}
#endif
Would that work for you?
That's not necessary and doesn't solve the problem: the struct device_node would be present in both branches, so that doesn't help. And given there are no preprocessor protections for OF or ACPI in the whole of mpam_devices.c, that's a non-issue, I'd say.
Otherwise I'm happy to just drop the guard in
the next version.
So can you say whether you have a .config that does not build? Or was that issue just pointed out by some picky AI review tool?
Otherwise I would drop the guards, and wait for the kernel test robot or Arnd's infamous randconfig builds to show up the exact problem.
For context: I did experiment with wrapping all the OF-specific code in
mpam_devices.c with #ifdef CONFIG_OF, but dropped it for the same reason
— the MPAM driver depends on ARM64 which selects OF. I then kept only the header guard as a precaution for potential external users, but as you point out, that's not necessary either.
Thanks,
Yin
Cheers,
Andre
+u32 cache_of_calculate_id(struct device_node *np);
+#endif
/*
* Get the cacheinfo structure for the cache associated with @cpu at
--
Thx and BRs,
Yin