Re: [PATCH 2/3] powerpc/cpuhp: BUG -> WARN conversion in offline path

2021-09-20 Thread Daniel Henrique Barboza




On 9/20/21 10:55, Nathan Lynch wrote:

If, due to bugs elsewhere, we get into unregister_cpu_online() with a CPU
that isn't marked hotpluggable, we can emit a warning and return an
appropriate error instead of crashing.

Signed-off-by: Nathan Lynch 
---


As mentioned by Christophe this will not solve the crash for kernels with
panic_on_warn, but at least it will panic with a clearer message on those
and will not panic for everyone else.


Reviewed-by: Daniel Henrique Barboza 




  arch/powerpc/kernel/sysfs.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index defecb3b1b15..08d8072d6e7a 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -928,7 +928,8 @@ static int unregister_cpu_online(unsigned int cpu)
struct device_attribute *attrs, *pmc_attrs;
int i, nattrs;
  
-	BUG_ON(!c->hotpluggable);

+   if (WARN_RATELIMIT(!c->hotpluggable, "cpu %d can't be offlined\n", cpu))
+   return -EBUSY;
  
  #ifdef CONFIG_PPC64

if (cpu_has_feature(CPU_FTR_SMT))



Re: [PATCH 2/3] powerpc/cpuhp: BUG -> WARN conversion in offline path

2021-09-20 Thread Nathan Lynch
Christophe Leroy  writes:

> Le 20/09/2021 à 15:55, Nathan Lynch a écrit :
>> If, due to bugs elsewhere, we get into unregister_cpu_online() with a CPU
>> that isn't marked hotpluggable, we can emit a warning and return an
>> appropriate error instead of crashing.
>
> Is it only a bug situation, or is it something that can happen in real 
> life ?
>
> If it can happen in real life, kernels with panic_on_warn will still be 
> impacted.

I only found this by inspection, and it can happen only due to a bug in
CPU device registration at boot. The flag must not be set if the
platform or CPU can't support going offline.


Re: [PATCH 2/3] powerpc/cpuhp: BUG -> WARN conversion in offline path

2021-09-20 Thread Christophe Leroy




Le 20/09/2021 à 15:55, Nathan Lynch a écrit :

If, due to bugs elsewhere, we get into unregister_cpu_online() with a CPU
that isn't marked hotpluggable, we can emit a warning and return an
appropriate error instead of crashing.


Is it only a bug situation, or is it something that can happen in real 
life ?


If it can happen in real life, kernels with panic_on_warn will still be 
impacted.




Signed-off-by: Nathan Lynch 
---
  arch/powerpc/kernel/sysfs.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index defecb3b1b15..08d8072d6e7a 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -928,7 +928,8 @@ static int unregister_cpu_online(unsigned int cpu)
struct device_attribute *attrs, *pmc_attrs;
int i, nattrs;
  
-	BUG_ON(!c->hotpluggable);

+   if (WARN_RATELIMIT(!c->hotpluggable, "cpu %d can't be offlined\n", cpu))
+   return -EBUSY;
  
  #ifdef CONFIG_PPC64

if (cpu_has_feature(CPU_FTR_SMT))



[PATCH 2/3] powerpc/cpuhp: BUG -> WARN conversion in offline path

2021-09-20 Thread Nathan Lynch
If, due to bugs elsewhere, we get into unregister_cpu_online() with a CPU
that isn't marked hotpluggable, we can emit a warning and return an
appropriate error instead of crashing.

Signed-off-by: Nathan Lynch 
---
 arch/powerpc/kernel/sysfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index defecb3b1b15..08d8072d6e7a 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -928,7 +928,8 @@ static int unregister_cpu_online(unsigned int cpu)
struct device_attribute *attrs, *pmc_attrs;
int i, nattrs;
 
-   BUG_ON(!c->hotpluggable);
+   if (WARN_RATELIMIT(!c->hotpluggable, "cpu %d can't be offlined\n", cpu))
+   return -EBUSY;
 
 #ifdef CONFIG_PPC64
if (cpu_has_feature(CPU_FTR_SMT))
-- 
2.31.1