* [PATCH] LoongArch: Do not create sysfs control file for io master CPUs
@ 2022-10-08 8:59 Tiezhu Yang
2022-10-08 9:27 ` WANG Xuerui
0 siblings, 1 reply; 5+ messages in thread
From: Tiezhu Yang @ 2022-10-08 8:59 UTC (permalink / raw
To: Huacai Chen, WANG Xuerui; +Cc: loongarch, linux-kernel
Now io master CPUs are not hotpluggable on LoongArch, in the current code,
only /sys/devices/system/cpu/cpu0/online is not created, let us set the
hotpluggable field of all the io master CPUs as 0, then prevent to create
sysfs control file for the other io master CPUs which confuses some user
space tools. This is similar with commit 9cce844abf07 ("MIPS: CPU#0 is not
hotpluggable").
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
arch/loongarch/kernel/smp.c | 8 --------
arch/loongarch/kernel/topology.c | 12 +++++++++++-
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
index b5fab30..ef89292 100644
--- a/arch/loongarch/kernel/smp.c
+++ b/arch/loongarch/kernel/smp.c
@@ -240,19 +240,11 @@ void loongson3_smp_finish(void)
#ifdef CONFIG_HOTPLUG_CPU
-static bool io_master(int cpu)
-{
- return test_bit(cpu, &loongson_sysconf.cores_io_master);
-}
-
int loongson3_cpu_disable(void)
{
unsigned long flags;
unsigned int cpu = smp_processor_id();
- if (io_master(cpu))
- return -EBUSY;
-
#ifdef CONFIG_NUMA
numa_remove_cpu(cpu);
#endif
diff --git a/arch/loongarch/kernel/topology.c b/arch/loongarch/kernel/topology.c
index ab1a75c..7e7a77f 100644
--- a/arch/loongarch/kernel/topology.c
+++ b/arch/loongarch/kernel/topology.c
@@ -5,6 +5,7 @@
#include <linux/node.h>
#include <linux/nodemask.h>
#include <linux/percpu.h>
+#include <asm/bootinfo.h>
static DEFINE_PER_CPU(struct cpu, cpu_devices);
@@ -33,6 +34,11 @@ void arch_unregister_cpu(int cpu)
EXPORT_SYMBOL(arch_unregister_cpu);
#endif
+static bool io_master(int cpu)
+{
+ return test_bit(cpu, &loongson_sysconf.cores_io_master);
+}
+
static int __init topology_init(void)
{
int i, ret;
@@ -40,7 +46,11 @@ static int __init topology_init(void)
for_each_present_cpu(i) {
struct cpu *c = &per_cpu(cpu_devices, i);
- c->hotpluggable = !!i;
+ if (io_master(i))
+ c->hotpluggable = 0;
+ else
+ c->hotpluggable = 1;
+
ret = register_cpu(c, i);
if (ret < 0)
pr_warn("topology_init: register_cpu %d failed (%d)\n", i, ret);
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] LoongArch: Do not create sysfs control file for io master CPUs
2022-10-08 8:59 [PATCH] LoongArch: Do not create sysfs control file for io master CPUs Tiezhu Yang
@ 2022-10-08 9:27 ` WANG Xuerui
2022-10-08 9:51 ` Tiezhu Yang
0 siblings, 1 reply; 5+ messages in thread
From: WANG Xuerui @ 2022-10-08 9:27 UTC (permalink / raw
To: Tiezhu Yang, Huacai Chen; +Cc: loongarch, linux-kernel
On 2022/10/8 16:59, Tiezhu Yang wrote:
> Now io master CPUs are not hotpluggable on LoongArch, in the current code,
> only /sys/devices/system/cpu/cpu0/online is not created, let us set the
> hotpluggable field of all the io master CPUs as 0, then prevent to create
> sysfs control file for the other io master CPUs which confuses some user
> space tools. This is similar with commit 9cce844abf07 ("MIPS: CPU#0 is not
> hotpluggable").
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
> arch/loongarch/kernel/smp.c | 8 --------
> arch/loongarch/kernel/topology.c | 12 +++++++++++-
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> index b5fab30..ef89292 100644
> --- a/arch/loongarch/kernel/smp.c
> +++ b/arch/loongarch/kernel/smp.c
> @@ -240,19 +240,11 @@ void loongson3_smp_finish(void)
>
> #ifdef CONFIG_HOTPLUG_CPU
>
> -static bool io_master(int cpu)
> -{
> - return test_bit(cpu, &loongson_sysconf.cores_io_master);
> -}
> -
> int loongson3_cpu_disable(void)
> {
> unsigned long flags;
> unsigned int cpu = smp_processor_id();
>
> - if (io_master(cpu))
> - return -EBUSY;
> -
Could this get invoked from somewhere other than the sysfs entries that
"confuse user-space tools", e.g. from somewhere else in kernel land? If
so (or if we can't rule out the possibility) keeping the guard here
might prove more prudent.
> #ifdef CONFIG_NUMA
> numa_remove_cpu(cpu);
> #endif
> diff --git a/arch/loongarch/kernel/topology.c b/arch/loongarch/kernel/topology.c
> index ab1a75c..7e7a77f 100644
> --- a/arch/loongarch/kernel/topology.c
> +++ b/arch/loongarch/kernel/topology.c
> @@ -5,6 +5,7 @@
> #include <linux/node.h>
> #include <linux/nodemask.h>
> #include <linux/percpu.h>
> +#include <asm/bootinfo.h>
>
> static DEFINE_PER_CPU(struct cpu, cpu_devices);
>
> @@ -33,6 +34,11 @@ void arch_unregister_cpu(int cpu)
> EXPORT_SYMBOL(arch_unregister_cpu);
> #endif
>
> +static bool io_master(int cpu)
> +{
> + return test_bit(cpu, &loongson_sysconf.cores_io_master);
> +}
> +
> static int __init topology_init(void)
> {
> int i, ret;
> @@ -40,7 +46,11 @@ static int __init topology_init(void)
> for_each_present_cpu(i) {
> struct cpu *c = &per_cpu(cpu_devices, i);
>
> - c->hotpluggable = !!i;
> + if (io_master(i))
> + c->hotpluggable = 0;
> + else
> + c->hotpluggable = 1;
> +
This is just "c->hotpluggable = !io_master(i);".
> ret = register_cpu(c, i);
> if (ret < 0)
> pr_warn("topology_init: register_cpu %d failed (%d)\n", i, ret);
Other changes should be okay as they are in line with the previous MIPS
change you referenced, but let's see what Huacai thinks.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] LoongArch: Do not create sysfs control file for io master CPUs
2022-10-08 9:27 ` WANG Xuerui
@ 2022-10-08 9:51 ` Tiezhu Yang
2022-10-08 10:44 ` Tiezhu Yang
0 siblings, 1 reply; 5+ messages in thread
From: Tiezhu Yang @ 2022-10-08 9:51 UTC (permalink / raw
To: WANG Xuerui, Huacai Chen; +Cc: loongarch, linux-kernel
On 10/08/2022 05:27 PM, WANG Xuerui wrote:
> On 2022/10/8 16:59, Tiezhu Yang wrote:
>> Now io master CPUs are not hotpluggable on LoongArch, in the current
>> code,
>> only /sys/devices/system/cpu/cpu0/online is not created, let us set the
>> hotpluggable field of all the io master CPUs as 0, then prevent to create
>> sysfs control file for the other io master CPUs which confuses some user
>> space tools. This is similar with commit 9cce844abf07 ("MIPS: CPU#0 is
>> not
>> hotpluggable").
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>> arch/loongarch/kernel/smp.c | 8 --------
>> arch/loongarch/kernel/topology.c | 12 +++++++++++-
>> 2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>> index b5fab30..ef89292 100644
>> --- a/arch/loongarch/kernel/smp.c
>> +++ b/arch/loongarch/kernel/smp.c
>> @@ -240,19 +240,11 @@ void loongson3_smp_finish(void)
>> #ifdef CONFIG_HOTPLUG_CPU
>> -static bool io_master(int cpu)
>> -{
>> - return test_bit(cpu, &loongson_sysconf.cores_io_master);
>> -}
>> -
>> int loongson3_cpu_disable(void)
>> {
>> unsigned long flags;
>> unsigned int cpu = smp_processor_id();
>> - if (io_master(cpu))
>> - return -EBUSY;
>> -
>
> Could this get invoked from somewhere other than the sysfs entries that
> "confuse user-space tools", e.g. from somewhere else in kernel land? If
> so (or if we can't rule out the possibility) keeping the guard here
> might prove more prudent.
>
If c->hotpluggable is 0, it will not generate a control file in sysfs
for this CPU, for example:
[root@linux loongson]# cat /sys/devices/system/cpu/cpu0/online
cat: /sys/devices/system/cpu/cpu0/online: No such file or directory
[root@linux loongson]# echo 0 > /sys/devices/system/cpu/cpu0/online
bash: /sys/devices/system/cpu/cpu0/online: Permission denied
So no need to check it here, just remove the code.
This was done in commit cbab54d9c2b2 ("MIPS: No need to check CPU 0 in
{loongson3,bmips,octeon}_cpu_disable()").
>> #ifdef CONFIG_NUMA
>> numa_remove_cpu(cpu);
>> #endif
>> diff --git a/arch/loongarch/kernel/topology.c
>> b/arch/loongarch/kernel/topology.c
>> index ab1a75c..7e7a77f 100644
>> --- a/arch/loongarch/kernel/topology.c
>> +++ b/arch/loongarch/kernel/topology.c
>> @@ -5,6 +5,7 @@
>> #include <linux/node.h>
>> #include <linux/nodemask.h>
>> #include <linux/percpu.h>
>> +#include <asm/bootinfo.h>
>> static DEFINE_PER_CPU(struct cpu, cpu_devices);
>> @@ -33,6 +34,11 @@ void arch_unregister_cpu(int cpu)
>> EXPORT_SYMBOL(arch_unregister_cpu);
>> #endif
>> +static bool io_master(int cpu)
>> +{
>> + return test_bit(cpu, &loongson_sysconf.cores_io_master);
>> +}
>> +
>> static int __init topology_init(void)
>> {
>> int i, ret;
>> @@ -40,7 +46,11 @@ static int __init topology_init(void)
>> for_each_present_cpu(i) {
>> struct cpu *c = &per_cpu(cpu_devices, i);
>> - c->hotpluggable = !!i;
>> + if (io_master(i))
>> + c->hotpluggable = 0;
>> + else
>> + c->hotpluggable = 1;
>> +
>
> This is just "c->hotpluggable = !io_master(i);".
>
Yes, I am OK either way, if it is necessary to send v2,
please let me know.
>> ret = register_cpu(c, i);
>> if (ret < 0)
>> pr_warn("topology_init: register_cpu %d failed (%d)\n",
>> i, ret);
> Other changes should be okay as they are in line with the previous MIPS
> change you referenced, but let's see what Huacai thinks.
>
Thanks,
Tiezhu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] LoongArch: Do not create sysfs control file for io master CPUs
2022-10-08 9:51 ` Tiezhu Yang
@ 2022-10-08 10:44 ` Tiezhu Yang
2022-10-08 14:58 ` Huacai Chen
0 siblings, 1 reply; 5+ messages in thread
From: Tiezhu Yang @ 2022-10-08 10:44 UTC (permalink / raw
To: WANG Xuerui, Huacai Chen; +Cc: loongarch, linux-kernel
On 10/08/2022 05:51 PM, Tiezhu Yang wrote:
>
>
> On 10/08/2022 05:27 PM, WANG Xuerui wrote:
>> On 2022/10/8 16:59, Tiezhu Yang wrote:
>>> Now io master CPUs are not hotpluggable on LoongArch, in the current
>>> code,
>>> only /sys/devices/system/cpu/cpu0/online is not created, let us set the
>>> hotpluggable field of all the io master CPUs as 0, then prevent to
>>> create
>>> sysfs control file for the other io master CPUs which confuses some user
>>> space tools. This is similar with commit 9cce844abf07 ("MIPS: CPU#0 is
>>> not
>>> hotpluggable").
>>>
>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>> ---
>>> arch/loongarch/kernel/smp.c | 8 --------
>>> arch/loongarch/kernel/topology.c | 12 +++++++++++-
>>> 2 files changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>> index b5fab30..ef89292 100644
>>> --- a/arch/loongarch/kernel/smp.c
>>> +++ b/arch/loongarch/kernel/smp.c
>>> @@ -240,19 +240,11 @@ void loongson3_smp_finish(void)
>>> #ifdef CONFIG_HOTPLUG_CPU
>>> -static bool io_master(int cpu)
>>> -{
>>> - return test_bit(cpu, &loongson_sysconf.cores_io_master);
>>> -}
>>> -
>>> int loongson3_cpu_disable(void)
>>> {
>>> unsigned long flags;
>>> unsigned int cpu = smp_processor_id();
>>> - if (io_master(cpu))
>>> - return -EBUSY;
>>> -
>>
>> Could this get invoked from somewhere other than the sysfs entries that
>> "confuse user-space tools", e.g. from somewhere else in kernel land? If
>> so (or if we can't rule out the possibility) keeping the guard here
>> might prove more prudent.
>>
takedown_cpu() kernel/cpu.c
take_cpu_down() kernel/cpu.c
__cpu_disable() arch/loongarch/include/asm/smp.h
loongson3_cpu_disable() arch/loongarch/kernel/smp.c
So I think you are right, keeping the guard here might prove more
prudent, then it is better move io_master() to a header file that
can be used in smp.c and topology.c.
Let us wait for more review comments, thank you.
>
> If c->hotpluggable is 0, it will not generate a control file in sysfs
> for this CPU, for example:
>
> [root@linux loongson]# cat /sys/devices/system/cpu/cpu0/online
> cat: /sys/devices/system/cpu/cpu0/online: No such file or directory
> [root@linux loongson]# echo 0 > /sys/devices/system/cpu/cpu0/online
> bash: /sys/devices/system/cpu/cpu0/online: Permission denied
>
> So no need to check it here, just remove the code.
>
> This was done in commit cbab54d9c2b2 ("MIPS: No need to check CPU 0 in
> {loongson3,bmips,octeon}_cpu_disable()").
>
>>> #ifdef CONFIG_NUMA
>>> numa_remove_cpu(cpu);
>>> #endif
>>> diff --git a/arch/loongarch/kernel/topology.c
>>> b/arch/loongarch/kernel/topology.c
>>> index ab1a75c..7e7a77f 100644
>>> --- a/arch/loongarch/kernel/topology.c
>>> +++ b/arch/loongarch/kernel/topology.c
>>> @@ -5,6 +5,7 @@
>>> #include <linux/node.h>
>>> #include <linux/nodemask.h>
>>> #include <linux/percpu.h>
>>> +#include <asm/bootinfo.h>
>>> static DEFINE_PER_CPU(struct cpu, cpu_devices);
>>> @@ -33,6 +34,11 @@ void arch_unregister_cpu(int cpu)
>>> EXPORT_SYMBOL(arch_unregister_cpu);
>>> #endif
>>> +static bool io_master(int cpu)
>>> +{
>>> + return test_bit(cpu, &loongson_sysconf.cores_io_master);
>>> +}
>>> +
>>> static int __init topology_init(void)
>>> {
>>> int i, ret;
>>> @@ -40,7 +46,11 @@ static int __init topology_init(void)
>>> for_each_present_cpu(i) {
>>> struct cpu *c = &per_cpu(cpu_devices, i);
>>> - c->hotpluggable = !!i;
>>> + if (io_master(i))
>>> + c->hotpluggable = 0;
>>> + else
>>> + c->hotpluggable = 1;
>>> +
>>
>> This is just "c->hotpluggable = !io_master(i);".
>>
>
> Yes, I am OK either way, if it is necessary to send v2,
> please let me know.
>
>>> ret = register_cpu(c, i);
>>> if (ret < 0)
>>> pr_warn("topology_init: register_cpu %d failed (%d)\n",
>>> i, ret);
>> Other changes should be okay as they are in line with the previous MIPS
>> change you referenced, but let's see what Huacai thinks.
>>
>
> Thanks,
> Tiezhu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] LoongArch: Do not create sysfs control file for io master CPUs
2022-10-08 10:44 ` Tiezhu Yang
@ 2022-10-08 14:58 ` Huacai Chen
0 siblings, 0 replies; 5+ messages in thread
From: Huacai Chen @ 2022-10-08 14:58 UTC (permalink / raw
To: Tiezhu Yang; +Cc: WANG Xuerui, loongarch, linux-kernel
On Sat, Oct 8, 2022 at 6:44 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
>
>
> On 10/08/2022 05:51 PM, Tiezhu Yang wrote:
> >
> >
> > On 10/08/2022 05:27 PM, WANG Xuerui wrote:
> >> On 2022/10/8 16:59, Tiezhu Yang wrote:
> >>> Now io master CPUs are not hotpluggable on LoongArch, in the current
> >>> code,
> >>> only /sys/devices/system/cpu/cpu0/online is not created, let us set the
> >>> hotpluggable field of all the io master CPUs as 0, then prevent to
> >>> create
> >>> sysfs control file for the other io master CPUs which confuses some user
> >>> space tools. This is similar with commit 9cce844abf07 ("MIPS: CPU#0 is
> >>> not
> >>> hotpluggable").
> >>>
> >>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> >>> ---
> >>> arch/loongarch/kernel/smp.c | 8 --------
> >>> arch/loongarch/kernel/topology.c | 12 +++++++++++-
> >>> 2 files changed, 11 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>> index b5fab30..ef89292 100644
> >>> --- a/arch/loongarch/kernel/smp.c
> >>> +++ b/arch/loongarch/kernel/smp.c
> >>> @@ -240,19 +240,11 @@ void loongson3_smp_finish(void)
> >>> #ifdef CONFIG_HOTPLUG_CPU
> >>> -static bool io_master(int cpu)
> >>> -{
> >>> - return test_bit(cpu, &loongson_sysconf.cores_io_master);
> >>> -}
> >>> -
> >>> int loongson3_cpu_disable(void)
> >>> {
> >>> unsigned long flags;
> >>> unsigned int cpu = smp_processor_id();
> >>> - if (io_master(cpu))
> >>> - return -EBUSY;
> >>> -
> >>
> >> Could this get invoked from somewhere other than the sysfs entries that
> >> "confuse user-space tools", e.g. from somewhere else in kernel land? If
> >> so (or if we can't rule out the possibility) keeping the guard here
> >> might prove more prudent.
> >>
>
> takedown_cpu() kernel/cpu.c
> take_cpu_down() kernel/cpu.c
> __cpu_disable() arch/loongarch/include/asm/smp.h
> loongson3_cpu_disable() arch/loongarch/kernel/smp.c
>
> So I think you are right, keeping the guard here might prove more
> prudent, then it is better move io_master() to a header file that
> can be used in smp.c and topology.c.
Agree, please send V2, thanks.
Huacai
>
> Let us wait for more review comments, thank you.
>
> >
> > If c->hotpluggable is 0, it will not generate a control file in sysfs
> > for this CPU, for example:
> >
> > [root@linux loongson]# cat /sys/devices/system/cpu/cpu0/online
> > cat: /sys/devices/system/cpu/cpu0/online: No such file or directory
> > [root@linux loongson]# echo 0 > /sys/devices/system/cpu/cpu0/online
> > bash: /sys/devices/system/cpu/cpu0/online: Permission denied
> >
> > So no need to check it here, just remove the code.
> >
> > This was done in commit cbab54d9c2b2 ("MIPS: No need to check CPU 0 in
> > {loongson3,bmips,octeon}_cpu_disable()").
> >
> >>> #ifdef CONFIG_NUMA
> >>> numa_remove_cpu(cpu);
> >>> #endif
> >>> diff --git a/arch/loongarch/kernel/topology.c
> >>> b/arch/loongarch/kernel/topology.c
> >>> index ab1a75c..7e7a77f 100644
> >>> --- a/arch/loongarch/kernel/topology.c
> >>> +++ b/arch/loongarch/kernel/topology.c
> >>> @@ -5,6 +5,7 @@
> >>> #include <linux/node.h>
> >>> #include <linux/nodemask.h>
> >>> #include <linux/percpu.h>
> >>> +#include <asm/bootinfo.h>
> >>> static DEFINE_PER_CPU(struct cpu, cpu_devices);
> >>> @@ -33,6 +34,11 @@ void arch_unregister_cpu(int cpu)
> >>> EXPORT_SYMBOL(arch_unregister_cpu);
> >>> #endif
> >>> +static bool io_master(int cpu)
> >>> +{
> >>> + return test_bit(cpu, &loongson_sysconf.cores_io_master);
> >>> +}
> >>> +
> >>> static int __init topology_init(void)
> >>> {
> >>> int i, ret;
> >>> @@ -40,7 +46,11 @@ static int __init topology_init(void)
> >>> for_each_present_cpu(i) {
> >>> struct cpu *c = &per_cpu(cpu_devices, i);
> >>> - c->hotpluggable = !!i;
> >>> + if (io_master(i))
> >>> + c->hotpluggable = 0;
> >>> + else
> >>> + c->hotpluggable = 1;
> >>> +
> >>
> >> This is just "c->hotpluggable = !io_master(i);".
> >>
> >
> > Yes, I am OK either way, if it is necessary to send v2,
> > please let me know.
> >
> >>> ret = register_cpu(c, i);
> >>> if (ret < 0)
> >>> pr_warn("topology_init: register_cpu %d failed (%d)\n",
> >>> i, ret);
> >> Other changes should be okay as they are in line with the previous MIPS
> >> change you referenced, but let's see what Huacai thinks.
> >>
> >
> > Thanks,
> > Tiezhu
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-08 14:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-08 8:59 [PATCH] LoongArch: Do not create sysfs control file for io master CPUs Tiezhu Yang
2022-10-08 9:27 ` WANG Xuerui
2022-10-08 9:51 ` Tiezhu Yang
2022-10-08 10:44 ` Tiezhu Yang
2022-10-08 14:58 ` Huacai Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.