arch: s390: kernel: reset 'c->hotpluggable' when failure occurs
Subsystems (3)
PATCH 1/1 arch: s390: kernel: reset 'c->hotpluggable' when failure occurs
When smp_add_present_cpu() fails, it has reset all things excluding 'c->hotpluggable', so need reset it as original state completely.
--- arch/s390/kernel/smp.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/s390/kernel/smp.c +1 -0
@@ -1016,6 +1016,7 @@ out_cpu:unregister_cpu (c);#endif out: + c->hotpluggable =0 ;return rc; }
Signature · 1 line
1.7.7.6
[Suggestion] arch: s390: mm: the warnings with allmodconfig and "EXTRA_CFLAGS=-W"
Hello Maintainers:
When allmodconfig for " IBM zSeries model z800 and z900"
It will report the related warnings ("EXTRA_CFLAGS=-W"):
mm/slub.c:1875:1: warning: ‘deactivate_slab’ uses dynamic stack allocation [enabled by default] mm/slub.c:1941:1: warning: ‘unfreeze_partials.isra.32’ uses dynamic stack allocation [enabled by default] mm/slub.c:2575:1: warning: ‘__slab_free’ uses dynamic stack allocation [enabled by default] mm/slub.c:1582:1: warning: ‘get_partial_node.isra.34’ uses dynamic stack allocation [enabled by default] mm/slub.c:2311:1: warning: ‘__slab_alloc.constprop.42’ uses dynamic stack allocation [enabled by default]
Is it OK ?
Thanks.
--
Chen Gang
Asianux Corporation
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [Suggestion] arch: s390: mm: the warnings with allmodconfig and "EXTRA_CFLAGS=-W"
Quoted 4 lines · On Tue, 25 Jun 2013 09:54:41 +0800 Chen Gang <[email protected]> wrote:
Hello Maintainers:
When allmodconfig for " IBM zSeries model z800 and z900"
Quoted 8 lines
It will report the related warnings ("EXTRA_CFLAGS=-W"):
mm/slub.c:1875:1: warning: ‘deactivate_slab’ uses dynamic stack allocation [enabled by default]mm/slub.c:1941:1: warning: ‘unfreeze_partials.isra.32’ uses dynamic stack allocation [enabled by default]mm/slub.c:2575:1: warning: ‘__slab_free’ uses dynamic stack allocation [enabled by default]mm/slub.c:1582:1: warning: ‘get_partial_node.isra.34’ uses dynamic stack allocation [enabled by default]mm/slub.c:2311:1: warning: ‘__slab_alloc.constprop.42’ uses dynamic stack allocation [enabled by default]Is it OK ?
Yes, these warnings should be ok. They are enabled by CONFIG_WARN_DYNAMIC_STACK, the purpose is to find all functions with dynamic stack allocations. The check if the allocations are truly ok needs to be done manually as the compiler can not find out the maximum allocation size automatically.
Signature · 10 lines
blue skies, Martin. "Reality continues to ruin my life." - Calvin. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [Suggestion] arch: s390: mm: the warnings with allmodconfig and "EXTRA_CFLAGS=-W"
Quoted 14 lines · Martin Schwidefsky
Quoted 11 lines · On Tue, 25 Jun 2013 09:54:41 +0800 Chen Gang <[email protected]> wrote:
Quoted 11 lines
Hello Maintainers:
When allmodconfig for " IBM zSeries model z800 and z900"
It will report the related warnings ("EXTRA_CFLAGS=-W"):
mm/slub.c:1875:1: warning: ‘deactivate_slab’ uses dynamic stack allocation [enabled by default]mm/slub.c:1941:1: warning: ‘unfreeze_partials.isra.32’ uses dynamic stack allocation [enabled by default]mm/slub.c:2575:1: warning: ‘__slab_free’ uses dynamic stack allocation [enabled by default]mm/slub.c:1582:1: warning: ‘get_partial_node.isra.34’ uses dynamic stack allocation [enabled by default]mm/slub.c:2311:1: warning: ‘__slab_alloc.constprop.42’ uses dynamic stack allocation [enabled by default]
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Is it OK ?
Yes, these warnings should be ok. They are enabled by
CONFIG_WARN_DYNAMIC_STACK, the purpose is to find all functions with dynamic stack allocations. The check if the allocations are truly ok needs to be done manually as the compiler can not find out the maximum allocation size automatically.
Thank you very much for your details information.
Signature · 3 lines
Chen Gang Asianux Corporation
Quoted 13 lines · Chen Gang
When
smp_add_present_cpu()fails, it has reset all things excluding 'c->hotpluggable', so need reset it as original state completely.Signed-off-by Chen Gang <[email protected]>--- arch/s390/kernel/smp.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)arch/s390/kernel/smp.c +0
-0
arch/s390/kernel/smp.c out_cpu: · line 1020
Quoted 5 lines
arch/s390/kernel/smp.c +1
-0@@ -1016,6 +1016,7 @@ out_cpu:unregister_cpu (c);#endif out: + c->hotpluggable =0 ;return rc;
No, that doesn't make sense. All cpus on s390 are always hotplugable. It really doesn't matter if the value of this field is 0 or 1 after an error.
Quoted 9 lines · Heiko Carstens
Quoted 8 lines · Chen Gang
Quoted 8 lines
When
smp_add_present_cpu()fails, it has reset all things excluding 'c->hotpluggable', so need reset it as original state completely.Signed-off-by Chen Gang <[email protected]>--- arch/s390/kernel/smp.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
Quoted 14 lines
Quoted 10 lines
arch/s390/kernel/smp.c out_cpu: · line 1020
Quoted 10 lines
arch/s390/kernel/smp.c +1
-0@@ -1016,6 +1016,7 @@ out_cpu:unregister_cpu (c);#endif out: + c->hotpluggable =0 ;return rc;No, that doesn't make sense. All cpus on s390 are always hotplugable. It really doesn't matter if the value of this field is 0 or 1 after an error.
If so, is it better to set 'c->hotpluggable' for all cpus on s390 during initializing ?
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Quoted 12 lines · Chen Gang
Quoted 10 lines · Heiko Carstens
Quoted 5 lines · Chen Gang
arch/s390/kernel/smp.c out_cpu: · line 1020
Quoted 5 lines
When
smp_add_present_cpu()fails, it has reset all things excluding 'c->hotpluggable', so need reset it as original state completely.arch/s390/kernel/smp.c +1
-0+ c->hotpluggable =0 ;return rc;No, that doesn't make sense. All cpus on s390 are always hotplugable. It really doesn't matter if the value of this field is 0 or 1 after an error.
Quoted 2 lines
If so, is it better to set '
c->hotpluggable' for all cpus on s390 during initializing ?
No, just leave the code as it is.
Quoted 13 lines · Heiko Carstens
Quoted 12 lines · Chen Gang
Quoted 10 lines · Heiko Carstens
Quoted 5 lines · Chen Gang
arch/s390/kernel/smp.c out_cpu: · line 1020
Quoted 5 lines
When
smp_add_present_cpu()fails, it has reset all things excluding 'c->hotpluggable', so need reset it as original state completely.arch/s390/kernel/smp.c +1
-0+ c->hotpluggable =0 ;return rc;No, that doesn't make sense. All cpus on s390 are always hotplugable. It really doesn't matter if the value of this field is 0 or 1 after an error.
Quoted 5 lines
Quoted 2 lines
If so, is it better to set '
c->hotpluggable' for all cpus on s390 during initializing ?No, just leave the code as it is.
Currently we have 3 possible states for 'struct cpu':
initialized, and set hotplugable. unitialized, and set hotplugable. unitialized, and unset hotplugable.
Either 2nd or 3rd is inconsistent.
The cpu_is_hotplugable() will be used in kernel/rcutorture.c. I do not know whether it will cause issue or not, but it seems dangerous when __smp_rescan_cpus() has already called, and has some unsucessfull call of smp_add_present_cpu().
Please help check.
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Quoted 14 lines · Chen Gang
Quoted 13 lines · Heiko Carstens
Quoted 12 lines · Chen Gang
Quoted 10 lines · Heiko Carstens
Quoted 5 lines · Chen Gang
Review
Quoted 5 lines
When
smp_add_present_cpu()fails, it has reset all things excluding 'c->hotpluggable', so need reset it as original state completely.(quoted diff) +1
-0+ c->hotpluggable = 0; return rc;No, that doesn't make sense. All cpus on s390 are always hotplugable. It really doesn't matter if the value of this field is 0 or 1 after an error.
Quoted 6 lines
Quoted 3 lines
Quoted 2 lines
If so, is it better to set '
c->hotpluggable' for all cpus on s390 during initializing ?No, just leave the code as it is.
Quoted 1 line
Please help check the diff below whether is valuable, thanks.
For architectures which may support 'hotpluggable', can loop all cpus during subsys_initcall().
The caller will skip the return value of subsys_initcall(), so can only return 0.
--------------------------------diff begin------------------------------
arch/s390/kernel/smp.c +3 -2
@@ -1065,8 +1065,9 @@ static int __init s390_smp_init(void)#endif for_each_present_cpu (cpu) { rc =smp_add_present_cpu (cpu); -if (rc) -return rc; +if (unlikely (rc)) +printk (KERN_WARNING "%s: register_cpu %d failed (%d)\n" , + __func__, cpu, rc); }return 0 ; }
--------------------------------diff end--------------------------------
Quoted 16 lines
Currently we have 3 possible states for 'struct cpu':
initialized, and set hotplugable. unitialized, and set hotplugable. unitialized, and unset hotplugable.Either 2nd or 3rd is inconsistent.
The
cpu_is_hotplugable()will be used inkernel/rcutorture.c. I do not know whether it will cause issue or not, but it seems dangerous when__smp_rescan_cpus()has already called, and has some unsucessfull call ofsmp_add_present_cpu().Please help check.
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
PATCH v2 1/1 arch: s390: kernel: scan all present cpu forcely.
In reply to Heiko CarstensThe architectures which may support 'hotpluggable', can scan all cpus during subsys_initcall(). the upper caller will skip the return value.
It also can initialize hotpluggable flag of all cpus in time, no matter whether any cpus fail or not.
--- arch/s390/kernel/smp.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
arch/s390/kernel/smp.c +3 -2
@@ -1064,8 +1064,9 @@ static int __init s390_smp_init(void)#endif for_each_present_cpu (cpu) { rc =smp_add_present_cpu (cpu); -if (rc) -return rc; +if (unlikely (rc)) +printk (KERN_WARNING "%s: add cpu %d failed (%d)\n" , + __func__, cpu, rc); }return 0 ; }
Signature · 1 line
1.7.7.6
Re: [PATCH v2] arch: s390: kernel: scan all present cpu forcely.
Quoted 16 lines · Chen Gang
The architectures which may support 'hotpluggable', can scan all cpus during
subsys_initcall(). the upper caller will skip the return value.It also can initialize hotpluggable flag of all cpus in time, no matter whether any cpus fail or not.
Signed-off-by Chen Gang <[email protected]>--- arch/s390/kernel/smp.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)arch/s390/kernel/smp.c +0
-0
arch/s390/kernel/smp.c static int __init s390_smp_init(void) · line 1069
Quoted 8 lines
arch/s390/kernel/smp.c +3
-2@@ -1064,8 +1064,9 @@ static int __init s390_smp_init(void)#endif for_each_present_cpu (cpu) { rc =smp_add_present_cpu (cpu); -if (rc) -return rc; +if (unlikely (rc)) +printk (KERN_WARNING "%s: add cpu %d failed (%d)\n" , + __func__, cpu, rc);
I have no idea how the patch description is supposed to correlate with your patch.
However your patch doesn't make sense anyway.
We have initcall_debug for .. initcall debugging, which your patch would break in addition, since this function would now return 0 instead of the return code.
Re: [PATCH v2] arch: s390: kernel: scan all present cpu forcely.
Quoted 12 lines · Heiko Carstens
Quoted 11 lines · Chen Gang
Quoted 11 lines
The architectures which may support 'hotpluggable', can scan all cpus during
subsys_initcall(). the upper caller will skip the return value.It also can initialize hotpluggable flag of all cpus in time, no matter whether any cpus fail or not.
Signed-off-by Chen Gang <[email protected]>--- arch/s390/kernel/smp.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
arch/s390/kernel/smp.c +0
-0@@ -1064,8 +1064,9 @@ static int __init s390_smp_init(void)Quoted 8 lines
arch/s390/kernel/smp.c static int __init s390_smp_init(void) · line 1069
Quoted 8 lines
arch/s390/kernel/smp.c +3
-2#endif for_each_present_cpu (cpu) { rc =smp_add_present_cpu (cpu); -if (rc) -return rc; +if (unlikely (rc)) +printk (KERN_WARNING "%s: add cpu %d failed (%d)\n" , + __func__, cpu, rc);I have no idea how the patch description is supposed to correlate with your patch.
Pardon, excuse me, my English is not quite well.
Quoted 1 line
However your patch doesn't make sense anyway.
At least it will continue to try to "add present cpu" as much as possible.
And also make sure of all 'hotpluggable' set.
Quoted 3 lines
We have
initcall_debugfor .. initcall debugging, which your patch would break in addition, since this function would now return 0 instead of the return code.
I have searched all another architectures, most of them are only return 0 in subsys_initcall().
Do you means we do not like them ?
Thanks
Signature · 1 line
Chen Gang
PATCH 1/1 include/linux/interrupt.h: add dummy irq_set_irq_wake() for "!GENERIC_HARDIRQS"
Since irq_set_irq_wake() has already declared in header file, when GENERIC_HARDIRQS enabled.
Recommend to define the dummy one for GENERIC_HARDIRQS disabled, and also let the other related "static inline" functions are independent from GENERIC_HARDIRQS.
So can avoid the compiling error below, and also let the code simpler and clearer.
The related compiling error (ARCH=s390 allmodconfig):
sound/soc/codecs/wm0010.c: In function ‘wm0010_spi_probe’:
sound/soc/codecs/wm0010.c:976:2: error: implicit declaration of function ‘irq_set_irq_wake’ [-Werror=implicit-function-declaration]
--- include/linux/interrupt.h | 23 ++++++++++------------- 1 files changed, 10 insertions(+), 13 deletions(-)
include/linux/interrupt.h +10 -13
@@ -344,16 +344,6 @@ static inline void enable_irq_lockdep_irqrestore(unsigned int irq, unsigned long/* IRQ wakeup (PM) control: */ extern int irq_set_irq_wake (unsigned int irq,unsigned int on); -static inline int enable_irq_wake (unsigned int irq) -{ -return irq_set_irq_wake (irq,1 ); -} - -static inline int disable_irq_wake (unsigned int irq) -{ -return irq_set_irq_wake (irq,0 ); -} -#else /* !CONFIG_GENERIC_HARDIRQS */ /* * NOTE: non-genirq architectures, if they want to support the lock
@@ -370,16 +360,23 @@ static inline int disable_irq_wake(unsigned int irq) enable_irq(irq)# endif -static inline int enable_irq_wake (unsigned int irq) +/* IRQ wakeup (PM) control: */ +static inline int irq_set_irq_wake (unsigned int irq,unsigned int on) {return 0 ; } +#endif /* CONFIG_GENERIC_HARDIRQS */ + +static inline int enable_irq_wake (unsigned int irq) +{ +return irq_set_irq_wake (irq,1 ); +} +static inline int disable_irq_wake (unsigned int irq) { -return 0 ; +return irq_set_irq_wake (irq,0 ); } -#endif /* CONFIG_GENERIC_HARDIRQS */ #ifdef CONFIG_IRQ_FORCED_THREADING
Signature · 1 line
1.7.7.6