| Message ID | 1456565906-2072-1-git-send-email-allen.pais@oracle.com |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
> This patch provides users with an option to > disable/enable cpu at runtime by writing to > /sys/devices/system/cpu/cpuX/online field. > > Eg: > $ echo [0/1] > /sys/devices/system/cpu/cpu2/online > > Signed-off-by: Allen Pais <allen.pais@oracle.com> > Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com> > --- > arch/sparc/kernel/smp_64.c | 48 +++++++++++++++++++++++-------------------- > arch/sparc/kernel/sysfs.c | 4 +++ > 2 files changed, 30 insertions(+), 22 deletions(-) > > diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c > index 19cd08d..5ea2f91 100644 > --- a/arch/sparc/kernel/smp_64.c > +++ b/arch/sparc/kernel/smp_64.c > @@ -1277,20 +1277,26 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle) > { > int ret = smp_boot_one_cpu(cpu, tidle); > > - if (!ret) { > - cpumask_set_cpu(cpu, &smp_commenced_mask); > - while (!cpu_online(cpu)) > - mb(); > - if (!cpu_online(cpu)) { > - ret = -ENODEV; > - } else { > - /* On SUN4V, writes to %tick and %stick are > - * not allowed. > - */ Any comments on this patch?? Thanks. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Allen Pais <allen.pais@oracle.com> Date: Fri, 11 Mar 2016 19:40:36 +0530 >> This patch provides users with an option to >> disable/enable cpu at runtime by writing to >> /sys/devices/system/cpu/cpuX/online field. >> >> Eg: >> $ echo [0/1] > /sys/devices/system/cpu/cpu2/online >> >> Signed-off-by: Allen Pais <allen.pais@oracle.com> >> Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com> >> --- >> arch/sparc/kernel/smp_64.c | 48 +++++++++++++++++++++++-------------------- >> arch/sparc/kernel/sysfs.c | 4 +++ >> 2 files changed, 30 insertions(+), 22 deletions(-) >> >> diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c >> index 19cd08d..5ea2f91 100644 >> --- a/arch/sparc/kernel/smp_64.c >> +++ b/arch/sparc/kernel/smp_64.c >> @@ -1277,20 +1277,26 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle) >> { >> int ret = smp_boot_one_cpu(cpu, tidle); >> >> - if (!ret) { >> - cpumask_set_cpu(cpu, &smp_commenced_mask); >> - while (!cpu_online(cpu)) >> - mb(); >> - if (!cpu_online(cpu)) { >> - ret = -ENODEV; >> - } else { >> - /* On SUN4V, writes to %tick and %stick are >> - * not allowed. >> - */ > > Any comments on this patch?? If I had the time to review it, I would have by now. Instead, it is in my patchwork backlog waiting for the opportunity to arise. Pinging like this won't help and only makes it take longer for me to get to reviewing your patch. So if your patch is sitting in patchwork, the only thing you can do is simply be patient. Thanks. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Allen Pais <allen.pais@oracle.com> Date: Sat, 27 Feb 2016 15:08:26 +0530 > @@ -1277,20 +1277,26 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle) > { > int ret = smp_boot_one_cpu(cpu, tidle); > > - if (!ret) { > - cpumask_set_cpu(cpu, &smp_commenced_mask); > - while (!cpu_online(cpu)) > - mb(); > - if (!cpu_online(cpu)) { > - ret = -ENODEV; > - } else { > - /* On SUN4V, writes to %tick and %stick are > - * not allowed. > - */ > - if (tlb_type != hypervisor) > - smp_synchronize_one_tick(cpu); > - } > + if (ret) > + goto out; > + > + cpumask_set_cpu(cpu, &smp_commenced_mask); > + while (!cpu_online(cpu)) > + mb(); > + if (!cpu_online(cpu)) { > + ret = -ENODEV; > + goto out; > } > + > + /* On SUN4V, writes to %tick and %stick are > + * not allowed. > + */ > + if (tlb_type != hypervisor) > + smp_synchronize_one_tick(cpu); > + > + smp_fill_in_sib_core_maps(); > + cpu_map_rebuild(); > +out: > return ret; > } > You've made this significantly harder to review and audit by moving the code around so much. Just add the new calls: smp_fill_in_sib_core_maps(); cpu_map_rebuild(); in the existing basic block containing the smp_synchronize_one_tick() call. Then it's obvious to reviewers what your change is actually doing. > > + set_cpu_online(cpu, false); > + > /* Make sure no interrupts point to this cpu. */ > fixup_irqs(); This looks like a bug fix, to make sure fixup_irqs() does the right thing, right? All the little bug fixes like this should be split out into separate patches. Thanks. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> return ret; >> } >> > > You've made this significantly harder to review and audit by moving the code > around so much. > > Just add the new calls: > > smp_fill_in_sib_core_maps(); > cpu_map_rebuild(); > > in the existing basic block containing the smp_synchronize_one_tick() call. > > Then it's obvious to reviewers what your change is actually doing. Sure, I'll fix this and resend. >> >> + set_cpu_online(cpu, false); >> + >> /* Make sure no interrupts point to this cpu. */ >> fixup_irqs(); > > This looks like a bug fix, to make sure fixup_irqs() does the right > thing, right? > > All the little bug fixes like this should be split out into separate > patches. I really did not think it was necessary, but I'll split it up as you said. Thanks. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index 19cd08d..5ea2f91 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1277,20 +1277,26 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle) { int ret = smp_boot_one_cpu(cpu, tidle); - if (!ret) { - cpumask_set_cpu(cpu, &smp_commenced_mask); - while (!cpu_online(cpu)) - mb(); - if (!cpu_online(cpu)) { - ret = -ENODEV; - } else { - /* On SUN4V, writes to %tick and %stick are - * not allowed. - */ - if (tlb_type != hypervisor) - smp_synchronize_one_tick(cpu); - } + if (ret) + goto out; + + cpumask_set_cpu(cpu, &smp_commenced_mask); + while (!cpu_online(cpu)) + mb(); + if (!cpu_online(cpu)) { + ret = -ENODEV; + goto out; } + + /* On SUN4V, writes to %tick and %stick are + * not allowed. + */ + if (tlb_type != hypervisor) + smp_synchronize_one_tick(cpu); + + smp_fill_in_sib_core_maps(); + cpu_map_rebuild(); +out: return ret; } @@ -1333,24 +1339,24 @@ void cpu_play_dead(void) int __cpu_disable(void) { int cpu = smp_processor_id(); - cpuinfo_sparc *c; int i; for_each_cpu(i, &cpu_core_map[cpu]) cpumask_clear_cpu(cpu, &cpu_core_map[i]); cpumask_clear(&cpu_core_map[cpu]); + for_each_cpu(i, &cpu_core_sib_map[cpu]) + cpumask_clear_cpu(cpu, &cpu_core_sib_map[i]); + cpumask_clear(&cpu_core_sib_map[cpu]); + for_each_cpu(i, &per_cpu(cpu_sibling_map, cpu)) cpumask_clear_cpu(cpu, &per_cpu(cpu_sibling_map, i)); cpumask_clear(&per_cpu(cpu_sibling_map, cpu)); - c = &cpu_data(cpu); - - c->core_id = 0; - c->proc_id = -1; - smp_wmb(); + set_cpu_online(cpu, false); + /* Make sure no interrupts point to this cpu. */ fixup_irqs(); @@ -1358,8 +1364,6 @@ int __cpu_disable(void) mdelay(1); local_irq_disable(); - set_cpu_online(cpu, false); - cpu_map_rebuild(); return 0; @@ -1385,7 +1389,7 @@ void __cpu_die(unsigned int cpu) do { hv_err = sun4v_cpu_stop(cpu); if (hv_err == HV_EOK) { - set_cpu_present(cpu, false); + set_cpu_online(cpu, false); break; } } while (--limit > 0); diff --git a/arch/sparc/kernel/sysfs.c b/arch/sparc/kernel/sysfs.c index 7f41d40..43dc6ef 100644 --- a/arch/sparc/kernel/sysfs.c +++ b/arch/sparc/kernel/sysfs.c @@ -240,6 +240,8 @@ static void unregister_cpu_online(unsigned int cpu) struct device *s = &c->dev; int i; + BUG_ON(!c->hotpluggable); + unregister_mmu_stats(s); for (i = 0; i < ARRAY_SIZE(cpu_core_attrs); i++) device_remove_file(s, &cpu_core_attrs[i]); @@ -305,7 +307,9 @@ static int __init topology_init(void) for_each_possible_cpu(cpu) { struct cpu *c = &per_cpu(cpu_devices, cpu); + c->hotpluggable = 1; register_cpu(c, cpu); + if (cpu_online(cpu)) register_cpu_online(cpu); }