On 11/25/2015 11:02 PM, Michael Ellerman wrote:
> On Tue, 2015-10-27 at 13:26 -0500, Nathan Fontenot wrote:
>
>> Add the ability to dlpar remove CPUs via hotplug rtas events, either by
>> specifying the drc-index of the CPU to remove or providing a count of cpus
>> to remove.
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> index f080e81..635f0ba 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> @@ -570,6 +571,143 @@ static ssize_t dlpar_cpu_remove(struct device_node
>> *dn, u32 drc_index)
>> return 0;
>> }
>>
>> +static struct device_node *cpu_drc_index_to_dn(u32 drc_index)
>> +{
>> + struct device_node *dn;
>> + u32 my_index;
>> + int rc;
>> +
>> + for_each_node_by_type(dn, "cpu") {
>> + rc = of_property_read_u32(dn, "ibm,my-drc-index", &my_index);
>> + if (rc)
>> + continue;
>> +
>> + if (my_index == drc_index)
>> + break;
>> + }
>> +
>> + return dn;
>> +}
>> +
>> +static int dlpar_cpu_remove_by_index(u32 drc_index)
>> +{
>> + struct device_node *dn;
>> + int rc;
>> +
>> + dn = cpu_drc_index_to_dn(drc_index);
>> + if (!dn)
>> + return -ENODEV;
>> +
>> + rc = dlpar_cpu_remove(dn, drc_index);
>> + of_node_put(dn);
>
> Does dlpar_cpu_remove() work when you still hold a reference to the dn?
Yes, this works while holding the dn reference here.
>
> It looks like dlpar_detach_node() does an of_node_put() also.
Correct. The of_node_put() in dlpar_detach_node() is there to do a put
from the initial node creation, without this the reference count would never
go to zero and the node would not be released.
>
>> + return rc;
>> +}
>> +
>> +static u32 *dlpar_cpus_to_remove(int cpus_to_remove)
>> +{
>> + struct device_node *dn;
>> + u32 *cpu_drcs;
>> + int cpus_found = 0;
>> + int i, rc;
>> +
>> + cpu_drcs = kcalloc(cpus_to_remove, sizeof(*cpu_drcs), GFP_KERNEL);
>> + if (!cpu_drcs)
>> + return NULL;
>> +
>> + i = 0;
>> + for_each_node_by_type(dn, "cpu") {
>> + cpus_found++;
>> +
>> + if (cpus_found > cpus_to_remove) {
>> + of_node_put(dn);
>> + break;
>> + }
>> +
>> + rc = of_property_read_u32(dn, "ibm,my-drc-index",
>> + &cpu_drcs[i++]);
>> + if (rc) {
>> + of_node_put(dn);
>> + break;
>
> I'm not sure about the logic here with cpus_found and i. If you break here
> cpus_found will be 1, but you found zero, which seems odd.
>
Agreed, it is a bit odd. The cpus_found var is meant to count the number of
cpus we find when looping through for_each_node_by_type() whereas i is just
meant to be an index into the cpu_cars array so we can save the drc-index
of the cpus we find. The variable i is just to index the array, nothing more.
Perhaps instead of having i to use as an index into the cpu_drcs array I
could just use [cpus_found - 1] to index cpu_drcs and get rid of i. Not sure
if that makes the code any easier to read.
> If instead you delayed the increment of i:
>
> rc = of_property_read_u32(dn, "ibm,my-drc-index",
> &cpu_drcs[i]);
> if (rc) {
> of_node_put(dn);
> break;
> }
>
> i++;
> }
>
> Then i would equal the number of cpus found at all times. If you need to count
> one more in the if below you can just do that there.
>
>> +
>> + /* We want to find cpus_to_remove + 1 CPUs to ensure we do not
>> + * remove the last CPU.
>> + */
>> + if (cpus_found <= cpus_to_remove) {
>> + pr_warn("Failed to find enough CPUs (%d of %d) to remove\n",
>> + cpus_found, cpus_to_remove);
>> + kfree(cpu_drcs);
>> + cpu_drcs = NULL;
>> + }
>
> On my two cpu system when I do "cpu remove count 1" this always says:
>
> pseries-hotplug-cpu: Failed to find enough CPUs (1 of 1) to remove
>
> Which confuses me.
>
> I suspect that's because I actually have one cpu *node*, which appears to
> Linux
> as two cpus (due to SMT). So I think it's working as expected, but it's not
> very clear from a user's perspective.
>
You are correct. For Power CPU DLPAR works on a node basis. If there
is only one node it will not remove the last CPU.
I should update the cpus_found to check to print a message if the remove
request would entail removing the last CPU and that the request is being
failed because of that.
Thanks for the feedback,
-Nathan
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
