[PATCH] arch: s390: kernel: reset 'c->hotpluggable' when failure occurs

[PATCH] arch: s390: kernel: reset 'c->hotpluggable' when failure occurs

[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 <gang.chen@asianux.com>
---
 arch/s390/kernel/smp.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index 15a016c..c4c6f42 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -1016,6 +1016,7 @@ out_cpu:
 	unregister_cpu(c);
 #endif
 out:
+	c->hotpluggable = 0;
 	return rc;
 }
 
-- 
1.7.7.6

[Suggestion] arch: s390: mm: the warnings with allmodconfig and "EXTRA_CFLAGS=-W"

[Chen Gang]

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 

Re: [Suggestion] arch: s390: mm: the warnings with allmodconfig and "EXTRA_CFLAGS=-W"

[Martin Schwidefsky]

On Tue, 25 Jun 2013 09:54:41 +0800
Chen Gang <gang.chen@asianux.com> wrote:

> 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 ?

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.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [Suggestion] arch: s390: mm: the warnings with allmodconfig and "EXTRA_CFLAGS=-W"

[Chen Gang]

On 06/25/2013 02:50 PM, Martin Schwidefsky wrote:
> On Tue, 25 Jun 2013 09:54:41 +0800
> Chen Gang <gang.chen@asianux.com> wrote:
> 
>> > 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 ?
> 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.

-- 
Chen Gang

Asianux Corporation

Re: [PATCH] arch: s390: kernel: reset 'c->hotpluggable' when failure occurs

[Heiko Carstens]

On Tue, Jun 25, 2013 at 09:46:45AM +0800, Chen Gang wrote:
> 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 <gang.chen@asianux.com>
> ---
>  arch/s390/kernel/smp.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
> index 15a016c..c4c6f42 100644
> --- a/arch/s390/kernel/smp.c
> +++ b/arch/s390/kernel/smp.c
> @@ -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.

Re: [PATCH] arch: s390: kernel: reset 'c->hotpluggable' when failure occurs

[Chen Gang]

On 06/25/2013 02:48 PM, Heiko Carstens wrote:
> On Tue, Jun 25, 2013 at 09:46:45AM +0800, Chen Gang wrote:
>> > 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 <gang.chen@asianux.com>
>> > ---
>> >  arch/s390/kernel/smp.c |    1 +
>> >  1 files changed, 1 insertions(+), 0 deletions(-)
>> > 
>> > diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
>> > index 15a016c..c4c6f42 100644
>> > --- a/arch/s390/kernel/smp.c
>> > +++ b/arch/s390/kernel/smp.c
>> > @@ -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.
-- 
Chen Gang

Asianux Corporation

Re: [PATCH] arch: s390: kernel: reset 'c->hotpluggable' when failure occurs

[Heiko Carstens]

On Tue, Jun 25, 2013 at 03:24:09PM +0800, Chen Gang wrote:
> On 06/25/2013 02:48 PM, Heiko Carstens wrote:
> > On Tue, Jun 25, 2013 at 09:46:45AM +0800, Chen Gang wrote:
> >> > When smp_add_present_cpu() fails, it has reset all things excluding
> >> > 'c->hotpluggable', so need reset it as original state completely.
> >> > 
> >> > +	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 ?

No, just leave the code as it is.

Re: [PATCH] arch: s390: kernel: reset 'c->hotpluggable' when failure occurs

[Chen Gang]

On 06/25/2013 05:09 PM, Heiko Carstens wrote:
> On Tue, Jun 25, 2013 at 03:24:09PM +0800, Chen Gang wrote:
>> On 06/25/2013 02:48 PM, Heiko Carstens wrote:
>>> On Tue, Jun 25, 2013 at 09:46:45AM +0800, Chen Gang wrote:
>>>>> When smp_add_present_cpu() fails, it has reset all things excluding
>>>>> 'c->hotpluggable', so need reset it as original state completely.
>>>>>
>>>>> +	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 ?
> 
> 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.
-- 
Chen Gang

Asianux Corporation

Re: [PATCH] arch: s390: kernel: reset 'c->hotpluggable' when failure occurs

[Chen Gang]

On 06/25/2013 05:31 PM, Chen Gang wrote:
> On 06/25/2013 05:09 PM, Heiko Carstens wrote:
>> On Tue, Jun 25, 2013 at 03:24:09PM +0800, Chen Gang wrote:
>>> On 06/25/2013 02:48 PM, Heiko Carstens wrote:
>>>> On Tue, Jun 25, 2013 at 09:46:45AM +0800, Chen Gang wrote:
>>>>>> When smp_add_present_cpu() fails, it has reset all things excluding
>>>>>> 'c->hotpluggable', so need reset it as original state completely.
>>>>>>
>>>>>> +	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 ?
>>
> No, just leave the code as it is.
>>
> 

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------------------------------

diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index 15a016c..147cc18 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -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--------------------------------


> 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.
> 


-- 
Chen Gang

Asianux Corporation

[PATCH v2] arch: s390: kernel: scan all present cpu forcely.

[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 <gang.chen@asianux.com>
---
 arch/s390/kernel/smp.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index d386c4e..75a118f 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -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;
 }
-- 
1.7.7.6

Re: [PATCH v2] arch: s390: kernel: scan all present cpu forcely.

[Heiko Carstens]

On Thu, Jun 27, 2013 at 10:43:02AM +0800, Chen Gang wrote:
> 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 <gang.chen@asianux.com>
> ---
>  arch/s390/kernel/smp.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
> index d386c4e..75a118f 100644
> --- a/arch/s390/kernel/smp.c
> +++ b/arch/s390/kernel/smp.c
> @@ -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.

[Chen Gang]

On 06/27/2013 04:18 PM, Heiko Carstens wrote:
> On Thu, Jun 27, 2013 at 10:43:02AM +0800, Chen Gang wrote:
>> > 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 <gang.chen@asianux.com>
>> > ---
>> >  arch/s390/kernel/smp.c |    5 +++--
>> >  1 files changed, 3 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
>> > index d386c4e..75a118f 100644
>> > --- a/arch/s390/kernel/smp.c
>> > +++ b/arch/s390/kernel/smp.c
>> > @@ -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.

Pardon, excuse me, my English is not quite well.

> 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.

> 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.

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
-- 
Chen Gang

[PATCH] include/linux/interrupt.h: add dummy irq_set_irq_wake() for "!GENERIC_HARDIRQS"

[Chen Gang]

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]


Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 include/linux/interrupt.h |   23 ++++++++++-------------
 1 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 5fa5afe..1a311e0 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -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
-- 
1.7.7.6