On dj., d’oct. 10 2024, Alexandre Ghiti wrote:

> Hi Miquel,
>
> On 08/10/2024 15:38, Miquel Sabaté Solà wrote:
>> On dl., de set. 30 2024, Miquel Sabaté Solà wrote:
>>
>>> On dv., de set. 13 2024, Miquel Sabaté Solà wrote:
>>>
>>>> When populating cache leaves we previously fetched the CPU device node
>>>> at the very beginning. But when ACPI is enabled we go through a
>>>> specific branch which returns early and does not call 'of_node_put' for
>>>> the node that was acquired.
>>>>
>>>> Since we are not using a CPU device node for the ACPI code anyways, we
>>>> can simply move the initialization of it just passed the ACPI block, and
>>>> we are guaranteed to have an 'of_node_put' call for the acquired node.
>>>> This prevents a bad reference count of the CPU device node.
>>>>
>>>> Moreover, the previous function did not check for errors when acquiring
>>>> the device node, so a return -ENOENT has been added for that case.
>>>>
>>>> Signed-off-by: Miquel Sabaté Solà <mikisabate@gmail.com>
>>>> ---
>>>> I was wondering if this should also be sent to stable, but  I have not seen
>>>> a report on it, and this is not responsible for an oops or anything like that.
>>>> So in the end I decided not to, but maybe you consider otherwise.
>>>>
>>>>   arch/riscv/kernel/cacheinfo.c | 7 +++++--
>>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
>>>> index d6c108c50cba..d32dfdba083e 100644
>>>> --- a/arch/riscv/kernel/cacheinfo.c
>>>> +++ b/arch/riscv/kernel/cacheinfo.c
>>>> @@ -75,8 +75,7 @@ int populate_cache_leaves(unsigned int cpu)
>>>>   {
>>>>   	struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
>>>>   	struct cacheinfo *this_leaf = this_cpu_ci->info_list;
>>>> -	struct device_node *np = of_cpu_device_node_get(cpu);
>>>> -	struct device_node *prev = NULL;
>>>> +	struct device_node *np, *prev;
>>>>   	int levels = 1, level = 1;
>>>>
>>>>   	if (!acpi_disabled) {
>>>> @@ -100,6 +99,10 @@ int populate_cache_leaves(unsigned int cpu)
>>>>   		return 0;
>>>>   	}
>>>>
>>>> +	np = of_cpu_device_node_get(cpu);
>>>> +	if (!np)
>>>> +		return -ENOENT;
>>>> +
>>>>   	if (of_property_read_bool(np, "cache-size"))
>>>>   		ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
>>>>   	if (of_property_read_bool(np, "i-cache-size"))
>>> Gently ping :)
>>>
>>> Could you take a look at this fix?
>>>
>>> Thanks,
>>> Miquel
>> Hello,
>>
>> Would it make sense to have this fix for rc3?
>
>
> Sorry for the late response. It probably won't make it to rc3 but I'll make sure
> it will in rc4 :)
>
> First:
>
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>
> And it needs the following Fixes tag (but no need to send a new version, b4 will
> pick it up):
>
> Fixes: 604f32ea6909 ("riscv: cacheinfo: initialize cacheinfo's level and type
> from ACPI PPTT")
>
> And about ccing stable, I'm not sure what could be the impact of this bad
> reference count (some warnings could appear, etc...) so as it is a small patch,
> I think it's worth backporting to stable.
>
> Thanks,
>
> Alex
>
>
>>
>> Thanks,
>> Miquel
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Nice, thank you!