[PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT

Florian Fainelli posted 1 patch 2 years, 8 months ago
drivers/base/arch_topology.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
[PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Florian Fainelli 2 years, 8 months ago
From: Pierre Gondois <pierre.gondois@arm.com>

commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream

fetch_cache_info() tries to get the number of cache leaves/levels
for each CPU in order to pre-allocate memory for cacheinfo struct.
Allocating this memory later triggers a:
  'BUG: sleeping function called from invalid context'
in PREEMPT_RT kernels.

If there is no cache related information available in DT or ACPI,
fetch_cache_info() fails and an error message is printed:
  'Early cacheinfo failed, ret = ...'

Not having cache information should be a valid configuration.
Remove the error message if fetch_cache_info() fails with -ENOENT.

Suggested-by: Conor Dooley <conor.dooley@microchip.com>
Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Link: https://lore.kernel.org/r/20230414081453.244787-4-pierre.gondois@arm.com
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
Changes in v2:

- Added missing upstream commit reference
- Added missing S-o-b

 drivers/base/arch_topology.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 147fb7d4af96..b741b5ba82bd 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -843,10 +843,11 @@ void __init init_cpu_topology(void)
 
 	for_each_possible_cpu(cpu) {
 		ret = fetch_cache_info(cpu);
-		if (ret) {
+		if (!ret)
+			continue;
+		else if (ret != -ENOENT)
 			pr_err("Early cacheinfo failed, ret = %d\n", ret);
-			break;
-		}
+		return;
 	}
 }
 
-- 
2.25.1

Re: [PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Greg Kroah-Hartman 2 years, 8 months ago
On Tue, May 30, 2023 at 01:19:55PM -0700, Florian Fainelli wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream

Wait, this is already in 6.3.2, so why add it again?

totally confused,

greg k-h
Re: [PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Conor Dooley 2 years, 8 months ago
Yo Florian,

On Tue, May 30, 2023 at 01:19:55PM -0700, Florian Fainelli wrote:
> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream
> 
> fetch_cache_info() tries to get the number of cache leaves/levels
> for each CPU in order to pre-allocate memory for cacheinfo struct.
> Allocating this memory later triggers a:
>   'BUG: sleeping function called from invalid context'
> in PREEMPT_RT kernels.
> 
> If there is no cache related information available in DT or ACPI,
> fetch_cache_info() fails and an error message is printed:
>   'Early cacheinfo failed, ret = ...'
> 
> Not having cache information should be a valid configuration.
> Remove the error message if fetch_cache_info() fails with -ENOENT.
> 
> Suggested-by: Conor Dooley <conor.dooley@microchip.com>
> Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Link: https://lore.kernel.org/r/20230414081453.244787-4-pierre.gondois@arm.com
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>

How come this now needs a backport? Did the rest of the series get
backported, but not this one since it has no fixes tag?

Cheers,
Conor.
Re: [PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Florian Fainelli 2 years, 8 months ago
Hi Conor,

On 5/30/23 14:39, Conor Dooley wrote:
> Yo Florian,
> 
> On Tue, May 30, 2023 at 01:19:55PM -0700, Florian Fainelli wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream
>>
>> fetch_cache_info() tries to get the number of cache leaves/levels
>> for each CPU in order to pre-allocate memory for cacheinfo struct.
>> Allocating this memory later triggers a:
>>    'BUG: sleeping function called from invalid context'
>> in PREEMPT_RT kernels.
>>
>> If there is no cache related information available in DT or ACPI,
>> fetch_cache_info() fails and an error message is printed:
>>    'Early cacheinfo failed, ret = ...'
>>
>> Not having cache information should be a valid configuration.
>> Remove the error message if fetch_cache_info() fails with -ENOENT.
>>
>> Suggested-by: Conor Dooley <conor.dooley@microchip.com>
>> Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> Link: https://lore.kernel.org/r/20230414081453.244787-4-pierre.gondois@arm.com
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> 
> How come this now needs a backport? Did the rest of the series get
> backported, but not this one since it has no fixes tag?

Humm, indeed, this has been present in v6.3.2 since I requested it to be 
included. The error that I saw this morning was not -ENOENT, but -EINVAL.

With those patches applied, no more -EINVAL:

cacheinfo: Allow early level detection when DT/ACPI info is missing/broken
cacheinfo: Add arm64 early level initializer implementation
cacheinfo: Add arch specific early level initializer
cacheinfo: Add use_arch[|_cache]_info field/function

I will submit those shortly unless we think they better not be in 6.3, 
in which case it would be nice to silence those -EINVAL errors.
-- 
Florian

Re: [PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Sudeep Holla 2 years, 8 months ago
On Tue, May 30, 2023 at 03:42:45PM -0700, Florian Fainelli wrote:
> Hi Conor,
> 
> On 5/30/23 14:39, Conor Dooley wrote:
> > Yo Florian,
> > 
> > On Tue, May 30, 2023 at 01:19:55PM -0700, Florian Fainelli wrote:
> > > From: Pierre Gondois <pierre.gondois@arm.com>
> > > 
> > > commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream
> > > 
> > > fetch_cache_info() tries to get the number of cache leaves/levels
> > > for each CPU in order to pre-allocate memory for cacheinfo struct.
> > > Allocating this memory later triggers a:
> > >    'BUG: sleeping function called from invalid context'
> > > in PREEMPT_RT kernels.
> > > 
> > > If there is no cache related information available in DT or ACPI,
> > > fetch_cache_info() fails and an error message is printed:
> > >    'Early cacheinfo failed, ret = ...'
> > > 
> > > Not having cache information should be a valid configuration.
> > > Remove the error message if fetch_cache_info() fails with -ENOENT.
> > > 
> > > Suggested-by: Conor Dooley <conor.dooley@microchip.com>
> > > Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
> > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > Link: https://lore.kernel.org/r/20230414081453.244787-4-pierre.gondois@arm.com
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> > 
> > How come this now needs a backport? Did the rest of the series get
> > backported, but not this one since it has no fixes tag?
> 
> Humm, indeed, this has been present in v6.3.2 since I requested it to be
> included. The error that I saw this morning was not -ENOENT, but -EINVAL.
> 
> With those patches applied, no more -EINVAL:
> 
> cacheinfo: Allow early level detection when DT/ACPI info is missing/broken
> cacheinfo: Add arm64 early level initializer implementation
> cacheinfo: Add arch specific early level initializer
> cacheinfo: Add use_arch[|_cache]_info field/function
> 
> I will submit those shortly unless we think they better not be in 6.3, in
> which case it would be nice to silence those -EINVAL errors.

I prefer this option instead of back porting all the above 4 as there are
some pending fixes for the issues found in those patches. I am fine if Greg
is happy with the backport, so no strong rejection from my side :).

-- 
Regards,
Sudeep
Re: [PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Florian Fainelli 2 years, 8 months ago

On 5/31/2023 1:53 AM, Sudeep Holla wrote:
> On Tue, May 30, 2023 at 03:42:45PM -0700, Florian Fainelli wrote:
>> Hi Conor,
>>
>> On 5/30/23 14:39, Conor Dooley wrote:
>>> Yo Florian,
>>>
>>> On Tue, May 30, 2023 at 01:19:55PM -0700, Florian Fainelli wrote:
>>>> From: Pierre Gondois <pierre.gondois@arm.com>
>>>>
>>>> commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream
>>>>
>>>> fetch_cache_info() tries to get the number of cache leaves/levels
>>>> for each CPU in order to pre-allocate memory for cacheinfo struct.
>>>> Allocating this memory later triggers a:
>>>>     'BUG: sleeping function called from invalid context'
>>>> in PREEMPT_RT kernels.
>>>>
>>>> If there is no cache related information available in DT or ACPI,
>>>> fetch_cache_info() fails and an error message is printed:
>>>>     'Early cacheinfo failed, ret = ...'
>>>>
>>>> Not having cache information should be a valid configuration.
>>>> Remove the error message if fetch_cache_info() fails with -ENOENT.
>>>>
>>>> Suggested-by: Conor Dooley <conor.dooley@microchip.com>
>>>> Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
>>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>>> Link: https://lore.kernel.org/r/20230414081453.244787-4-pierre.gondois@arm.com
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>>>
>>> How come this now needs a backport? Did the rest of the series get
>>> backported, but not this one since it has no fixes tag?
>>
>> Humm, indeed, this has been present in v6.3.2 since I requested it to be
>> included. The error that I saw this morning was not -ENOENT, but -EINVAL.
>>
>> With those patches applied, no more -EINVAL:
>>
>> cacheinfo: Allow early level detection when DT/ACPI info is missing/broken
>> cacheinfo: Add arm64 early level initializer implementation
>> cacheinfo: Add arch specific early level initializer
>> cacheinfo: Add use_arch[|_cache]_info field/function
>>
>> I will submit those shortly unless we think they better not be in 6.3, in
>> which case it would be nice to silence those -EINVAL errors.
> 
> I prefer this option instead of back porting all the above 4 as there are
> some pending fixes for the issues found in those patches. I am fine if Greg
> is happy with the backport, so no strong rejection from my side :).

OK, so are you suggesting that we specific check for -EINVAL and -ENOENT 
rather than take all of the 4 above patches, if so, any preference on 
how to do it given the state of 6.3 stable?
-- 
Florian
Re: [PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Sudeep Holla 2 years, 8 months ago
On Wed, May 31, 2023 at 08:28:26AM -0700, Florian Fainelli wrote:
> 
> 
> On 5/31/2023 1:53 AM, Sudeep Holla wrote:
> > On Tue, May 30, 2023 at 03:42:45PM -0700, Florian Fainelli wrote:
> > > Hi Conor,
> > > 
> > > On 5/30/23 14:39, Conor Dooley wrote:
> > > > Yo Florian,
> > > > 
> > > > On Tue, May 30, 2023 at 01:19:55PM -0700, Florian Fainelli wrote:
> > > > > From: Pierre Gondois <pierre.gondois@arm.com>
> > > > > 
> > > > > commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream
> > > > > 
> > > > > fetch_cache_info() tries to get the number of cache leaves/levels
> > > > > for each CPU in order to pre-allocate memory for cacheinfo struct.
> > > > > Allocating this memory later triggers a:
> > > > >     'BUG: sleeping function called from invalid context'
> > > > > in PREEMPT_RT kernels.
> > > > > 
> > > > > If there is no cache related information available in DT or ACPI,
> > > > > fetch_cache_info() fails and an error message is printed:
> > > > >     'Early cacheinfo failed, ret = ...'
> > > > > 
> > > > > Not having cache information should be a valid configuration.
> > > > > Remove the error message if fetch_cache_info() fails with -ENOENT.
> > > > > 
> > > > > Suggested-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
> > > > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > Link: https://lore.kernel.org/r/20230414081453.244787-4-pierre.gondois@arm.com
> > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > > Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> > > > 
> > > > How come this now needs a backport? Did the rest of the series get
> > > > backported, but not this one since it has no fixes tag?
> > > 
> > > Humm, indeed, this has been present in v6.3.2 since I requested it to be
> > > included. The error that I saw this morning was not -ENOENT, but -EINVAL.
> > > 
> > > With those patches applied, no more -EINVAL:
> > > 
> > > cacheinfo: Allow early level detection when DT/ACPI info is missing/broken
> > > cacheinfo: Add arm64 early level initializer implementation
> > > cacheinfo: Add arch specific early level initializer
> > > cacheinfo: Add use_arch[|_cache]_info field/function
> > > 
> > > I will submit those shortly unless we think they better not be in 6.3, in
> > > which case it would be nice to silence those -EINVAL errors.
> > 
> > I prefer this option instead of back porting all the above 4 as there are
> > some pending fixes for the issues found in those patches. I am fine if Greg
> > is happy with the backport, so no strong rejection from my side :).
> 
> OK, so are you suggesting that we specific check for -EINVAL and -ENOENT
> rather than take all of the 4 above patches,

Yes that is my preference ATM or if possible to wait until all the fixes
are sorted for the bugs associated with above 4 commits [1] and [2].
I have queued [1] but waiting for response/patch on [2] and hence not yet
bothered Greg.

> if so, any preference on how to do it given the state of 6.3 stable?

I don't understand what exactly do you mean ?

--
Regards,
Sudeep

[1] https://lore.kernel.org/all/20230508084115.1157-1-kprateek.nayak@amd.com
[2] https://lore.kernel.org/all/20230518012703.GA19967@ranerica-svr.sc.intel.com
Re: [PATCH stable 6.3 v2] arch_topology: Remove early cacheinfo error message if -ENOENT
Posted by Conor Dooley 2 years, 8 months ago
On Wed, May 31, 2023 at 09:53:56AM +0100, Sudeep Holla wrote:
> On Tue, May 30, 2023 at 03:42:45PM -0700, Florian Fainelli wrote:
> > Hi Conor,
> > 
> > On 5/30/23 14:39, Conor Dooley wrote:
> > > Yo Florian,
> > > 
> > > On Tue, May 30, 2023 at 01:19:55PM -0700, Florian Fainelli wrote:
> > > > From: Pierre Gondois <pierre.gondois@arm.com>
> > > > 
> > > > commit 3522340199cc060b70f0094e3039bdb43c3f6ee1 upstream
> > > > 
> > > > fetch_cache_info() tries to get the number of cache leaves/levels
> > > > for each CPU in order to pre-allocate memory for cacheinfo struct.
> > > > Allocating this memory later triggers a:
> > > >    'BUG: sleeping function called from invalid context'
> > > > in PREEMPT_RT kernels.
> > > > 
> > > > If there is no cache related information available in DT or ACPI,
> > > > fetch_cache_info() fails and an error message is printed:
> > > >    'Early cacheinfo failed, ret = ...'
> > > > 
> > > > Not having cache information should be a valid configuration.
> > > > Remove the error message if fetch_cache_info() fails with -ENOENT.
> > > > 
> > > > Suggested-by: Conor Dooley <conor.dooley@microchip.com>
> > > > Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
> > > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > > Link: https://lore.kernel.org/r/20230414081453.244787-4-pierre.gondois@arm.com
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> > > 
> > > How come this now needs a backport? Did the rest of the series get
> > > backported, but not this one since it has no fixes tag?
> > 
> > Humm, indeed, this has been present in v6.3.2 since I requested it to be
> > included. The error that I saw this morning was not -ENOENT, but -EINVAL.
> > 
> > With those patches applied, no more -EINVAL:
> > 
> > cacheinfo: Allow early level detection when DT/ACPI info is missing/broken
> > cacheinfo: Add arm64 early level initializer implementation
> > cacheinfo: Add arch specific early level initializer
> > cacheinfo: Add use_arch[|_cache]_info field/function
> > 
> > I will submit those shortly unless we think they better not be in 6.3, in
> > which case it would be nice to silence those -EINVAL errors.
> 
> I prefer this option instead of back porting all the above 4 as there are
> some pending fixes for the issues found in those patches. I am fine if Greg
> is happy with the backport, so no strong rejection from my side :).

Just to be clear, I was not objecting, just curious!