2014-10-20 03:29:18

by Neil Zhang

[permalink] [raw]
Subject: [PATCH] drivers: base: update cpu offline info when do hotplug

The current per-cpu offline info won't be updated if it is
hotplugged in/out by a kernel governer.
Let's update it via cpu notifier.

Signed-off-by: Neil Zhang <[email protected]>
---
drivers/base/cpu.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 006b1bc..9d61824 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -418,10 +418,35 @@ static void __init cpu_dev_register_generic(void)
#endif
}

+static int device_hotplug_notifier(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+ unsigned int cpu = (unsigned long)hcpu;
+ struct device *dev = get_cpu_device(cpu);
+ int ret;
+
+ switch (action & ~CPU_TASKS_FROZEN) {
+ case CPU_ONLINE:
+ dev->offline = false;
+ ret = NOTIFY_OK;
+ break;
+ case CPU_DYING:
+ dev->offline = true;
+ ret = NOTIFY_OK;
+ break;
+ default:
+ ret = NOTIFY_DONE;
+ break;
+ }
+
+ return ret;
+}
+
void __init cpu_dev_init(void)
{
if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups))
panic("Failed to register CPU subsystem");

cpu_dev_register_generic();
+ cpu_notifier(device_hotplug_notifier, 0);
}
--
1.7.9.5


2014-10-20 04:44:53

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Mon, Oct 20, 2014 at 11:29:08AM +0800, Neil Zhang wrote:
> The current per-cpu offline info won't be updated if it is
> hotplugged in/out by a kernel governer.
> Let's update it via cpu notifier.
>
> Signed-off-by: Neil Zhang <[email protected]>
> ---
> drivers/base/cpu.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 006b1bc..9d61824 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -418,10 +418,35 @@ static void __init cpu_dev_register_generic(void)
> #endif
> }
>
> +static int device_hotplug_notifier(struct notifier_block *nfb,
> + unsigned long action, void *hcpu)
> +{
> + unsigned int cpu = (unsigned long)hcpu;
> + struct device *dev = get_cpu_device(cpu);
> + int ret;
> +
> + switch (action & ~CPU_TASKS_FROZEN) {
> + case CPU_ONLINE:
> + dev->offline = false;
> + ret = NOTIFY_OK;
> + break;
> + case CPU_DYING:
> + dev->offline = true;
> + ret = NOTIFY_OK;
> + break;
> + default:
> + ret = NOTIFY_DONE;
> + break;
> + }
> +
> + return ret;
> +}
> +
> void __init cpu_dev_init(void)
> {
> if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups))
> panic("Failed to register CPU subsystem");
>
> cpu_dev_register_generic();
> + cpu_notifier(device_hotplug_notifier, 0);
> }

How much noise is this going to cause on a big/little system that
constantly hot unplug/plugs processors all of the time?

greg k-h

2014-10-20 06:39:30

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug

Greg,

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: 2014??10??20?? 12:44
To: Neil Zhang
Cc: [email protected]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Mon, Oct 20, 2014 at 11:29:08AM +0800, Neil Zhang wrote:
> The current per-cpu offline info won't be updated if it is hotplugged
> in/out by a kernel governer.
> Let's update it via cpu notifier.
>
> Signed-off-by: Neil Zhang <[email protected]>
> ---
> drivers/base/cpu.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index
> 006b1bc..9d61824 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -418,10 +418,35 @@ static void __init
> cpu_dev_register_generic(void) #endif }
>
> +static int device_hotplug_notifier(struct notifier_block *nfb,
> + unsigned long action, void *hcpu) {
> + unsigned int cpu = (unsigned long)hcpu;
> + struct device *dev = get_cpu_device(cpu);
> + int ret;
> +
> + switch (action & ~CPU_TASKS_FROZEN) {
> + case CPU_ONLINE:
> + dev->offline = false;
> + ret = NOTIFY_OK;
> + break;
> + case CPU_DYING:
> + dev->offline = true;
> + ret = NOTIFY_OK;
> + break;
> + default:
> + ret = NOTIFY_DONE;
> + break;
> + }
> +
> + return ret;
> +}
> +
> void __init cpu_dev_init(void)
> {
> if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups))
> panic("Failed to register CPU subsystem");
>
> cpu_dev_register_generic();
> + cpu_notifier(device_hotplug_notifier, 0);
> }

How much noise is this going to cause on a big/little system that constantly hot unplug/plugs processors all of the time?

Can you explain more what kind of noise will be introduced on a big/little system?
As I know IKS on arm will use cpu_suspend way to power down a core.
But I don't know well about other architectures.
Please give your suggestions.
Thanks!

greg k-h

Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-20 06:49:42

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>> How much noise is this going to cause on a big/little system that
>> constantly hot unplug/plugs processors all of the time?
>
> Can you explain more what kind of noise will be introduced on a big/little system?

Have you tested this on such a machine?

> As I know IKS on arm will use cpu_suspend way to power down a core.

Are you sure that it also doesn't use that same functionality to drop a
processor to save power?

Why do you need/want this notification? What are you going to do with
this information that you don't already have?

thanks,

greg k-h

2014-10-20 07:40:58

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug

Greg,


-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: 2014??10??20?? 14:48
To: Neil Zhang
Cc: [email protected]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>> How much noise is this going to cause on a big/little system that
>> constantly hot unplug/plugs processors all of the time?
>
> Can you explain more what kind of noise will be introduced on a big/little system?

Have you tested this on such a machine?

I didn't have such kind of machine on hand.
Can anyone has such machine to verify it?
Thanks!

> As I know IKS on arm will use cpu_suspend way to power down a core.

Are you sure that it also doesn't use that same functionality to drop a processor to save power?

As I know it use cpu_suspend to switch out a processor in IKS and there is no cpu hotplug notifier in this procedure.


Why do you need/want this notification? What are you going to do with this information that you don't already have?

The offline won't be updated if an in kernel hotplug governor plug in / out a core which cause the sysfs interface report a wrong status.


thanks,

greg k-h


Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-20 17:03:06

by Dan Streetman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> Greg,
>
>
> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: 2014年10月20日 14:48
> To: Neil Zhang
> Cc: [email protected]
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>
> On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>>> How much noise is this going to cause on a big/little system that
>>> constantly hot unplug/plugs processors all of the time?
>>
>> Can you explain more what kind of noise will be introduced on a big/little system?
>
> Have you tested this on such a machine?
>
> I didn't have such kind of machine on hand.
> Can anyone has such machine to verify it?
> Thanks!

I tested this on a ppc PowerVM system, using dlpar operations to
remove/add cpus.

Without this patch the cpu online nodes get out of sync with the main
online node (and the actual state of the cpus), because they aren't
updated as the cpus are brought up/down:

[root@br10p02 cpu]$ pwd
/sys/devices/system/cpu
[root@br10p02 cpu]$ cat online
0-39
[root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
-eq 1 && echo -n "$n " ; done ; echo ""
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47


While with the patch, the cpu online nodes are kept up to date as the
cpus are brought up/down:

[root@br10p02 cpu]$ pwd
/sys/devices/system/cpu
[root@br10p02 cpu]$ cat online
0-39
[root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
-eq 1 && echo -n "$n " ; done ; echo ""
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
27 28 29 30 31 32 33 34 35 36 37 38 39


Feel free to add

Tested-by: Dan Streetman <[email protected]>

>
>> As I know IKS on arm will use cpu_suspend way to power down a core.
>
> Are you sure that it also doesn't use that same functionality to drop a processor to save power?
>
> As I know it use cpu_suspend to switch out a processor in IKS and there is no cpu hotplug notifier in this procedure.
>
>
> Why do you need/want this notification? What are you going to do with this information that you don't already have?
>
> The offline won't be updated if an in kernel hotplug governor plug in / out a core which cause the sysfs interface report a wrong status.
>
>
> thanks,
>
> greg k-h
>
>
> Best Regards,
> Neil Zhang

2014-10-21 00:47:18

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug

Dan,

> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Dan
> Streetman
> Sent: 2014年10月21日 1:03
> To: Neil Zhang
> Cc: Greg KH; [email protected]
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> hotplug
>
> On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> > Greg,
> >
> >
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > Sent: 2014年10月20日 14:48
> > To: Neil Zhang
> > Cc: [email protected]
> > Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> > hotplug
> >
> > On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> >>> How much noise is this going to cause on a big/little system that
> >>> constantly hot unplug/plugs processors all of the time?
> >>
> >> Can you explain more what kind of noise will be introduced on a
> big/little system?
> >
> > Have you tested this on such a machine?
> >
> > I didn't have such kind of machine on hand.
> > Can anyone has such machine to verify it?
> > Thanks!
>
> I tested this on a ppc PowerVM system, using dlpar operations to
> remove/add cpus.
>
> Without this patch the cpu online nodes get out of sync with the main
> online node (and the actual state of the cpus), because they aren't
> updated as the cpus are brought up/down:
>
> [root@br10p02 cpu]$ pwd
> /sys/devices/system/cpu
> [root@br10p02 cpu]$ cat online
> 0-39
> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online ) -
> eq 1 && echo -n "$n " ; done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
>
>
> While with the patch, the cpu online nodes are kept up to date as the
> cpus are brought up/down:
>
> [root@br10p02 cpu]$ pwd
> /sys/devices/system/cpu
> [root@br10p02 cpu]$ cat online
> 0-39
> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online ) -
> eq 1 && echo -n "$n " ; done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> 27 28 29 30 31 32 33 34 35 36 37 38 39
>
>
> Feel free to add

Thanks for your kindly test!

>
> Tested-by: Dan Streetman <[email protected]>
>
> >
> >> As I know IKS on arm will use cpu_suspend way to power down a core.
> >
> > Are you sure that it also doesn't use that same functionality to drop
> a processor to save power?
> >
> > As I know it use cpu_suspend to switch out a processor in IKS and
> there is no cpu hotplug notifier in this procedure.
> >
> >
> > Why do you need/want this notification? What are you going to do
> with this information that you don't already have?
> >
> > The offline won't be updated if an in kernel hotplug governor plug in
> / out a core which cause the sysfs interface report a wrong status.
> >
> >
> > thanks,
> >
> > greg k-h
> >
> >
> > Best Regards,
> > Neil Zhang


Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-21 02:58:20

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

Hi Neil and Dan,

(2014/10/21 2:02), Dan Streetman wrote:
> On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
>> Greg,
>>
>>
>> -----Original Message-----
>> From: Greg KH [mailto:[email protected]]
>> Sent: 2014年10月20日 14:48
>> To: Neil Zhang
>> Cc: [email protected]
>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>>
>> On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>>>> How much noise is this going to cause on a big/little system that
>>>> constantly hot unplug/plugs processors all of the time?
>>>
>>> Can you explain more what kind of noise will be introduced on a big/little system?
>>
>> Have you tested this on such a machine?
>>
>> I didn't have such kind of machine on hand.
>> Can anyone has such machine to verify it?
>> Thanks!
>
> I tested this on a ppc PowerVM system, using dlpar operations to
> remove/add cpus.
>
> Without this patch the cpu online nodes get out of sync with the main
> online node (and the actual state of the cpus), because they aren't
> updated as the cpus are brought up/down:
>

> [root@br10p02 cpu]$ pwd
> /sys/devices/system/cpu
> [root@br10p02 cpu]$ cat online
> 0-39
> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> -eq 1 && echo -n "$n " ; done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47

How is the issue reproduced.

Here is a result on my x86 box with linux-3.18-rc1.

- before offline CPU
# cd /sys/devices/system/cpu/
# cat online
0-59
# for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ; done ; echo ""
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55
56 57 58 59

- after offline CPU{1..59}
# for n in {1..59} ; do echo 0 > cpu$n/online; done
# cat online
0
# for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ; done ; echo ""
0

It seems that dev->offline is set to correct valute.

Thanks,
Yasuaki Ishimatsu

>
>
> While with the patch, the cpu online nodes are kept up to date as the
> cpus are brought up/down:
>
> [root@br10p02 cpu]$ pwd
> /sys/devices/system/cpu
> [root@br10p02 cpu]$ cat online
> 0-39
> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> -eq 1 && echo -n "$n " ; done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> 27 28 29 30 31 32 33 34 35 36 37 38 39
>
>
> Feel free to add
>
> Tested-by: Dan Streetman <[email protected]>
>
>>
>>> As I know IKS on arm will use cpu_suspend way to power down a core.
>>
>> Are you sure that it also doesn't use that same functionality to drop a processor to save power?
>>
>> As I know it use cpu_suspend to switch out a processor in IKS and there is no cpu hotplug notifier in this procedure.
>>
>>
>> Why do you need/want this notification? What are you going to do with this information that you don't already have?
>>
>> The offline won't be updated if an in kernel hotplug governor plug in / out a core which cause the sysfs interface report a wrong status.
>>
>>
>> thanks,
>>
>> greg k-h
>>
>>
>> Best Regards,
>> Neil Zhang
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2014-10-21 03:19:13

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug

Yasuaki,

> -----Original Message-----
> From: Yasuaki Ishimatsu [mailto:[email protected]]
> Sent: 2014年10月21日 10:57
> To: Dan Streetman; Neil Zhang
> Cc: Greg KH; [email protected]
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>
> Hi Neil and Dan,
>
> (2014/10/21 2:02), Dan Streetman wrote:
> > On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> >> Greg,
> >>
> >>
> >> -----Original Message-----
> >> From: Greg KH [mailto:[email protected]]
> >> Sent: 2014年10月20日 14:48
> >> To: Neil Zhang
> >> Cc: [email protected]
> >> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> >> hotplug
> >>
> >> On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> >>>> How much noise is this going to cause on a big/little system that
> >>>> constantly hot unplug/plugs processors all of the time?
> >>>
> >>> Can you explain more what kind of noise will be introduced on a big/little
> system?
> >>
> >> Have you tested this on such a machine?
> >>
> >> I didn't have such kind of machine on hand.
> >> Can anyone has such machine to verify it?
> >> Thanks!
> >
> > I tested this on a ppc PowerVM system, using dlpar operations to
> > remove/add cpus.
> >
> > Without this patch the cpu online nodes get out of sync with the main
> > online node (and the actual state of the cpus), because they aren't
> > updated as the cpus are brought up/down:
> >
>
> > [root@br10p02 cpu]$ pwd
> > /sys/devices/system/cpu
> > [root@br10p02 cpu]$ cat online
> > 0-39
> > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> > -eq 1 && echo -n "$n " ; done ; echo ""
> > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> > 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
>
> How is the issue reproduced.
>
> Here is a result on my x86 box with linux-3.18-rc1.
>
> - before offline CPU
> # cd /sys/devices/system/cpu/
> # cat online
> 0-59
> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ;
> done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28
> 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54
> 55
> 56 57 58 59
>
> - after offline CPU{1..59}
> # for n in {1..59} ; do echo 0 > cpu$n/online; done # cat online
> 0
> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ;
> done ; echo ""
> 0
>
> It seems that dev->offline is set to correct valute.
>

Please use an in kernel governor to up / down a core instead of sysfs interface.

> Thanks,
> Yasuaki Ishimatsu
>
> >
> >
> > While with the patch, the cpu online nodes are kept up to date as the
> > cpus are brought up/down:
> >
> > [root@br10p02 cpu]$ pwd
> > /sys/devices/system/cpu
> > [root@br10p02 cpu]$ cat online
> > 0-39
> > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> > -eq 1 && echo -n "$n " ; done ; echo ""
> > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> > 27 28 29 30 31 32 33 34 35 36 37 38 39
> >
> >
> > Feel free to add
> >
> > Tested-by: Dan Streetman <[email protected]>
> >
> >>
> >>> As I know IKS on arm will use cpu_suspend way to power down a core.
> >>
> >> Are you sure that it also doesn't use that same functionality to drop a
> processor to save power?
> >>
> >> As I know it use cpu_suspend to switch out a processor in IKS and there is
> no cpu hotplug notifier in this procedure.
> >>
> >>
> >> Why do you need/want this notification? What are you going to do with this
> information that you don't already have?
> >>
> >> The offline won't be updated if an in kernel hotplug governor plug in / out
> a core which cause the sysfs interface report a wrong status.
> >>
> >>
> >> thanks,
> >>
> >> greg k-h
> >>
> >>
> >> Best Regards,
> >> Neil Zhang
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-kernel" in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>


Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-21 03:27:42

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

(2014/10/21 12:18), Neil Zhang wrote:
> Yasuaki,
>
>> -----Original Message-----
>> From: Yasuaki Ishimatsu [mailto:[email protected]]
>> Sent: 2014年10月21日 10:57
>> To: Dan Streetman; Neil Zhang
>> Cc: Greg KH; [email protected]
>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>>
>> Hi Neil and Dan,
>>
>> (2014/10/21 2:02), Dan Streetman wrote:
>>> On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
>>>> Greg,
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Greg KH [mailto:[email protected]]
>>>> Sent: 2014年10月20日 14:48
>>>> To: Neil Zhang
>>>> Cc: [email protected]
>>>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
>>>> hotplug
>>>>
>>>> On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>>>>>> How much noise is this going to cause on a big/little system that
>>>>>> constantly hot unplug/plugs processors all of the time?
>>>>>
>>>>> Can you explain more what kind of noise will be introduced on a big/little
>> system?
>>>>
>>>> Have you tested this on such a machine?
>>>>
>>>> I didn't have such kind of machine on hand.
>>>> Can anyone has such machine to verify it?
>>>> Thanks!
>>>
>>> I tested this on a ppc PowerVM system, using dlpar operations to
>>> remove/add cpus.
>>>
>>> Without this patch the cpu online nodes get out of sync with the main
>>> online node (and the actual state of the cpus), because they aren't
>>> updated as the cpus are brought up/down:
>>>
>>
>>> [root@br10p02 cpu]$ pwd
>>> /sys/devices/system/cpu
>>> [root@br10p02 cpu]$ cat online
>>> 0-39
>>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>>> -eq 1 && echo -n "$n " ; done ; echo ""
>>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
>>> 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
>>
>> How is the issue reproduced.
>>
>> Here is a result on my x86 box with linux-3.18-rc1.
>>
>> - before offline CPU
>> # cd /sys/devices/system/cpu/
>> # cat online
>> 0-59
>> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ;
>> done ; echo ""
>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28
>> 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54
>> 55
>> 56 57 58 59
>>
>> - after offline CPU{1..59}
>> # for n in {1..59} ; do echo 0 > cpu$n/online; done # cat online
>> 0
>> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ;
>> done ; echo ""
>> 0
>>
>> It seems that dev->offline is set to correct valute.
>>
>

> Please use an in kernel governor to up / down a core instead of sysfs interface.

Thank you for the information. But I don't know the in kernel governor?
Could you point a documentation of it?

Thanks,
Yasuaki Ishimatsu

>
>> Thanks,
>> Yasuaki Ishimatsu
>>
>>>
>>>
>>> While with the patch, the cpu online nodes are kept up to date as the
>>> cpus are brought up/down:
>>>
>>> [root@br10p02 cpu]$ pwd
>>> /sys/devices/system/cpu
>>> [root@br10p02 cpu]$ cat online
>>> 0-39
>>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>>> -eq 1 && echo -n "$n " ; done ; echo ""
>>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
>>> 27 28 29 30 31 32 33 34 35 36 37 38 39
>>>
>>>
>>> Feel free to add
>>>
>>> Tested-by: Dan Streetman <[email protected]>
>>>
>>>>
>>>>> As I know IKS on arm will use cpu_suspend way to power down a core.
>>>>
>>>> Are you sure that it also doesn't use that same functionality to drop a
>> processor to save power?
>>>>
>>>> As I know it use cpu_suspend to switch out a processor in IKS and there is
>> no cpu hotplug notifier in this procedure.
>>>>
>>>>
>>>> Why do you need/want this notification? What are you going to do with this
>> information that you don't already have?
>>>>
>>>> The offline won't be updated if an in kernel hotplug governor plug in / out
>> a core which cause the sysfs interface report a wrong status.
>>>>
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>>
>>>>
>>>> Best Regards,
>>>> Neil Zhang
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-kernel" in the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>>
>>
>
>
> Best Regards,
> Neil Zhang
> �{.n�+�������+%��lzwm��b�맲��r��zX���w��{ay�ʇڙ�,j��f���h���z��w��� ���j:+v���w�j�m��������zZ+�����ݢj"��!�iO��z��v�^�m���� nƊ��Y&�
>

2014-10-21 03:36:24

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug



> -----Original Message-----
> From: Yasuaki Ishimatsu [mailto:[email protected]]
> Sent: 2014年10月21日 11:27
> To: Neil Zhang; Dan Streetman
> Cc: Greg KH; [email protected]
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>
> (2014/10/21 12:18), Neil Zhang wrote:
> > Yasuaki,
> >
> >> -----Original Message-----
> >> From: Yasuaki Ishimatsu [mailto:[email protected]]
> >> Sent: 2014年10月21日 10:57
> >> To: Dan Streetman; Neil Zhang
> >> Cc: Greg KH; [email protected]
> >> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> >> hotplug
> >>
> >> Hi Neil and Dan,
> >>
> >> (2014/10/21 2:02), Dan Streetman wrote:
> >>> On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> >>>> Greg,
> >>>>
> >>>>
> >>>> -----Original Message-----
> >>>> From: Greg KH [mailto:[email protected]]
> >>>> Sent: 2014年10月20日 14:48
> >>>> To: Neil Zhang
> >>>> Cc: [email protected]
> >>>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> >>>> hotplug
> >>>>
> >>>> On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> >>>>>> How much noise is this going to cause on a big/little system that
> >>>>>> constantly hot unplug/plugs processors all of the time?
> >>>>>
> >>>>> Can you explain more what kind of noise will be introduced on a
> >>>>> big/little
> >> system?
> >>>>
> >>>> Have you tested this on such a machine?
> >>>>
> >>>> I didn't have such kind of machine on hand.
> >>>> Can anyone has such machine to verify it?
> >>>> Thanks!
> >>>
> >>> I tested this on a ppc PowerVM system, using dlpar operations to
> >>> remove/add cpus.
> >>>
> >>> Without this patch the cpu online nodes get out of sync with the
> >>> main online node (and the actual state of the cpus), because they
> >>> aren't updated as the cpus are brought up/down:
> >>>
> >>
> >>> [root@br10p02 cpu]$ pwd
> >>> /sys/devices/system/cpu
> >>> [root@br10p02 cpu]$ cat online
> >>> 0-39
> >>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> >>> -eq 1 && echo -n "$n " ; done ; echo ""
> >>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
> >>> 26
> >>> 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
> >>
> >> How is the issue reproduced.
> >>
> >> Here is a result on my x86 box with linux-3.18-rc1.
> >>
> >> - before offline CPU
> >> # cd /sys/devices/system/cpu/
> >> # cat online
> >> 0-59
> >> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n
> >> "$n " ; done ; echo ""
> >> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
> >> 26 27 28
> >> 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51
> >> 52 53 54
> >> 55
> >> 56 57 58 59
> >>
> >> - after offline CPU{1..59}
> >> # for n in {1..59} ; do echo 0 > cpu$n/online; done # cat online
> >> 0
> >> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n
> >> "$n " ; done ; echo ""
> >> 0
> >>
> >> It seems that dev->offline is set to correct valute.
> >>
> >
>
> > Please use an in kernel governor to up / down a core instead of sysfs
> interface.
>
> Thank you for the information. But I don't know the in kernel governor?
> Could you point a documentation of it?
>

Simply means that you call cpu_down / cpu_up directly in kernel base on the profiler.

> Thanks,
> Yasuaki Ishimatsu
>
> >
> >> Thanks,
> >> Yasuaki Ishimatsu
> >>
> >>>
> >>>
> >>> While with the patch, the cpu online nodes are kept up to date as
> >>> the cpus are brought up/down:
> >>>
> >>> [root@br10p02 cpu]$ pwd
> >>> /sys/devices/system/cpu
> >>> [root@br10p02 cpu]$ cat online
> >>> 0-39
> >>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> >>> -eq 1 && echo -n "$n " ; done ; echo ""
> >>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
> >>> 26
> >>> 27 28 29 30 31 32 33 34 35 36 37 38 39
> >>>
> >>>
> >>> Feel free to add
> >>>
> >>> Tested-by: Dan Streetman <[email protected]>
> >>>
> >>>>
> >>>>> As I know IKS on arm will use cpu_suspend way to power down a core.
> >>>>
> >>>> Are you sure that it also doesn't use that same functionality to
> >>>> drop a
> >> processor to save power?
> >>>>
> >>>> As I know it use cpu_suspend to switch out a processor in IKS and
> >>>> there is
> >> no cpu hotplug notifier in this procedure.
> >>>>
> >>>>
> >>>> Why do you need/want this notification? What are you going to do
> >>>> with this
> >> information that you don't already have?
> >>>>
> >>>> The offline won't be updated if an in kernel hotplug governor plug
> >>>> in / out
> >> a core which cause the sysfs interface report a wrong status.
> >>>>
> >>>>
> >>>> thanks,
> >>>>
> >>>> greg k-h
> >>>>
> >>>>
> >>>> Best Regards,
> >>>> Neil Zhang
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe
> >>> linux-kernel" in the body of a message to [email protected]
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>> Please read the FAQ at http://www.tux.org/lkml/
> >>>
> >>
> >
> >
> > Best Regards,
> > Neil Zhang
> >  {.n + +% lzwm b 맲 r zX  w {ay ʇڙ ,j
> > f h z  w
>
> j:+v w j m zZ+ ݢj" ! iO z v ^ m
> nƊ Y&
> >
>


Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-21 04:48:27

by Yasuaki Ishimatsu

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

(2014/10/21 12:36), Neil Zhang wrote:
>
>
>> -----Original Message-----
>> From: Yasuaki Ishimatsu [mailto:[email protected]]
>> Sent: 2014年10月21日 11:27
>> To: Neil Zhang; Dan Streetman
>> Cc: Greg KH; [email protected]
>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>>
>> (2014/10/21 12:18), Neil Zhang wrote:
>>> Yasuaki,
>>>
>>>> -----Original Message-----
>>>> From: Yasuaki Ishimatsu [mailto:[email protected]]
>>>> Sent: 2014年10月21日 10:57
>>>> To: Dan Streetman; Neil Zhang
>>>> Cc: Greg KH; [email protected]
>>>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
>>>> hotplug
>>>>
>>>> Hi Neil and Dan,
>>>>
>>>> (2014/10/21 2:02), Dan Streetman wrote:
>>>>> On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
>>>>>> Greg,
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Greg KH [mailto:[email protected]]
>>>>>> Sent: 2014年10月20日 14:48
>>>>>> To: Neil Zhang
>>>>>> Cc: [email protected]
>>>>>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
>>>>>> hotplug
>>>>>>
>>>>>> On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>>>>>>>> How much noise is this going to cause on a big/little system that
>>>>>>>> constantly hot unplug/plugs processors all of the time?
>>>>>>>
>>>>>>> Can you explain more what kind of noise will be introduced on a
>>>>>>> big/little
>>>> system?
>>>>>>
>>>>>> Have you tested this on such a machine?
>>>>>>
>>>>>> I didn't have such kind of machine on hand.
>>>>>> Can anyone has such machine to verify it?
>>>>>> Thanks!
>>>>>
>>>>> I tested this on a ppc PowerVM system, using dlpar operations to
>>>>> remove/add cpus.
>>>>>
>>>>> Without this patch the cpu online nodes get out of sync with the
>>>>> main online node (and the actual state of the cpus), because they
>>>>> aren't updated as the cpus are brought up/down:
>>>>>
>>>>
>>>>> [root@br10p02 cpu]$ pwd
>>>>> /sys/devices/system/cpu
>>>>> [root@br10p02 cpu]$ cat online
>>>>> 0-39
>>>>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>>>>> -eq 1 && echo -n "$n " ; done ; echo ""
>>>>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
>>>>> 26
>>>>> 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
>>>>
>>>> How is the issue reproduced.
>>>>
>>>> Here is a result on my x86 box with linux-3.18-rc1.
>>>>
>>>> - before offline CPU
>>>> # cd /sys/devices/system/cpu/
>>>> # cat online
>>>> 0-59
>>>> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n
>>>> "$n " ; done ; echo ""
>>>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
>>>> 26 27 28
>>>> 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51
>>>> 52 53 54
>>>> 55
>>>> 56 57 58 59
>>>>
>>>> - after offline CPU{1..59}
>>>> # for n in {1..59} ; do echo 0 > cpu$n/online; done # cat online
>>>> 0
>>>> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n
>>>> "$n " ; done ; echo ""
>>>> 0
>>>>
>>>> It seems that dev->offline is set to correct valute.
>>>>
>>>
>>
>>> Please use an in kernel governor to up / down a core instead of sysfs
>> interface.
>>
>> Thank you for the information. But I don't know the in kernel governor?
>> Could you point a documentation of it?
>>
>

> Simply means that you call cpu_down / cpu_up directly in kernel base on the profiler.

I understood it.
When using sysfs, device_online/offline() changes dev->offline.
Therefore, this problem did not occur in my previous test.

Thanks,
Yasuaki Ishimatsu


>
>> Thanks,
>> Yasuaki Ishimatsu
>>
>>>
>>>> Thanks,
>>>> Yasuaki Ishimatsu
>>>>
>>>>>
>>>>>
>>>>> While with the patch, the cpu online nodes are kept up to date as
>>>>> the cpus are brought up/down:
>>>>>
>>>>> [root@br10p02 cpu]$ pwd
>>>>> /sys/devices/system/cpu
>>>>> [root@br10p02 cpu]$ cat online
>>>>> 0-39
>>>>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>>>>> -eq 1 && echo -n "$n " ; done ; echo ""
>>>>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
>>>>> 26
>>>>> 27 28 29 30 31 32 33 34 35 36 37 38 39
>>>>>
>>>>>
>>>>> Feel free to add
>>>>>
>>>>> Tested-by: Dan Streetman <[email protected]>
>>>>>
>>>>>>
>>>>>>> As I know IKS on arm will use cpu_suspend way to power down a core.
>>>>>>
>>>>>> Are you sure that it also doesn't use that same functionality to
>>>>>> drop a
>>>> processor to save power?
>>>>>>
>>>>>> As I know it use cpu_suspend to switch out a processor in IKS and
>>>>>> there is
>>>> no cpu hotplug notifier in this procedure.
>>>>>>
>>>>>>
>>>>>> Why do you need/want this notification? What are you going to do
>>>>>> with this
>>>> information that you don't already have?
>>>>>>
>>>>>> The offline won't be updated if an in kernel hotplug governor plug
>>>>>> in / out
>>>> a core which cause the sysfs interface report a wrong status.
>>>>>>
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> greg k-h
>>>>>>
>>>>>>
>>>>>> Best Regards,
>>>>>> Neil Zhang
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe
>>>>> linux-kernel" in the body of a message to [email protected]
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> Please read the FAQ at http://www.tux.org/lkml/
>>>>>
>>>>
>>>
>>>
>>> Best Regards,
>>> Neil Zhang
>>>  {.n + +% lzwm b 맲 r zX  w {ay ʇڙ ,j
>>> f h z  w
>>
>> j:+v w j m zZ+ ݢj" ! iO z v ^ m
>> nƊ Y&
>>>
>>
>
>
> Best Regards,
> Neil Zhang
> ��칻�&�~�&���+-��ݶ��w��˛���m�b��dz�ޖ)���w*jg��������ݢj/���z�ޖ��2�ޙ���&�)ߡ�a�����G���h��j:+v���w�٥>W����i�axPj�m���� -�+��d�_
>

2014-10-21 12:59:19

by Dan Streetman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Mon, Oct 20, 2014 at 10:57 PM, Yasuaki Ishimatsu
<[email protected]> wrote:
> Hi Neil and Dan,
>
>
> (2014/10/21 2:02), Dan Streetman wrote:
>>
>> On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
>>>
>>> Greg,
>>>
>>>
>>> -----Original Message-----
>>> From: Greg KH [mailto:[email protected]]
>>> Sent: 2014年10月20日 14:48
>>> To: Neil Zhang
>>> Cc: [email protected]
>>> Subject: Re: [PATCH] drivers: base: update cpu offline info when do
>>> hotplug
>>>
>>> On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>>>>>
>>>>> How much noise is this going to cause on a big/little system that
>>>>> constantly hot unplug/plugs processors all of the time?
>>>>
>>>>
>>>> Can you explain more what kind of noise will be introduced on a
>>>> big/little system?
>>>
>>>
>>> Have you tested this on such a machine?
>>>
>>> I didn't have such kind of machine on hand.
>>> Can anyone has such machine to verify it?
>>> Thanks!
>>
>>
>> I tested this on a ppc PowerVM system, using dlpar operations to
>> remove/add cpus.
>>
>> Without this patch the cpu online nodes get out of sync with the main
>> online node (and the actual state of the cpus), because they aren't
>> updated as the cpus are brought up/down:
>>
>
>> [root@br10p02 cpu]$ pwd
>> /sys/devices/system/cpu
>> [root@br10p02 cpu]$ cat online
>> 0-39
>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>> -eq 1 && echo -n "$n " ; done ; echo ""
>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
>> 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
>
>
> How is the issue reproduced.
>
> Here is a result on my x86 box with linux-3.18-rc1.

This is not an issue when using the sysfs interface to bring cpus up
or down. This is only an issue when using any other method to bring
cpus up or down. I used dlpar operations, which invoke the
arch/powerpc/platforms/pseries/dlpar.c code, which brings cpus up and
down. The dlpar operations are invoked by a hypervisor on a "LPAR"
(virtualized guest) system to tell it some of the (virtualized) cpus
are being added/removed.

>
> - before offline CPU
> # cd /sys/devices/system/cpu/
> # cat online
> 0-59
> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ;
> done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28
> 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53
> 54 55
> 56 57 58 59
>
> - after offline CPU{1..59}
> # for n in {1..59} ; do echo 0 > cpu$n/online; done
> # cat online
> 0
> # for n in {0..59} ; do test $( cat cpu$n/online ) -eq 1 && echo -n "$n " ;
> done ; echo ""
> 0
>
> It seems that dev->offline is set to correct valute.
>
> Thanks,
> Yasuaki Ishimatsu
>
>>
>>
>> While with the patch, the cpu online nodes are kept up to date as the
>> cpus are brought up/down:
>>
>> [root@br10p02 cpu]$ pwd
>> /sys/devices/system/cpu
>> [root@br10p02 cpu]$ cat online
>> 0-39
>> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>> -eq 1 && echo -n "$n " ; done ; echo ""
>> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
>> 27 28 29 30 31 32 33 34 35 36 37 38 39
>>
>>
>> Feel free to add
>>
>> Tested-by: Dan Streetman <[email protected]>
>>
>>>
>>>> As I know IKS on arm will use cpu_suspend way to power down a core.
>>>
>>>
>>> Are you sure that it also doesn't use that same functionality to drop a
>>> processor to save power?
>>>
>>> As I know it use cpu_suspend to switch out a processor in IKS and there
>>> is no cpu hotplug notifier in this procedure.
>>>
>>>
>>> Why do you need/want this notification? What are you going to do with
>>> this information that you don't already have?
>>>
>>> The offline won't be updated if an in kernel hotplug governor plug in /
>>> out a core which cause the sysfs interface report a wrong status.
>>>
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>>
>>> Best Regards,
>>> Neil Zhang
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
>

2014-10-21 13:03:27

by Dan Streetman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Sun, Oct 19, 2014 at 11:29 PM, Neil Zhang <[email protected]> wrote:
> The current per-cpu offline info won't be updated if it is
> hotplugged in/out by a kernel governer.
> Let's update it via cpu notifier.
>
> Signed-off-by: Neil Zhang <[email protected]>
> ---
> drivers/base/cpu.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 006b1bc..9d61824 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -418,10 +418,35 @@ static void __init cpu_dev_register_generic(void)
> #endif
> }
>
> +static int device_hotplug_notifier(struct notifier_block *nfb,
> + unsigned long action, void *hcpu)
> +{
> + unsigned int cpu = (unsigned long)hcpu;
> + struct device *dev = get_cpu_device(cpu);
> + int ret;
> +
> + switch (action & ~CPU_TASKS_FROZEN) {
> + case CPU_ONLINE:
> + dev->offline = false;
> + ret = NOTIFY_OK;
> + break;
> + case CPU_DYING:
> + dev->offline = true;

one thing i just thought of here, since dev->offline is getting set
here, can you remove it being set in drivers/base/core.c at
device_online() and device_offline() ? That's probably redundant now
right?


> + ret = NOTIFY_OK;
> + break;
> + default:
> + ret = NOTIFY_DONE;
> + break;
> + }
> +
> + return ret;
> +}
> +
> void __init cpu_dev_init(void)
> {
> if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups))
> panic("Failed to register CPU subsystem");
>
> cpu_dev_register_generic();
> + cpu_notifier(device_hotplug_notifier, 0);
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2014-10-22 01:32:22

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug

Dan,

> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Dan
> Streetman
> Sent: 2014年10月21日 21:03
> To: Neil Zhang
> Cc: linux-kernel; Greg Kroah-Hartman
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>
> On Sun, Oct 19, 2014 at 11:29 PM, Neil Zhang <[email protected]> wrote:
> > The current per-cpu offline info won't be updated if it is hotplugged
> > in/out by a kernel governer.
> > Let's update it via cpu notifier.
> >
> > Signed-off-by: Neil Zhang <[email protected]>
> > ---
> > drivers/base/cpu.c | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index
> > 006b1bc..9d61824 100644
> > --- a/drivers/base/cpu.c
> > +++ b/drivers/base/cpu.c
> > @@ -418,10 +418,35 @@ static void __init
> > cpu_dev_register_generic(void) #endif }
> >
> > +static int device_hotplug_notifier(struct notifier_block *nfb,
> > + unsigned long action, void
> > +*hcpu) {
> > + unsigned int cpu = (unsigned long)hcpu;
> > + struct device *dev = get_cpu_device(cpu);
> > + int ret;
> > +
> > + switch (action & ~CPU_TASKS_FROZEN) {
> > + case CPU_ONLINE:
> > + dev->offline = false;
> > + ret = NOTIFY_OK;
> > + break;
> > + case CPU_DYING:
> > + dev->offline = true;
>
> one thing i just thought of here, since dev->offline is getting set here, can
> you remove it being set in drivers/base/core.c at
> device_online() and device_offline() ? That's probably redundant now right?

Yes, I ever thought about it.
But I'm not sure whether some other devices (such as memory) will use it later too.
So I still kept it there since seems won't cause any problem.

>
>
> > + ret = NOTIFY_OK;
> > + break;
> > + default:
> > + ret = NOTIFY_DONE;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > void __init cpu_dev_init(void)
> > {
> > if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups))
> > panic("Failed to register CPU subsystem");
> >
> > cpu_dev_register_generic();
> > + cpu_notifier(device_hotplug_notifier, 0);
> > }
> > --
> > 1.7.9.5
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-kernel" in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/

Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-23 14:05:51

by Dan Streetman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Tue, Oct 21, 2014 at 9:02 AM, Dan Streetman <[email protected]> wrote:
> On Sun, Oct 19, 2014 at 11:29 PM, Neil Zhang <[email protected]> wrote:
>> The current per-cpu offline info won't be updated if it is
>> hotplugged in/out by a kernel governer.
>> Let's update it via cpu notifier.
>>
>> Signed-off-by: Neil Zhang <[email protected]>
>> ---
>> drivers/base/cpu.c | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
>> index 006b1bc..9d61824 100644
>> --- a/drivers/base/cpu.c
>> +++ b/drivers/base/cpu.c
>> @@ -418,10 +418,35 @@ static void __init cpu_dev_register_generic(void)
>> #endif
>> }
>>
>> +static int device_hotplug_notifier(struct notifier_block *nfb,
>> + unsigned long action, void *hcpu)
>> +{
>> + unsigned int cpu = (unsigned long)hcpu;
>> + struct device *dev = get_cpu_device(cpu);
>> + int ret;
>> +
>> + switch (action & ~CPU_TASKS_FROZEN) {
>> + case CPU_ONLINE:
>> + dev->offline = false;
>> + ret = NOTIFY_OK;
>> + break;
>> + case CPU_DYING:
>> + dev->offline = true;
>
> one thing i just thought of here, since dev->offline is getting set
> here, can you remove it being set in drivers/base/core.c at
> device_online() and device_offline() ? That's probably redundant now
> right?

sorry ignore that; device_on|offline() is common code, not cpu-specific.

>
>
>> + ret = NOTIFY_OK;
>> + break;
>> + default:
>> + ret = NOTIFY_DONE;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> void __init cpu_dev_init(void)
>> {
>> if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups))
>> panic("Failed to register CPU subsystem");
>>
>> cpu_dev_register_generic();
>> + cpu_notifier(device_hotplug_notifier, 0);
>> }
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/

2014-10-27 01:43:17

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug

Greg,


> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Dan
> Streetman
> Sent: 2014年10月21日 1:03
> To: Neil Zhang
> Cc: Greg KH; [email protected]
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>
> On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> > Greg,
> >
> >
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > Sent: 2014年10月20日 14:48
> > To: Neil Zhang
> > Cc: [email protected]
> > Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> > hotplug
> >
> > On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> >>> How much noise is this going to cause on a big/little system that
> >>> constantly hot unplug/plugs processors all of the time?
> >>
> >> Can you explain more what kind of noise will be introduced on a big/little
> system?
> >
> > Have you tested this on such a machine?
> >
> > I didn't have such kind of machine on hand.
> > Can anyone has such machine to verify it?
> > Thanks!
>
> I tested this on a ppc PowerVM system, using dlpar operations to remove/add
> cpus.
>
> Without this patch the cpu online nodes get out of sync with the main online
> node (and the actual state of the cpus), because they aren't updated as the
> cpus are brought up/down:
>
> [root@br10p02 cpu]$ pwd
> /sys/devices/system/cpu
> [root@br10p02 cpu]$ cat online
> 0-39
> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online ) -eq 1 &&
> echo -n "$n " ; done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
>
>
> While with the patch, the cpu online nodes are kept up to date as the cpus are
> brought up/down:
>
> [root@br10p02 cpu]$ pwd
> /sys/devices/system/cpu
> [root@br10p02 cpu]$ cat online
> 0-39
> [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online ) -eq 1 &&
> echo -n "$n " ; done ; echo ""
> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> 27 28 29 30 31 32 33 34 35 36 37 38 39
>
>
> Feel free to add
>
> Tested-by: Dan Streetman <[email protected]>
>

It's a real bug in the kernel.
What's your comments about this patch?

> >
> >> As I know IKS on arm will use cpu_suspend way to power down a core.
> >
> > Are you sure that it also doesn't use that same functionality to drop a
> processor to save power?
> >
> > As I know it use cpu_suspend to switch out a processor in IKS and there is
> no cpu hotplug notifier in this procedure.
> >
> >
> > Why do you need/want this notification? What are you going to do with this
> information that you don't already have?
> >
> > The offline won't be updated if an in kernel hotplug governor plug in / out
> a core which cause the sysfs interface report a wrong status.
> >
> >
> > thanks,
> >
> > greg k-h
> >
> >
> > Best Regards,
> > Neil Zhang

Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-27 01:59:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Sun, Oct 26, 2014 at 06:43:11PM -0700, Neil Zhang wrote:
> Greg,
>
>
> > -----Original Message-----
> > From: [email protected] [mailto:[email protected]] On Behalf Of Dan
> > Streetman
> > Sent: 2014年10月21日 1:03
> > To: Neil Zhang
> > Cc: Greg KH; [email protected]
> > Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
> >
> > On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> > > Greg,
> > >
> > >
> > > -----Original Message-----
> > > From: Greg KH [mailto:[email protected]]
> > > Sent: 2014年10月20日 14:48
> > > To: Neil Zhang
> > > Cc: [email protected]
> > > Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> > > hotplug
> > >
> > > On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> > >>> How much noise is this going to cause on a big/little system that
> > >>> constantly hot unplug/plugs processors all of the time?
> > >>
> > >> Can you explain more what kind of noise will be introduced on a big/little
> > system?
> > >
> > > Have you tested this on such a machine?
> > >
> > > I didn't have such kind of machine on hand.
> > > Can anyone has such machine to verify it?
> > > Thanks!
> >
> > I tested this on a ppc PowerVM system, using dlpar operations to remove/add
> > cpus.
> >
> > Without this patch the cpu online nodes get out of sync with the main online
> > node (and the actual state of the cpus), because they aren't updated as the
> > cpus are brought up/down:
> >
> > [root@br10p02 cpu]$ pwd
> > /sys/devices/system/cpu
> > [root@br10p02 cpu]$ cat online
> > 0-39
> > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online ) -eq 1 &&
> > echo -n "$n " ; done ; echo ""
> > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> > 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
> >
> >
> > While with the patch, the cpu online nodes are kept up to date as the cpus are
> > brought up/down:
> >
> > [root@br10p02 cpu]$ pwd
> > /sys/devices/system/cpu
> > [root@br10p02 cpu]$ cat online
> > 0-39
> > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online ) -eq 1 &&
> > echo -n "$n " ; done ; echo ""
> > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26
> > 27 28 29 30 31 32 33 34 35 36 37 38 39
> >
> >
> > Feel free to add
> >
> > Tested-by: Dan Streetman <[email protected]>
> >
>
> It's a real bug in the kernel.

As this has been this way for many years, I tend to think it's not all
that important...

> What's your comments about this patch?

It's one of 1800+ patches that need my comments, please be patient.

greg k-h

2014-10-27 02:17:21

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug



> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: 2014年10月27日 9:59
> To: Neil Zhang
> Cc: Dan Streetman; [email protected]
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>
> On Sun, Oct 26, 2014 at 06:43:11PM -0700, Neil Zhang wrote:
> > Greg,
> >
> >
> > > -----Original Message-----
> > > From: [email protected] [mailto:[email protected]] On Behalf Of
> > > Dan Streetman
> > > Sent: 2014年10月21日 1:03
> > > To: Neil Zhang
> > > Cc: Greg KH; [email protected]
> > > Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> > > hotplug
> > >
> > > On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> > > > Greg,
> > > >
> > > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:[email protected]]
> > > > Sent: 2014年10月20日 14:48
> > > > To: Neil Zhang
> > > > Cc: [email protected]
> > > > Subject: Re: [PATCH] drivers: base: update cpu offline info when
> > > > do hotplug
> > > >
> > > > On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> > > >>> How much noise is this going to cause on a big/little system
> > > >>> that constantly hot unplug/plugs processors all of the time?
> > > >>
> > > >> Can you explain more what kind of noise will be introduced on a
> > > >> big/little
> > > system?
> > > >
> > > > Have you tested this on such a machine?
> > > >
> > > > I didn't have such kind of machine on hand.
> > > > Can anyone has such machine to verify it?
> > > > Thanks!
> > >
> > > I tested this on a ppc PowerVM system, using dlpar operations to
> > > remove/add cpus.
> > >
> > > Without this patch the cpu online nodes get out of sync with the
> > > main online node (and the actual state of the cpus), because they
> > > aren't updated as the cpus are brought up/down:
> > >
> > > [root@br10p02 cpu]$ pwd
> > > /sys/devices/system/cpu
> > > [root@br10p02 cpu]$ cat online
> > > 0-39
> > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> > > -eq 1 && echo -n "$n " ; done ; echo ""
> > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
> > > 26
> > > 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
> > >
> > >
> > > While with the patch, the cpu online nodes are kept up to date as
> > > the cpus are brought up/down:
> > >
> > > [root@br10p02 cpu]$ pwd
> > > /sys/devices/system/cpu
> > > [root@br10p02 cpu]$ cat online
> > > 0-39
> > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> > > -eq 1 && echo -n "$n " ; done ; echo ""
> > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
> > > 26
> > > 27 28 29 30 31 32 33 34 35 36 37 38 39
> > >
> > >
> > > Feel free to add
> > >
> > > Tested-by: Dan Streetman <[email protected]>
> > >
> >
> > It's a real bug in the kernel.
>
> As this has been this way for many years, I tend to think it's not all that
> important...

Actually this bug was introduced by the following patch.

commit 0902a9044fa5b7a0456ea4daacec2c2b3189ba8c
Author: Rafael J. Wysocki <[email protected]>
Date: Fri May 3 00:25:49 2013 +0200

Driver core: Use generic offline/online for CPU offline/online

So seems not that long :)

>
> > What's your comments about this patch?
>
> It's one of 1800+ patches that need my comments, please be patient.

Got it, thanks!

>
> greg k-h

Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-27 02:27:49

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Sun, Oct 26, 2014 at 07:17:14PM -0700, Neil Zhang wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > Sent: 2014年10月27日 9:59
> > To: Neil Zhang
> > Cc: Dan Streetman; [email protected]
> > Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
> >
> > On Sun, Oct 26, 2014 at 06:43:11PM -0700, Neil Zhang wrote:
> > > Greg,
> > >
> > >
> > > > -----Original Message-----
> > > > From: [email protected] [mailto:[email protected]] On Behalf Of
> > > > Dan Streetman
> > > > Sent: 2014年10月21日 1:03
> > > > To: Neil Zhang
> > > > Cc: Greg KH; [email protected]
> > > > Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> > > > hotplug
> > > >
> > > > On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
> > > > > Greg,
> > > > >
> > > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:[email protected]]
> > > > > Sent: 2014年10月20日 14:48
> > > > > To: Neil Zhang
> > > > > Cc: [email protected]
> > > > > Subject: Re: [PATCH] drivers: base: update cpu offline info when
> > > > > do hotplug
> > > > >
> > > > > On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> > > > >>> How much noise is this going to cause on a big/little system
> > > > >>> that constantly hot unplug/plugs processors all of the time?
> > > > >>
> > > > >> Can you explain more what kind of noise will be introduced on a
> > > > >> big/little
> > > > system?
> > > > >
> > > > > Have you tested this on such a machine?
> > > > >
> > > > > I didn't have such kind of machine on hand.
> > > > > Can anyone has such machine to verify it?
> > > > > Thanks!
> > > >
> > > > I tested this on a ppc PowerVM system, using dlpar operations to
> > > > remove/add cpus.
> > > >
> > > > Without this patch the cpu online nodes get out of sync with the
> > > > main online node (and the actual state of the cpus), because they
> > > > aren't updated as the cpus are brought up/down:
> > > >
> > > > [root@br10p02 cpu]$ pwd
> > > > /sys/devices/system/cpu
> > > > [root@br10p02 cpu]$ cat online
> > > > 0-39
> > > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> > > > -eq 1 && echo -n "$n " ; done ; echo ""
> > > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
> > > > 26
> > > > 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
> > > >
> > > >
> > > > While with the patch, the cpu online nodes are kept up to date as
> > > > the cpus are brought up/down:
> > > >
> > > > [root@br10p02 cpu]$ pwd
> > > > /sys/devices/system/cpu
> > > > [root@br10p02 cpu]$ cat online
> > > > 0-39
> > > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
> > > > -eq 1 && echo -n "$n " ; done ; echo ""
> > > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
> > > > 26
> > > > 27 28 29 30 31 32 33 34 35 36 37 38 39
> > > >
> > > >
> > > > Feel free to add
> > > >
> > > > Tested-by: Dan Streetman <[email protected]>
> > > >
> > >
> > > It's a real bug in the kernel.
> >
> > As this has been this way for many years, I tend to think it's not all that
> > important...
>
> Actually this bug was introduced by the following patch.
>
> commit 0902a9044fa5b7a0456ea4daacec2c2b3189ba8c
> Author: Rafael J. Wysocki <[email protected]>
> Date: Fri May 3 00:25:49 2013 +0200
>
> Driver core: Use generic offline/online for CPU offline/online
>
> So seems not that long :)

Ok, over a year.

Any reason why this information wasn't in this patch? Also, why not cc:
the authors of that patch as well? Surely they would want to know about
this, right?

greg k-h

2014-10-27 02:31:07

by Neil Zhang

[permalink] [raw]
Subject: RE: [PATCH] drivers: base: update cpu offline info when do hotplug



> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: 2014年10月27日 10:27
> To: Neil Zhang
> Cc: Dan Streetman; [email protected]
> Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>
> On Sun, Oct 26, 2014 at 07:17:14PM -0700, Neil Zhang wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:[email protected]]
> > > Sent: 2014年10月27日 9:59
> > > To: Neil Zhang
> > > Cc: Dan Streetman; [email protected]
> > > Subject: Re: [PATCH] drivers: base: update cpu offline info when do
> > > hotplug
> > >
> > > On Sun, Oct 26, 2014 at 06:43:11PM -0700, Neil Zhang wrote:
> > > > Greg,
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: [email protected] [mailto:[email protected]] On Behalf
> > > > > Of Dan Streetman
> > > > > Sent: 2014年10月21日 1:03
> > > > > To: Neil Zhang
> > > > > Cc: Greg KH; [email protected]
> > > > > Subject: Re: [PATCH] drivers: base: update cpu offline info when
> > > > > do hotplug
> > > > >
> > > > > On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]>
> wrote:
> > > > > > Greg,
> > > > > >
> > > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg KH [mailto:[email protected]]
> > > > > > Sent: 2014年10月20日 14:48
> > > > > > To: Neil Zhang
> > > > > > Cc: [email protected]
> > > > > > Subject: Re: [PATCH] drivers: base: update cpu offline info
> > > > > > when do hotplug
> > > > > >
> > > > > > On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
> > > > > >>> How much noise is this going to cause on a big/little system
> > > > > >>> that constantly hot unplug/plugs processors all of the time?
> > > > > >>
> > > > > >> Can you explain more what kind of noise will be introduced on
> > > > > >> a big/little
> > > > > system?
> > > > > >
> > > > > > Have you tested this on such a machine?
> > > > > >
> > > > > > I didn't have such kind of machine on hand.
> > > > > > Can anyone has such machine to verify it?
> > > > > > Thanks!
> > > > >
> > > > > I tested this on a ppc PowerVM system, using dlpar operations to
> > > > > remove/add cpus.
> > > > >
> > > > > Without this patch the cpu online nodes get out of sync with the
> > > > > main online node (and the actual state of the cpus), because
> > > > > they aren't updated as the cpus are brought up/down:
> > > > >
> > > > > [root@br10p02 cpu]$ pwd
> > > > > /sys/devices/system/cpu
> > > > > [root@br10p02 cpu]$ cat online
> > > > > 0-39
> > > > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat
> > > > > cpu$n/online ) -eq 1 && echo -n "$n " ; done ; echo ""
> > > > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
> > > > > 25
> > > > > 26
> > > > > 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
> > > > >
> > > > >
> > > > > While with the patch, the cpu online nodes are kept up to date
> > > > > as the cpus are brought up/down:
> > > > >
> > > > > [root@br10p02 cpu]$ pwd
> > > > > /sys/devices/system/cpu
> > > > > [root@br10p02 cpu]$ cat online
> > > > > 0-39
> > > > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat
> > > > > cpu$n/online ) -eq 1 && echo -n "$n " ; done ; echo ""
> > > > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
> > > > > 25
> > > > > 26
> > > > > 27 28 29 30 31 32 33 34 35 36 37 38 39
> > > > >
> > > > >
> > > > > Feel free to add
> > > > >
> > > > > Tested-by: Dan Streetman <[email protected]>
> > > > >
> > > >
> > > > It's a real bug in the kernel.
> > >
> > > As this has been this way for many years, I tend to think it's not
> > > all that important...
> >
> > Actually this bug was introduced by the following patch.
> >
> > commit 0902a9044fa5b7a0456ea4daacec2c2b3189ba8c
> > Author: Rafael J. Wysocki <[email protected]>
> > Date: Fri May 3 00:25:49 2013 +0200
> >
> > Driver core: Use generic offline/online for CPU offline/online
> >
> > So seems not that long :)
>
> Ok, over a year.
>
> Any reason why this information wasn't in this patch? Also, why not cc:
> the authors of that patch as well? Surely they would want to know about this,
> right?
>
> greg k-h

Thanks for your remind.
I will submit another patch to include these infos.

Best Regards,
Neil Zhang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-27 16:29:10

by Dan Streetman

[permalink] [raw]
Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug

On Sun, Oct 26, 2014 at 10:26 PM, Greg KH <[email protected]> wrote:
> On Sun, Oct 26, 2014 at 07:17:14PM -0700, Neil Zhang wrote:
>>
>>
>> > -----Original Message-----
>> > From: Greg KH [mailto:[email protected]]
>> > Sent: 2014年10月27日 9:59
>> > To: Neil Zhang
>> > Cc: Dan Streetman; [email protected]
>> > Subject: Re: [PATCH] drivers: base: update cpu offline info when do hotplug
>> >
>> > On Sun, Oct 26, 2014 at 06:43:11PM -0700, Neil Zhang wrote:
>> > > Greg,
>> > >
>> > >
>> > > > -----Original Message-----
>> > > > From: [email protected] [mailto:[email protected]] On Behalf Of
>> > > > Dan Streetman
>> > > > Sent: 2014年10月21日 1:03
>> > > > To: Neil Zhang
>> > > > Cc: Greg KH; [email protected]
>> > > > Subject: Re: [PATCH] drivers: base: update cpu offline info when do
>> > > > hotplug
>> > > >
>> > > > On Mon, Oct 20, 2014 at 3:40 AM, Neil Zhang <[email protected]> wrote:
>> > > > > Greg,
>> > > > >
>> > > > >
>> > > > > -----Original Message-----
>> > > > > From: Greg KH [mailto:[email protected]]
>> > > > > Sent: 2014年10月20日 14:48
>> > > > > To: Neil Zhang
>> > > > > Cc: [email protected]
>> > > > > Subject: Re: [PATCH] drivers: base: update cpu offline info when
>> > > > > do hotplug
>> > > > >
>> > > > > On Sun, Oct 19, 2014 at 11:39:23PM -0700, Neil Zhang wrote:
>> > > > >>> How much noise is this going to cause on a big/little system
>> > > > >>> that constantly hot unplug/plugs processors all of the time?
>> > > > >>
>> > > > >> Can you explain more what kind of noise will be introduced on a
>> > > > >> big/little
>> > > > system?
>> > > > >
>> > > > > Have you tested this on such a machine?
>> > > > >
>> > > > > I didn't have such kind of machine on hand.
>> > > > > Can anyone has such machine to verify it?
>> > > > > Thanks!
>> > > >
>> > > > I tested this on a ppc PowerVM system, using dlpar operations to
>> > > > remove/add cpus.
>> > > >
>> > > > Without this patch the cpu online nodes get out of sync with the
>> > > > main online node (and the actual state of the cpus), because they
>> > > > aren't updated as the cpus are brought up/down:
>> > > >
>> > > > [root@br10p02 cpu]$ pwd
>> > > > /sys/devices/system/cpu
>> > > > [root@br10p02 cpu]$ cat online
>> > > > 0-39
>> > > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>> > > > -eq 1 && echo -n "$n " ; done ; echo ""
>> > > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
>> > > > 26
>> > > > 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
>> > > >
>> > > >
>> > > > While with the patch, the cpu online nodes are kept up to date as
>> > > > the cpus are brought up/down:
>> > > >
>> > > > [root@br10p02 cpu]$ pwd
>> > > > /sys/devices/system/cpu
>> > > > [root@br10p02 cpu]$ cat online
>> > > > 0-39
>> > > > [root@br10p02 cpu]$ for n in {0..47} ; do test $( cat cpu$n/online )
>> > > > -eq 1 && echo -n "$n " ; done ; echo ""
>> > > > 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
>> > > > 26
>> > > > 27 28 29 30 31 32 33 34 35 36 37 38 39
>> > > >
>> > > >
>> > > > Feel free to add
>> > > >
>> > > > Tested-by: Dan Streetman <[email protected]>
>> > > >
>> > >
>> > > It's a real bug in the kernel.
>> >
>> > As this has been this way for many years, I tend to think it's not all that
>> > important...
>>
>> Actually this bug was introduced by the following patch.
>>
>> commit 0902a9044fa5b7a0456ea4daacec2c2b3189ba8c
>> Author: Rafael J. Wysocki <[email protected]>
>> Date: Fri May 3 00:25:49 2013 +0200
>>
>> Driver core: Use generic offline/online for CPU offline/online
>>
>> So seems not that long :)
>
> Ok, over a year.
>
> Any reason why this information wasn't in this patch? Also, why not cc:
> the authors of that patch as well? Surely they would want to know about
> this, right?

To add a bit more info to this, while the PPC (on PowerVM) method for
cpu hotplug, in arch/powerpc/platform/pseries/dlpar.c
dlpar_offline_cpu(), does require this patch because it only takes a
cpu offline during hot remove, the x86/acpi code appears to be
different as it fully unregisters a cpu during hot remove, in
drivers/acpi/acpi_processor.c acpi_processor_remove() - so I believe
the entire cpuN directory would be removed. I don't have a
hw-hotpluggable x86 system though, and it doesn't look like qemu
really supports cpu hotremove yet, so I can't test that. I don't know
how other archs handle cpu hotplug.

Also, the ppc pseries dlpar code may be changed in the future to
unregister the cpu instead of only setting it offline; I've cc'ed
Nathan who probably would be doing that.

But regardless, after commit 0902a90, when a cpu is being taken up or
down by anything other than generic offline/online code, the cpu's
->offline state does need to be updated. If not by a hotplug listener
like this, then possibly by kernel/cpu.c set_cpu_online()...



>
> greg k-h