[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f14db9e0-95c8-4389-9a27-69f0ba9b5329@arm.com>
Date: Mon, 28 Jul 2025 09:37:44 +0100
From: Ben Horgan <ben.horgan@....com>
To: James Morse <james.morse@....com>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Cc: Rob Herring <robh@...nel.org>, Rohit Mathew <rohit.mathew@....com>,
Shanker Donthineni <sdonthineni@...dia.com>, Zeng Heng
<zengheng4@...wei.com>, Lecopzer Chen <lecopzerc@...dia.com>,
Carl Worth <carl@...amperecomputing.com>,
shameerali.kolothum.thodi@...wei.com,
D Scott Phillips OS <scott@...amperecomputing.com>, lcherian@...vell.com,
bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
dfustini@...libre.com, amitsinght@...vell.com,
David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>
Subject: Re: [RFC PATCH 04/36] cacheinfo: Expose the code to generate a
cache-id from a device_node
Hi James,
On 7/25/25 18:08, James Morse wrote:
> Hi Ben,
>
> On 14/07/2025 12:40, Ben Horgan wrote:
>> On 7/11/25 19:36, James Morse wrote:
>>> 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().
>>>
>>> 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().
>
>>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>>> index 613410705a47..0fdd6358ee73 100644
>>> --- a/drivers/base/cacheinfo.c
>>> +++ b/drivers/base/cacheinfo.c
>>> @@ -207,8 +207,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)
>>> +unsigned long cache_of_calculate_id(struct device_node *cache_node)
>>> {
>>> struct device_node *cpu;
>>> u32 min_id = ~0;
>>> @@ -219,15 +218,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 ~0UL;
>>> }
>>> 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;
>
>> Looks like some 32bit/64bit confusion. Don't we want to return ~0UL if min_id == ~0?
>
> Certainly some confusion - yup, because cache_of_calculate_id() needs to return something
> that is out of range and (u32)-1 might be valid...
>
> I think changing min_id to be defined as:
> | unsigned long min_id = ~0UL;
>
> fixes this - any trip round the loop that doesn't match anything will eventually return ~0UL.
Yes, that would work.
>
>
> Thanks! - I always get the 'UL' suffixes wrong.
>
> James
Thanks,
Ben
Powered by blists - more mailing lists