diff mbox series

[V2] MIPS: CPU#0 is not hotpluggable

Message ID 1594896024-16624-1-git-send-email-chenhc@lemote.com (mailing list archive)
State Mainlined
Commit 9cce844abf07b683cff5f0273977d5f8d0af94c7
Headers show
Series [V2] MIPS: CPU#0 is not hotpluggable | expand

Commit Message

Huacai Chen July 16, 2020, 10:40 a.m. UTC
Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
/system/cpu/cpu0/online which confuses some user-space tools.

Cc: stable@vger.kernel.org
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
 arch/mips/kernel/topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Huacai Chen July 25, 2020, 6:37 a.m. UTC | #1
Hi, Thomas,

What do you think about this patch? Other archs also do the same thing
except those support hotplug CPU#0.

grep hotpluggable arch -rwI
arch/riscv/kernel/setup.c:        cpu->hotpluggable = cpu_has_hotplug(i);
arch/powerpc/kernel/sysfs.c:    BUG_ON(!c->hotpluggable);
arch/powerpc/kernel/sysfs.c:            c->hotpluggable = 1;
arch/powerpc/kernel/sysfs.c:        if (cpu_online(cpu) || c->hotpluggable) {
arch/arm/kernel/setup.c:        cpuinfo->cpu.hotpluggable =
platform_can_hotplug_cpu(cpu);
arch/sh/kernel/topology.c:        c->hotpluggable = 1;
arch/ia64/kernel/topology.c:     * CPEI target, then it is hotpluggable
arch/ia64/kernel/topology.c:        sysfs_cpus[num].cpu.hotpluggable = 1;
arch/xtensa/kernel/setup.c:        cpu->hotpluggable = !!i;
arch/s390/kernel/smp.c:    c->hotpluggable = 1;
arch/mips/kernel/topology.c:        c->hotpluggable = 1;
arch/arm64/kernel/cpuinfo.c: * In case the boot CPU is hotpluggable,
we record its initial state and
arch/arm64/kernel/setup.c:        cpu->hotpluggable = cpu_can_disable(i);
arch/x86/kernel/topology.c:        per_cpu(cpu_devices,
num).cpu.hotpluggable = 1;

On Thu, Jul 16, 2020 at 6:38 PM Huacai Chen <chenhc@lemote.com> wrote:
>
> Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
> /system/cpu/cpu0/online which confuses some user-space tools.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
>  arch/mips/kernel/topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/topology.c b/arch/mips/kernel/topology.c
> index cd3e1f8..08ad637 100644
> --- a/arch/mips/kernel/topology.c
> +++ b/arch/mips/kernel/topology.c
> @@ -20,7 +20,7 @@ static int __init topology_init(void)
>         for_each_present_cpu(i) {
>                 struct cpu *c = &per_cpu(cpu_devices, i);
>
> -               c->hotpluggable = 1;
> +               c->hotpluggable = !!i;
>                 ret = register_cpu(c, i);
>                 if (ret)
>                         printk(KERN_WARNING "topology_init: register_cpu %d "
> --
> 2.7.0
>
Greg KH July 25, 2020, 6:49 a.m. UTC | #2
On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
> Hi, Thomas,
> 
> What do you think about this patch? Other archs also do the same thing
> except those support hotplug CPU#0.
> 
> grep hotpluggable arch -rwI
> arch/riscv/kernel/setup.c:        cpu->hotpluggable = cpu_has_hotplug(i);
> arch/powerpc/kernel/sysfs.c:    BUG_ON(!c->hotpluggable);
> arch/powerpc/kernel/sysfs.c:            c->hotpluggable = 1;
> arch/powerpc/kernel/sysfs.c:        if (cpu_online(cpu) || c->hotpluggable) {
> arch/arm/kernel/setup.c:        cpuinfo->cpu.hotpluggable =
> platform_can_hotplug_cpu(cpu);
> arch/sh/kernel/topology.c:        c->hotpluggable = 1;
> arch/ia64/kernel/topology.c:     * CPEI target, then it is hotpluggable
> arch/ia64/kernel/topology.c:        sysfs_cpus[num].cpu.hotpluggable = 1;
> arch/xtensa/kernel/setup.c:        cpu->hotpluggable = !!i;
> arch/s390/kernel/smp.c:    c->hotpluggable = 1;
> arch/mips/kernel/topology.c:        c->hotpluggable = 1;
> arch/arm64/kernel/cpuinfo.c: * In case the boot CPU is hotpluggable,
> we record its initial state and
> arch/arm64/kernel/setup.c:        cpu->hotpluggable = cpu_can_disable(i);
> arch/x86/kernel/topology.c:        per_cpu(cpu_devices,
> num).cpu.hotpluggable = 1;
> 
> On Thu, Jul 16, 2020 at 6:38 PM Huacai Chen <chenhc@lemote.com> wrote:
> >
> > Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
> > /system/cpu/cpu0/online which confuses some user-space tools.

What userspace tools are confused by this?  They should be able to
handle a cpu not being able to be removed, right?


> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Huacai Chen <chenhc@lemote.com>
> > ---
> >  arch/mips/kernel/topology.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/kernel/topology.c b/arch/mips/kernel/topology.c
> > index cd3e1f8..08ad637 100644
> > --- a/arch/mips/kernel/topology.c
> > +++ b/arch/mips/kernel/topology.c
> > @@ -20,7 +20,7 @@ static int __init topology_init(void)
> >         for_each_present_cpu(i) {
> >                 struct cpu *c = &per_cpu(cpu_devices, i);
> >
> > -               c->hotpluggable = 1;
> > +               c->hotpluggable = !!i;

Seems to be the same as what xtensa did, so it's probably not a big
deal.

thanks,

greg k-h
Huacai Chen July 25, 2020, 6:57 a.m. UTC | #3
Hi Greg,

On Sat, Jul 25, 2020 at 2:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
> > Hi, Thomas,
> >
> > What do you think about this patch? Other archs also do the same thing
> > except those support hotplug CPU#0.
> >
> > grep hotpluggable arch -rwI
> > arch/riscv/kernel/setup.c:        cpu->hotpluggable = cpu_has_hotplug(i);
> > arch/powerpc/kernel/sysfs.c:    BUG_ON(!c->hotpluggable);
> > arch/powerpc/kernel/sysfs.c:            c->hotpluggable = 1;
> > arch/powerpc/kernel/sysfs.c:        if (cpu_online(cpu) || c->hotpluggable) {
> > arch/arm/kernel/setup.c:        cpuinfo->cpu.hotpluggable =
> > platform_can_hotplug_cpu(cpu);
> > arch/sh/kernel/topology.c:        c->hotpluggable = 1;
> > arch/ia64/kernel/topology.c:     * CPEI target, then it is hotpluggable
> > arch/ia64/kernel/topology.c:        sysfs_cpus[num].cpu.hotpluggable = 1;
> > arch/xtensa/kernel/setup.c:        cpu->hotpluggable = !!i;
> > arch/s390/kernel/smp.c:    c->hotpluggable = 1;
> > arch/mips/kernel/topology.c:        c->hotpluggable = 1;
> > arch/arm64/kernel/cpuinfo.c: * In case the boot CPU is hotpluggable,
> > we record its initial state and
> > arch/arm64/kernel/setup.c:        cpu->hotpluggable = cpu_can_disable(i);
> > arch/x86/kernel/topology.c:        per_cpu(cpu_devices,
> > num).cpu.hotpluggable = 1;
> >
> > On Thu, Jul 16, 2020 at 6:38 PM Huacai Chen <chenhc@lemote.com> wrote:
> > >
> > > Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
> > > /system/cpu/cpu0/online which confuses some user-space tools.
>
> What userspace tools are confused by this?  They should be able to
> handle a cpu not being able to be removed, right?
It causes ltp's "hotplug" test fails, and ltp considers CPUs with a
"online" node be hotpluggable.

>
>
> > >
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Huacai Chen <chenhc@lemote.com>
> > > ---
> > >  arch/mips/kernel/topology.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/mips/kernel/topology.c b/arch/mips/kernel/topology.c
> > > index cd3e1f8..08ad637 100644
> > > --- a/arch/mips/kernel/topology.c
> > > +++ b/arch/mips/kernel/topology.c
> > > @@ -20,7 +20,7 @@ static int __init topology_init(void)
> > >         for_each_present_cpu(i) {
> > >                 struct cpu *c = &per_cpu(cpu_devices, i);
> > >
> > > -               c->hotpluggable = 1;
> > > +               c->hotpluggable = !!i;
>
> Seems to be the same as what xtensa did, so it's probably not a big
> deal.
>
> thanks,
>
> greg k-h
Greg KH July 25, 2020, 7:45 a.m. UTC | #4
On Sat, Jul 25, 2020 at 02:57:31PM +0800, Huacai Chen wrote:
> Hi Greg,
> 
> On Sat, Jul 25, 2020 at 2:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
> > > Hi, Thomas,
> > >
> > > What do you think about this patch? Other archs also do the same thing
> > > except those support hotplug CPU#0.
> > >
> > > grep hotpluggable arch -rwI
> > > arch/riscv/kernel/setup.c:        cpu->hotpluggable = cpu_has_hotplug(i);
> > > arch/powerpc/kernel/sysfs.c:    BUG_ON(!c->hotpluggable);
> > > arch/powerpc/kernel/sysfs.c:            c->hotpluggable = 1;
> > > arch/powerpc/kernel/sysfs.c:        if (cpu_online(cpu) || c->hotpluggable) {
> > > arch/arm/kernel/setup.c:        cpuinfo->cpu.hotpluggable =
> > > platform_can_hotplug_cpu(cpu);
> > > arch/sh/kernel/topology.c:        c->hotpluggable = 1;
> > > arch/ia64/kernel/topology.c:     * CPEI target, then it is hotpluggable
> > > arch/ia64/kernel/topology.c:        sysfs_cpus[num].cpu.hotpluggable = 1;
> > > arch/xtensa/kernel/setup.c:        cpu->hotpluggable = !!i;
> > > arch/s390/kernel/smp.c:    c->hotpluggable = 1;
> > > arch/mips/kernel/topology.c:        c->hotpluggable = 1;
> > > arch/arm64/kernel/cpuinfo.c: * In case the boot CPU is hotpluggable,
> > > we record its initial state and
> > > arch/arm64/kernel/setup.c:        cpu->hotpluggable = cpu_can_disable(i);
> > > arch/x86/kernel/topology.c:        per_cpu(cpu_devices,
> > > num).cpu.hotpluggable = 1;
> > >
> > > On Thu, Jul 16, 2020 at 6:38 PM Huacai Chen <chenhc@lemote.com> wrote:
> > > >
> > > > Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
> > > > /system/cpu/cpu0/online which confuses some user-space tools.
> >
> > What userspace tools are confused by this?  They should be able to
> > handle a cpu not being able to be removed, right?
> It causes ltp's "hotplug" test fails, and ltp considers CPUs with a
> "online" node be hotpluggable.

Is that always true?
Huacai Chen July 25, 2020, 8:29 a.m. UTC | #5
Hi, Greg,

On Sat, Jul 25, 2020 at 3:45 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jul 25, 2020 at 02:57:31PM +0800, Huacai Chen wrote:
> > Hi Greg,
> >
> > On Sat, Jul 25, 2020 at 2:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
> > > > Hi, Thomas,
> > > >
> > > > What do you think about this patch? Other archs also do the same thing
> > > > except those support hotplug CPU#0.
> > > >
> > > > grep hotpluggable arch -rwI
> > > > arch/riscv/kernel/setup.c:        cpu->hotpluggable = cpu_has_hotplug(i);
> > > > arch/powerpc/kernel/sysfs.c:    BUG_ON(!c->hotpluggable);
> > > > arch/powerpc/kernel/sysfs.c:            c->hotpluggable = 1;
> > > > arch/powerpc/kernel/sysfs.c:        if (cpu_online(cpu) || c->hotpluggable) {
> > > > arch/arm/kernel/setup.c:        cpuinfo->cpu.hotpluggable =
> > > > platform_can_hotplug_cpu(cpu);
> > > > arch/sh/kernel/topology.c:        c->hotpluggable = 1;
> > > > arch/ia64/kernel/topology.c:     * CPEI target, then it is hotpluggable
> > > > arch/ia64/kernel/topology.c:        sysfs_cpus[num].cpu.hotpluggable = 1;
> > > > arch/xtensa/kernel/setup.c:        cpu->hotpluggable = !!i;
> > > > arch/s390/kernel/smp.c:    c->hotpluggable = 1;
> > > > arch/mips/kernel/topology.c:        c->hotpluggable = 1;
> > > > arch/arm64/kernel/cpuinfo.c: * In case the boot CPU is hotpluggable,
> > > > we record its initial state and
> > > > arch/arm64/kernel/setup.c:        cpu->hotpluggable = cpu_can_disable(i);
> > > > arch/x86/kernel/topology.c:        per_cpu(cpu_devices,
> > > > num).cpu.hotpluggable = 1;
> > > >
> > > > On Thu, Jul 16, 2020 at 6:38 PM Huacai Chen <chenhc@lemote.com> wrote:
> > > > >
> > > > > Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
> > > > > /system/cpu/cpu0/online which confuses some user-space tools.
> > >
> > > What userspace tools are confused by this?  They should be able to
> > > handle a cpu not being able to be removed, right?
> > It causes ltp's "hotplug" test fails, and ltp considers CPUs with a
> > "online" node be hotpluggable.
>
> Is that always true?
Yes, someone who meet the same problem report a bug to LTP, and LTP
maintainer said that this should be fixed in kernel.

Huacai
Greg KH July 25, 2020, 8:57 a.m. UTC | #6
On Sat, Jul 25, 2020 at 04:29:28PM +0800, Huacai Chen wrote:
> Hi, Greg,
> 
> On Sat, Jul 25, 2020 at 3:45 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Jul 25, 2020 at 02:57:31PM +0800, Huacai Chen wrote:
> > > Hi Greg,
> > >
> > > On Sat, Jul 25, 2020 at 2:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
> > > > > Hi, Thomas,
> > > > >
> > > > > What do you think about this patch? Other archs also do the same thing
> > > > > except those support hotplug CPU#0.
> > > > >
> > > > > grep hotpluggable arch -rwI
> > > > > arch/riscv/kernel/setup.c:        cpu->hotpluggable = cpu_has_hotplug(i);
> > > > > arch/powerpc/kernel/sysfs.c:    BUG_ON(!c->hotpluggable);
> > > > > arch/powerpc/kernel/sysfs.c:            c->hotpluggable = 1;
> > > > > arch/powerpc/kernel/sysfs.c:        if (cpu_online(cpu) || c->hotpluggable) {
> > > > > arch/arm/kernel/setup.c:        cpuinfo->cpu.hotpluggable =
> > > > > platform_can_hotplug_cpu(cpu);
> > > > > arch/sh/kernel/topology.c:        c->hotpluggable = 1;
> > > > > arch/ia64/kernel/topology.c:     * CPEI target, then it is hotpluggable
> > > > > arch/ia64/kernel/topology.c:        sysfs_cpus[num].cpu.hotpluggable = 1;
> > > > > arch/xtensa/kernel/setup.c:        cpu->hotpluggable = !!i;
> > > > > arch/s390/kernel/smp.c:    c->hotpluggable = 1;
> > > > > arch/mips/kernel/topology.c:        c->hotpluggable = 1;
> > > > > arch/arm64/kernel/cpuinfo.c: * In case the boot CPU is hotpluggable,
> > > > > we record its initial state and
> > > > > arch/arm64/kernel/setup.c:        cpu->hotpluggable = cpu_can_disable(i);
> > > > > arch/x86/kernel/topology.c:        per_cpu(cpu_devices,
> > > > > num).cpu.hotpluggable = 1;
> > > > >
> > > > > On Thu, Jul 16, 2020 at 6:38 PM Huacai Chen <chenhc@lemote.com> wrote:
> > > > > >
> > > > > > Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
> > > > > > /system/cpu/cpu0/online which confuses some user-space tools.
> > > >
> > > > What userspace tools are confused by this?  They should be able to
> > > > handle a cpu not being able to be removed, right?
> > > It causes ltp's "hotplug" test fails, and ltp considers CPUs with a
> > > "online" node be hotpluggable.
> >
> > Is that always true?
> Yes, someone who meet the same problem report a bug to LTP, and LTP
> maintainer said that this should be fixed in kernel.

So the action _always_ has to succeed and can never return an error?
That feels wrong even for normal systems.

thanks,

greg k-h
Jiaxun Yang July 26, 2020, 2:17 a.m. UTC | #7
在 2020/7/25 下午4:57, Greg KH 写道:
> On Sat, Jul 25, 2020 at 04:29:28PM +0800, Huacai Chen wrote:
>> Hi, Greg,
>>
>> On Sat, Jul 25, 2020 at 3:45 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>> On Sat, Jul 25, 2020 at 02:57:31PM +0800, Huacai Chen wrote:
>>>> Hi Greg,
>>>>
>>>> On Sat, Jul 25, 2020 at 2:49 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>> On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
>>>>>> Hi, Thomas,
>>>>>>
>>>>>> What do you think about this patch? Other archs also do the same thing
>>>>>> except those support hotplug CPU#0.
>>>>>>
>>>>>> grep hotpluggable arch -rwI
>>>>>> arch/riscv/kernel/setup.c:        cpu->hotpluggable = cpu_has_hotplug(i);
>>>>>> arch/powerpc/kernel/sysfs.c:    BUG_ON(!c->hotpluggable);
>>>>>> arch/powerpc/kernel/sysfs.c:            c->hotpluggable = 1;
>>>>>> arch/powerpc/kernel/sysfs.c:        if (cpu_online(cpu) || c->hotpluggable) {
>>>>>> arch/arm/kernel/setup.c:        cpuinfo->cpu.hotpluggable =
>>>>>> platform_can_hotplug_cpu(cpu);
>>>>>> arch/sh/kernel/topology.c:        c->hotpluggable = 1;
>>>>>> arch/ia64/kernel/topology.c:     * CPEI target, then it is hotpluggable
>>>>>> arch/ia64/kernel/topology.c:        sysfs_cpus[num].cpu.hotpluggable = 1;
>>>>>> arch/xtensa/kernel/setup.c:        cpu->hotpluggable = !!i;
>>>>>> arch/s390/kernel/smp.c:    c->hotpluggable = 1;
>>>>>> arch/mips/kernel/topology.c:        c->hotpluggable = 1;
>>>>>> arch/arm64/kernel/cpuinfo.c: * In case the boot CPU is hotpluggable,
>>>>>> we record its initial state and
>>>>>> arch/arm64/kernel/setup.c:        cpu->hotpluggable = cpu_can_disable(i);
>>>>>> arch/x86/kernel/topology.c:        per_cpu(cpu_devices,
>>>>>> num).cpu.hotpluggable = 1;
>>>>>>
>>>>>> On Thu, Jul 16, 2020 at 6:38 PM Huacai Chen <chenhc@lemote.com> wrote:
>>>>>>> Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
>>>>>>> /system/cpu/cpu0/online which confuses some user-space tools.
>>>>> What userspace tools are confused by this?  They should be able to
>>>>> handle a cpu not being able to be removed, right?
>>>> It causes ltp's "hotplug" test fails, and ltp considers CPUs with a
>>>> "online" node be hotpluggable.
>>> Is that always true?
>> Yes, someone who meet the same problem report a bug to LTP, and LTP
>> maintainer said that this should be fixed in kernel.
> So the action _always_ has to succeed and can never return an error?
> That feels wrong even for normal systems.

If that returns error then it means kernel's hotplug function is not 
working as
expected. Thus, LTP as a testsuit will consider the error as a indicator 
of malfunction
kernel.

Thanks

- Jiaxun


>
> thanks,
>
> greg k-h
Thomas Bogendoerfer July 26, 2020, 8:25 a.m. UTC | #8
On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
> Hi, Thomas,
> 
> What do you think about this patch? Other archs also do the same thing
> except those support hotplug CPU#0.

I'm ok with the patch, I'm just wondering if this is a hardware or
software limitation. If it's the latter, what needs to be done to
support it ?

Thomas.
Thomas Bogendoerfer July 26, 2020, 8:25 a.m. UTC | #9
On Thu, Jul 16, 2020 at 06:40:23PM +0800, Huacai Chen wrote:
> Now CPU#0 is not hotpluggable on MIPS, so prevent to create /sys/devices
> /system/cpu/cpu0/online which confuses some user-space tools.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> ---
>  arch/mips/kernel/topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

applied to mips-next.

Thomas.
Jiaxun Yang July 26, 2020, 11:30 a.m. UTC | #10
在 2020/7/26 下午4:25, Thomas Bogendoerfer 写道:
> On Sat, Jul 25, 2020 at 02:37:52PM +0800, Huacai Chen wrote:
>> Hi, Thomas,
>>
>> What do you think about this patch? Other archs also do the same thing
>> except those support hotplug CPU#0.
> I'm ok with the patch, I'm just wondering if this is a hardware or
> software limitation. If it's the latter, what needs to be done to
> support it ?

It should be a software limitation, x86 have already deal with that [1].
But there is no reason to spend extra effort on CPU0 hotplug. I don't 
think any user is expecting this feature.

Thanks.

[1]: 
https://events.static.linuxfound.org/sites/events/files/lcjpcojp13_fenghua.pdf

- Jiaxun
>
> Thomas.
>
diff mbox series

Patch

diff --git a/arch/mips/kernel/topology.c b/arch/mips/kernel/topology.c
index cd3e1f8..08ad637 100644
--- a/arch/mips/kernel/topology.c
+++ b/arch/mips/kernel/topology.c
@@ -20,7 +20,7 @@  static int __init topology_init(void)
 	for_each_present_cpu(i) {
 		struct cpu *c = &per_cpu(cpu_devices, i);
 
-		c->hotpluggable = 1;
+		c->hotpluggable = !!i;
 		ret = register_cpu(c, i);
 		if (ret)
 			printk(KERN_WARNING "topology_init: register_cpu %d "