diff mbox

[5/5,v2] kernel handling of CPU DLPAR

Message ID 4AD4C3A3.5050103@austin.ibm.com (mailing list archive)
State Superseded
Delegated to: Benjamin Herrenschmidt
Headers show

Commit Message

Nathan Fontenot Oct. 13, 2009, 6:14 p.m. UTC
This adds the capability to DLPAR add and remove CPUs from the kernel. The
creates two new files /sys/devices/system/cpu/probe and
/sys/devices/system/cpu/release to handle the DLPAR addition and removal of
CPUs respectively.

CPU DLPAR add is accomplished by writing the drc-index of the CPU to the
probe file, and removal is done by writing the device-tree path of the cpu
to the release file.

Updated to include #ifdef CONFIG_HOTPLUG_CPU around the cpu hotplug specific
bits so that it will build without CONFIG_HOTPLUG_CPU defined.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
---

Comments

Michael Ellerman Oct. 13, 2009, 10:30 p.m. UTC | #1
On Tue, 2009-10-13 at 13:14 -0500, Nathan Fontenot wrote:
> This adds the capability to DLPAR add and remove CPUs from the kernel. The
> creates two new files /sys/devices/system/cpu/probe and
> /sys/devices/system/cpu/release to handle the DLPAR addition and removal of
> CPUs respectively.

How does this relate to the existing cpu hotplug mechanism? Or is this
making the cpu exist (possible), vs marking it as online?

Is some other platform going to want to do the same? ie. should the
probe/release part be in generic code?

> Index: powerpc/arch/powerpc/platforms/pseries/dlpar.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/platforms/pseries/dlpar.c	2009-10-13 13:08:22.000000000 -0500
> +++ powerpc/arch/powerpc/platforms/pseries/dlpar.c	2009-10-13 13:09:00.000000000 -0500
> @@ -1,11 +1,11 @@
>  /*
> - * dlpar.c - support for dynamic reconfiguration (including PCI
> - * Hotplug and Dynamic Logical Partitioning on RPA platforms).
> + * dlpar.c - support for dynamic reconfiguration (including PCI,

We know it's dlpar.c :)

> + * Memory, and CPU Hotplug and Dynamic Logical Partitioning on
> + * PAPR platforms).
>   *
>   * Copyright (C) 2009 Nathan Fontenot
>   * Copyright (C) 2009 IBM Corporation
>   *
> - *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License version
>   * 2 as published by the Free Software Foundation.
> @@ -19,6 +19,7 @@
>  #include <linux/memory_hotplug.h>
>  #include <linux/sysdev.h>
>  #include <linux/sysfs.h>
> +#include <linux/cpu.h>
>  
> 
>  #include <asm/prom.h>
> @@ -408,6 +409,82 @@
>  	return 0;
>  }
>  
> +#ifdef CONFIG_HOTPLUG_CPU
> +static ssize_t cpu_probe_store(struct class *class, const char *buf,
> +			       size_t count)
> +{
> +	struct device_node *dn;
> +	unsigned long drc_index;
> +	char *cpu_name;
> +	int rc;
> +
> +	rc = strict_strtoul(buf, 0, &drc_index);
> +	if (rc)
> +		return -EINVAL;
> +
> +	rc = acquire_drc(drc_index);
> +	if (rc)
> +		return rc;
> +
> +	dn = configure_connector(drc_index);
> +	if (!dn) {
> +		release_drc(drc_index);
> +		return rc;
> +	}
> +
> +	/* fixup dn name */
> +	cpu_name = kzalloc(strlen(dn->full_name) + strlen("/cpus/") + 1,
> +			   GFP_KERNEL);
> +	if (!cpu_name) {
> +		free_cc_nodes(dn);
> +		release_drc(drc_index);
> +		return -ENOMEM;
> +	}
> +
> +	sprintf(cpu_name, "/cpus/%s", dn->full_name);
> +	kfree(dn->full_name);
> +	dn->full_name = cpu_name;

What was all that? Firmware gives us a bogus full name? But the parent
is right?

> +	rc = add_device_tree_nodes(dn);
> +	if (rc)
> +		release_drc(drc_index);
> +
> +	return rc ? rc : count;

You're sure rc is < 0.

> +}
> +
> +static ssize_t cpu_release_store(struct class *class, const char *buf,
> +				 size_t count)
> +{
> +	struct device_node *dn;
> +	u32 *drc_index;
> +	int rc;
> +
> +	dn = of_find_node_by_path(buf);
> +	if (!dn)
> +		return -EINVAL;
> +
> +	drc_index = (u32 *)of_get_property(dn, "ibm,my-drc-index", NULL);

No cast required.

> +	if (!drc_index) {
> +		of_node_put(dn);
> +		return -EINVAL;
> +	}
> +
> +	rc = release_drc(*drc_index);
> +	if (rc) {
> +		of_node_put(dn);
> +		return rc;
> +	}
> +
> +	rc = remove_device_tree_nodes(dn);
> +	if (rc)
> +		acquire_drc(*drc_index);
> +
> +	of_node_put(dn);
> +	return rc ? rc : count;
> +}


cheers
Nathan Fontenot Oct. 15, 2009, 3:40 p.m. UTC | #2
Michael Ellerman wrote:
> On Tue, 2009-10-13 at 13:14 -0500, Nathan Fontenot wrote:
>> This adds the capability to DLPAR add and remove CPUs from the kernel. The
>> creates two new files /sys/devices/system/cpu/probe and
>> /sys/devices/system/cpu/release to handle the DLPAR addition and removal of
>> CPUs respectively.
> 
> How does this relate to the existing cpu hotplug mechanism? Or is this
> making the cpu exist (possible), vs marking it as online?

This update makes the cpu exist, it does not mark the cpu online.

> 
> Is some other platform going to want to do the same? ie. should the
> probe/release part be in generic code?

I thought about making this generic code, perhaps a follow-on patch to move
the creation of the probe/release files to generic code to see what the
community thinks.  I would assume there would still need to be a arch and/or
platforms specific callout to do the real work.

> 
>> Index: powerpc/arch/powerpc/platforms/pseries/dlpar.c
>> ===================================================================
>> --- powerpc.orig/arch/powerpc/platforms/pseries/dlpar.c	2009-10-13 13:08:22.000000000 -0500
>> +++ powerpc/arch/powerpc/platforms/pseries/dlpar.c	2009-10-13 13:09:00.000000000 -0500
>> @@ -1,11 +1,11 @@
>>  /*
>> - * dlpar.c - support for dynamic reconfiguration (including PCI
>> - * Hotplug and Dynamic Logical Partitioning on RPA platforms).
>> + * dlpar.c - support for dynamic reconfiguration (including PCI,
> 
> We know it's dlpar.c :)

heh! good point.  Consider it gone.

> 
>> + * Memory, and CPU Hotplug and Dynamic Logical Partitioning on
>> + * PAPR platforms).
>>   *
>>   * Copyright (C) 2009 Nathan Fontenot
>>   * Copyright (C) 2009 IBM Corporation
>>   *
>> - *
>>   * This program is free software; you can redistribute it and/or
>>   * modify it under the terms of the GNU General Public License version
>>   * 2 as published by the Free Software Foundation.
>> @@ -19,6 +19,7 @@
>>  #include <linux/memory_hotplug.h>
>>  #include <linux/sysdev.h>
>>  #include <linux/sysfs.h>
>> +#include <linux/cpu.h>
>>  
>>
>>  #include <asm/prom.h>
>> @@ -408,6 +409,82 @@
>>  	return 0;
>>  }
>>  
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +static ssize_t cpu_probe_store(struct class *class, const char *buf,
>> +			       size_t count)
>> +{
>> +	struct device_node *dn;
>> +	unsigned long drc_index;
>> +	char *cpu_name;
>> +	int rc;
>> +
>> +	rc = strict_strtoul(buf, 0, &drc_index);
>> +	if (rc)
>> +		return -EINVAL;
>> +
>> +	rc = acquire_drc(drc_index);
>> +	if (rc)
>> +		return rc;
>> +
>> +	dn = configure_connector(drc_index);
>> +	if (!dn) {
>> +		release_drc(drc_index);
>> +		return rc;
>> +	}
>> +
>> +	/* fixup dn name */
>> +	cpu_name = kzalloc(strlen(dn->full_name) + strlen("/cpus/") + 1,
>> +			   GFP_KERNEL);
>> +	if (!cpu_name) {
>> +		free_cc_nodes(dn);
>> +		release_drc(drc_index);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	sprintf(cpu_name, "/cpus/%s", dn->full_name);
>> +	kfree(dn->full_name);
>> +	dn->full_name = cpu_name;
> 
> What was all that? Firmware gives us a bogus full name? But the parent
> is right?

Yeah, this has always been an oddity to me.  The name that is given to us
from firmware puts the cpu node in the root of the device tree as opposed
to /cpus.  Since cpu nodes are in /cpus, the name is fixed up to reflect this.

This name fixup has always been done by the drmgr userspace command, which is
where this functionality is being imported from.

> 
>> +	rc = add_device_tree_nodes(dn);
>> +	if (rc)
>> +		release_drc(drc_index);
>> +
>> +	return rc ? rc : count;
> 
> You're sure rc is < 0.
> 
Oh, good catch.

>> +}
>> +
>> +static ssize_t cpu_release_store(struct class *class, const char *buf,
>> +				 size_t count)
>> +{
>> +	struct device_node *dn;
>> +	u32 *drc_index;
>> +	int rc;
>> +
>> +	dn = of_find_node_by_path(buf);
>> +	if (!dn)
>> +		return -EINVAL;
>> +
>> +	drc_index = (u32 *)of_get_property(dn, "ibm,my-drc-index", NULL);
> 
> No cast required.

ok.

thanks for the review.

-Nathan

> 
>> +	if (!drc_index) {
>> +		of_node_put(dn);
>> +		return -EINVAL;
>> +	}
>> +
>> +	rc = release_drc(*drc_index);
>> +	if (rc) {
>> +		of_node_put(dn);
>> +		return rc;
>> +	}
>> +
>> +	rc = remove_device_tree_nodes(dn);
>> +	if (rc)
>> +		acquire_drc(*drc_index);
>> +
>> +	of_node_put(dn);
>> +	return rc ? rc : count;
>> +}
> 
> 
> cheers
Michael Ellerman Oct. 16, 2009, 12:52 a.m. UTC | #3
On Thu, 2009-10-15 at 10:40 -0500, Nathan Fontenot wrote:
> Michael Ellerman wrote:
> > On Tue, 2009-10-13 at 13:14 -0500, Nathan Fontenot wrote:
> >> This adds the capability to DLPAR add and remove CPUs from the kernel. The
> >> creates two new files /sys/devices/system/cpu/probe and
> >> /sys/devices/system/cpu/release to handle the DLPAR addition and removal of
> >> CPUs respectively.
> > 
> > How does this relate to the existing cpu hotplug mechanism? Or is this
> > making the cpu exist (possible), vs marking it as online?
> 
> This update makes the cpu exist, it does not mark the cpu online.
> 
> > 
> > Is some other platform going to want to do the same? ie. should the
> > probe/release part be in generic code?
> 
> I thought about making this generic code, perhaps a follow-on patch to move
> the creation of the probe/release files to generic code to see what the
> community thinks.  I would assume there would still need to be a arch and/or
> platforms specific callout to do the real work.

I'm not so worried about the code being generic, there's not much that
would be generic. More the mechanism. We don't want to add probe/release
to powerpc, and then have a different mechanism get added later by some
other platform, or generically.

But I guess you CC'ed lkml and there's not much more to do, I don't know
specifically who'd care about it.

cheers
diff mbox

Patch

Index: powerpc/arch/powerpc/platforms/pseries/dlpar.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/dlpar.c	2009-10-13 13:08:22.000000000 -0500
+++ powerpc/arch/powerpc/platforms/pseries/dlpar.c	2009-10-13 13:09:00.000000000 -0500
@@ -1,11 +1,11 @@ 
 /*
- * dlpar.c - support for dynamic reconfiguration (including PCI
- * Hotplug and Dynamic Logical Partitioning on RPA platforms).
+ * dlpar.c - support for dynamic reconfiguration (including PCI,
+ * Memory, and CPU Hotplug and Dynamic Logical Partitioning on
+ * PAPR platforms).
  *
  * Copyright (C) 2009 Nathan Fontenot
  * Copyright (C) 2009 IBM Corporation
  *
- *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License version
  * 2 as published by the Free Software Foundation.
@@ -19,6 +19,7 @@ 
 #include <linux/memory_hotplug.h>
 #include <linux/sysdev.h>
 #include <linux/sysfs.h>
+#include <linux/cpu.h>
 
 
 #include <asm/prom.h>
@@ -408,6 +409,82 @@ 
 	return 0;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+static ssize_t cpu_probe_store(struct class *class, const char *buf,
+			       size_t count)
+{
+	struct device_node *dn;
+	unsigned long drc_index;
+	char *cpu_name;
+	int rc;
+
+	rc = strict_strtoul(buf, 0, &drc_index);
+	if (rc)
+		return -EINVAL;
+
+	rc = acquire_drc(drc_index);
+	if (rc)
+		return rc;
+
+	dn = configure_connector(drc_index);
+	if (!dn) {
+		release_drc(drc_index);
+		return rc;
+	}
+
+	/* fixup dn name */
+	cpu_name = kzalloc(strlen(dn->full_name) + strlen("/cpus/") + 1,
+			   GFP_KERNEL);
+	if (!cpu_name) {
+		free_cc_nodes(dn);
+		release_drc(drc_index);
+		return -ENOMEM;
+	}
+
+	sprintf(cpu_name, "/cpus/%s", dn->full_name);
+	kfree(dn->full_name);
+	dn->full_name = cpu_name;
+
+	rc = add_device_tree_nodes(dn);
+	if (rc)
+		release_drc(drc_index);
+
+	return rc ? rc : count;
+}
+
+static ssize_t cpu_release_store(struct class *class, const char *buf,
+				 size_t count)
+{
+	struct device_node *dn;
+	u32 *drc_index;
+	int rc;
+
+	dn = of_find_node_by_path(buf);
+	if (!dn)
+		return -EINVAL;
+
+	drc_index = (u32 *)of_get_property(dn, "ibm,my-drc-index", NULL);
+	if (!drc_index) {
+		of_node_put(dn);
+		return -EINVAL;
+	}
+
+	rc = release_drc(*drc_index);
+	if (rc) {
+		of_node_put(dn);
+		return rc;
+	}
+
+	rc = remove_device_tree_nodes(dn);
+	if (rc)
+		acquire_drc(*drc_index);
+
+	of_node_put(dn);
+	return rc ? rc : count;
+}
+
+#endif /* CONFIG_HOTPLUG_CPU */
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 
 static struct property *clone_property(struct property *old_prop)
@@ -553,6 +630,13 @@ 
 
 static struct class_attribute class_attr_mem_release =
 			__ATTR(release, S_IWUSR, NULL, memory_release_store);
+#endif /* CONFIG_MEMORY_HOTPLUG */
+
+#ifdef CONFIG_HOTPLUG_CPU
+static struct class_attribute class_attr_cpu_probe =
+			__ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
+static struct class_attribute class_attr_cpu_release =
+			__ATTR(release, S_IWUSR, NULL, cpu_release_store);
 #endif
 
 static int pseries_dlpar_init(void)
@@ -567,6 +651,18 @@ 
 		       "release file\n");
 #endif
 
+#ifdef CONFIG_HOTPLUG_CPU
+	if (sysfs_create_file(&cpu_sysdev_class.kset.kobj,
+			      &class_attr_cpu_probe.attr))
+		printk(KERN_INFO "DLPAR: Could not create sysfs cpu "
+		       "probe file\n");
+
+	if (sysfs_create_file(&cpu_sysdev_class.kset.kobj,
+			      &class_attr_cpu_release.attr))
+		printk(KERN_INFO "DLPAR: Could not create sysfs cpu "
+		       "release file\n");
+#endif
+
 	return 0;
 }
 device_initcall(pseries_dlpar_init);