[RFC PATCH v2 09/12] ACPI: Update memory driver for hotplug framework

[Toshi Kani]

Changed acpi_memory_device_notify() to request a hotplug operation
by calling shp_submit_req().  It no longer initiates hot-add or
hot-delete operation by calling add_memory() or remove_memory()
directly.  Removed the enabled and failed flags from acpi_memory_info
since they are no longer used.

Changed acpi_memory_device_add() to not call add_memory() to online
a memory device.  Similarly, changed acpi_memory_device_remove()
to not call remove_memory() to offline a memory device.

Added acpi_memory_resource() to set memory information to a hotplug
request.

Signed-off-by: Toshi Kani 
---
 drivers/acpi/acpi_memhotplug.c |  271 +---
 1 file changed, 89 insertions(+), 182 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index b679bf8..67868f5 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define ACPI_MEMORY_DEVICE_CLASS   "memory"
 #define ACPI_MEMORY_DEVICE_HID "PNP0C80"
@@ -55,6 +56,8 @@ MODULE_LICENSE("GPL");
 
 static int acpi_memory_device_add(struct acpi_device *device);
 static int acpi_memory_device_remove(struct acpi_device *device, int type);
+static int acpi_memory_device_resource(struct acpi_device *device,
+   struct shp_request *shp_req);
 
 static const struct acpi_device_id memory_device_ids[] = {
{ACPI_MEMORY_DEVICE_HID, 0},
@@ -69,6 +72,7 @@ static struct acpi_driver acpi_memory_device_driver = {
.ops = {
.add = acpi_memory_device_add,
.remove = acpi_memory_device_remove,
+   .resource = acpi_memory_device_resource,
},
 };
 
@@ -153,59 +157,12 @@ acpi_memory_get_device_resources(struct 
acpi_memory_device *mem_device)
return 0;
 }
 
-static int
-acpi_memory_get_device(acpi_handle handle,
-  struct acpi_memory_device **mem_device)
-{
-   acpi_status status;
-   acpi_handle phandle;
-   struct acpi_device *device = NULL;
-   struct acpi_device *pdevice = NULL;
-   int result;
-
-
-   if (!acpi_bus_get_device(handle, &device) && device)
-   goto end;
-
-   status = acpi_get_parent(handle, &phandle);
-   if (ACPI_FAILURE(status)) {
-   ACPI_EXCEPTION((AE_INFO, status, "Cannot find acpi parent"));
-   return -EINVAL;
-   }
-
-   /* Get the parent device */
-   result = acpi_bus_get_device(phandle, &pdevice);
-   if (result) {
-   acpi_handle_warn(phandle, "Cannot get acpi bus device\n");
-   return -EINVAL;
-   }
-
-   /*
-* Now add the notified device.  This creates the acpi_device
-* and invokes .add function
-*/
-   result = acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE);
-   if (result) {
-   acpi_handle_warn(handle, "Cannot add acpi bus\n");
-   return -EINVAL;
-   }
-
-  end:
-   *mem_device = acpi_driver_data(device);
-   if (!(*mem_device)) {
-   dev_err(&device->dev, "driver data not found\n");
-   return -ENODEV;
-   }
-
-   return 0;
-}
-
-static int acpi_memory_check_device(struct acpi_memory_device *mem_device)
+static int acpi_memory_check_device(acpi_handle handle)
 {
unsigned long long current_status;
 
/* Get device present/absent information from the _STA */
-   if (ACPI_FAILURE(acpi_evaluate_integer(mem_device->device->handle, 
"_STA",
+   if (ACPI_FAILURE(acpi_evaluate_integer(handle, "_STA",
   NULL, ¤t_status)))
return -ENODEV;
/*
@@ -220,148 +177,46 @@ static int acpi_memory_check_device(struct 
acpi_memory_device *mem_device)
return 0;
 }
 
-static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
-{
-   int result, num_enabled = 0;
-   struct acpi_memory_info *info;
-   int node;
-
-   node = acpi_get_node(mem_device->device->handle);
-   /*
-* Tell the VM there is more memory here...
-* Note: Assume that this function returns zero on success
-* We don't have memory-hot-add rollback function,now.
-* (i.e. memory-hot-remove function)
-*/
-   list_for_each_entry(info, &mem_device->res_list, list) {
-   if (info->enabled) { /* just sanity check...*/
-   num_enabled++;
-   continue;
-   }
-   /*
-* If the memory block size is zero, please ignore it.
-* Don't try to do the following memory hotplug flowchart.
-*/
-   if (!info->length)
-   con

[RFC PATCH v2 12/12] ACPI: Update sysfs eject for hotplug framework

[Toshi Kani]

Changed acpi_eject_store() to request a hot-delete operation by
calling shp_submit_req().  It no longer initiates a hot-delete
operation by calling acpi_bus_hot_remove_device().

Deleted acpi_bus_hot_remove_device() since it no longer has any
caller and should not be called for hot-delete.

Deleted eject_pending bit from acpi_device_flags since the ACPI
container driver no longer sets it for hot-delete, and sysfs
eject no longer checks it in acpi_bus_hot_remove_device().

Signed-off-by: Toshi Kani 
---
 drivers/acpi/scan.c |  122 ---
 include/acpi/acpi_bus.h |4 --
 2 files changed, 23 insertions(+), 103 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index c88be6c..5e47b49 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -13,6 +13,7 @@
 #include 
 
 #include 
+#include 
 
 #include "internal.h"
 
@@ -105,85 +106,6 @@ acpi_device_modalias_show(struct device *dev, struct 
device_attribute *attr, cha
 }
 static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
 
-/**
- * acpi_bus_hot_remove_device: hot-remove a device and its children
- * @context: struct acpi_eject_event pointer (freed in this func)
- *
- * Hot-remove a device and its children. This function frees up the
- * memory space passed by arg context, so that the caller may call
- * this function asynchronously through acpi_os_hotplug_execute().
- */
-void acpi_bus_hot_remove_device(void *context)
-{
-   struct acpi_eject_event *ej_event = (struct acpi_eject_event *) context;
-   struct acpi_device *device;
-   acpi_handle handle = ej_event->handle;
-   acpi_handle temp;
-   struct acpi_object_list arg_list;
-   union acpi_object arg;
-   acpi_status status = AE_OK;
-   u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
-
-   if (acpi_bus_get_device(handle, &device))
-   goto err_out;
-
-   if (!device)
-   goto err_out;
-
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-   "Hot-removing device %s...\n", dev_name(&device->dev)));
-
-   if (acpi_bus_trim(device, 1)) {
-   printk(KERN_ERR PREFIX
-   "Removing device failed\n");
-   goto err_out;
-   }
-
-   /* device has been freed */
-   device = NULL;
-
-   /* power off device */
-   status = acpi_evaluate_object(handle, "_PS3", NULL, NULL);
-   if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
-   printk(KERN_WARNING PREFIX
-   "Power-off device failed\n");
-
-   if (ACPI_SUCCESS(acpi_get_handle(handle, "_LCK", &temp))) {
-   arg_list.count = 1;
-   arg_list.pointer = &arg;
-   arg.type = ACPI_TYPE_INTEGER;
-   arg.integer.value = 0;
-   acpi_evaluate_object(handle, "_LCK", &arg_list, NULL);
-   }
-
-   arg_list.count = 1;
-   arg_list.pointer = &arg;
-   arg.type = ACPI_TYPE_INTEGER;
-   arg.integer.value = 1;
-
-   /*
-* TBD: _EJD support.
-*/
-   status = acpi_evaluate_object(handle, "_EJ0", &arg_list, NULL);
-   if (ACPI_FAILURE(status)) {
-   if (status != AE_NOT_FOUND)
-   printk(KERN_WARNING PREFIX
-   "Eject device failed\n");
-   goto err_out;
-   }
-
-   kfree(context);
-   return;
-
-err_out:
-   /* Inform firmware the hot-remove operation has completed w/ error */
-   (void) acpi_evaluate_hotplug_ost(handle,
-   ej_event->event, ost_code, NULL);
-   kfree(context);
-   return;
-}
-EXPORT_SYMBOL(acpi_bus_hot_remove_device);
-
 static ssize_t
 acpi_eject_store(struct device *d, struct device_attribute *attr,
const char *buf, size_t count)
@@ -192,44 +114,44 @@ acpi_eject_store(struct device *d, struct 
device_attribute *attr,
acpi_status status;
acpi_object_type type = 0;
struct acpi_device *acpi_device = to_acpi_device(d);
-   struct acpi_eject_event *ej_event;
+   struct shp_request *shp_req;
 
if ((!count) || (buf[0] != '1')) {
return -EINVAL;
}
 #ifndef FORCE_EJECT
if (acpi_device->driver == NULL) {
-   ret = -ENODEV;
-   goto err;
+   return -ENODEV;
}
 #endif
status = acpi_get_type(acpi_device->handle, &type);
if (ACPI_FAILURE(status) || (!acpi_device->flags.ejectable)) {
-   ret = -ENODEV;
-   goto err;
+   return -ENODEV;
}
 
-   ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
-   if (!ej_event) {
-   ret = -ENOMEM;
+   shp_req = shp_alloc_request(SHP_HOTPLUG

[RFC PATCH v2 10/12] ACPI: Update container driver for hotplug framework

[Toshi Kani]

Changed container_notify_cb() to request a hotplug operation by
calling shp_submit_req().  It no longer initiates hot-add by calling
acpi_bus_add().  Also, it no longer sets device->flags.eject_pending
and generates KOBJ_OFFLINE event for hot-delete.

Signed-off-by: Toshi Kani 
---
 drivers/acpi/container.c |   95 ++
 1 file changed, 37 insertions(+), 58 deletions(-)

diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 811910b..89af4fc 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define PREFIX "ACPI: "
 
@@ -135,77 +136,37 @@ static int acpi_container_remove(struct acpi_device 
*device, int type)
return status;
 }
 
-static int container_device_add(struct acpi_device **device, acpi_handle 
handle)
-{
-   acpi_handle phandle;
-   struct acpi_device *pdev;
-   int result;
-
-
-   if (acpi_get_parent(handle, &phandle)) {
-   return -ENODEV;
-   }
-
-   if (acpi_bus_get_device(phandle, &pdev)) {
-   return -ENODEV;
-   }
-
-   if (acpi_bus_add(device, pdev, handle, ACPI_BUS_TYPE_DEVICE)) {
-   return -ENODEV;
-   }
-
-   result = acpi_bus_start(*device);
-
-   return result;
-}
-
-static void container_notify_cb(acpi_handle handle, u32 type, void *context)
+static void container_notify_cb(acpi_handle handle, u32 event, void *context)
 {
struct acpi_device *device = NULL;
-   int result;
-   int present;
-   acpi_status status;
+   struct shp_request *shp_req;
+   enum shp_operation shp_op;
u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
 
-   switch (type) {
+   switch (event) {
case ACPI_NOTIFY_BUS_CHECK:
/* Fall through */
case ACPI_NOTIFY_DEVICE_CHECK:
-   pr_debug("Container driver received %s event\n",
-  (type == ACPI_NOTIFY_BUS_CHECK) ?
-  "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
-
-   present = is_device_present(handle);
-   status = acpi_bus_get_device(handle, &device);
-   if (!present) {
-   if (ACPI_SUCCESS(status)) {
-   /* device exist and this is a remove request */
-   device->flags.eject_pending = 1;
-   kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
-   return;
-   }
-   break;
+   if (!is_device_present(handle)) {
+   acpi_handle_err(handle, "Device not enabled\n");
+   goto err;
}
 
-   if (!ACPI_FAILURE(status) || device)
-   break;
-
-   result = container_device_add(&device, handle);
-   if (result) {
-   acpi_handle_warn(handle, "Failed to add container\n");
-   break;
+   if (!acpi_bus_get_device(handle, &device)) {
+   acpi_handle_err(handle, "Device added already\n");
+   goto err;
}
 
-   kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
-   ost_code = ACPI_OST_SC_SUCCESS;
+   shp_op = SHP_HOTPLUG_ADD;
break;
 
case ACPI_NOTIFY_EJECT_REQUEST:
-   if (!acpi_bus_get_device(handle, &device) && device) {
-   device->flags.eject_pending = 1;
-   kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
-   return;
+   if (acpi_bus_get_device(handle, &device)) {
+   acpi_handle_err(handle, "Device not added yet\n");
+   goto err;
}
+
+   shp_op = SHP_HOTPLUG_DEL;
break;
 
default:
@@ -213,8 +174,26 @@ static void container_notify_cb(acpi_handle handle, u32 
type, void *context)
return;
}
 
-   /* Inform firmware that the hotplug operation has completed */
-   (void) acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
+   shp_req = shp_alloc_request(shp_op);
+   if (!shp_req) {
+   acpi_handle_err(handle, "No memory to request hotplug\n");
+   goto err;
+   }
+
+   shp_req->handle = (void *)handle;
+   shp_req->event = event;
+
+   if (shp_submit_req(shp_req)) {
+   acpi_handle_err(handle, "Failed to request hotplug\n");
+   kfree(shp_req);
+   goto err;
+   }
+
+   return;
+
+err:
+   /* Inform firmware that the hotplug operation completed w/

[RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

Added include/acpi/sys_hotplug.h, which is ACPI-specific system
device hotplug header and defines the order values of ACPI-specific
handlers.

Signed-off-by: Toshi Kani 
---
 include/acpi/sys_hotplug.h |   48 
 1 file changed, 48 insertions(+)
 create mode 100644 include/acpi/sys_hotplug.h

diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
new file mode 100644
index 000..ad80f61
--- /dev/null
+++ b/include/acpi/sys_hotplug.h
@@ -0,0 +1,48 @@
+/*
+ * sys_hotplug.h - ACPI System device hot-plug framework
+ *
+ * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
+ * Toshi Kani 
+ *
+ * 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.
+ */
+
+#ifndef _ACPI_SYS_HOTPLUG_H
+#define _ACPI_SYS_HOTPLUG_H
+
+#include 
+#include 
+#include 
+
+/*
+ * System device hot-plug operation proceeds in the following order.
+ *   Validate phase -> Execute phase -> Commit phase
+ *
+ * The order values below define the calling sequence of ACPI-specific
+ * handlers for each phase in ascending order.  The order value of
+ * platform-neutral handlers are defined in .
+ */
+
+/* Add Validate order values */
+#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must be 
first */
+
+/* Add Execute order values */
+#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
+#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
+
+/* Add Commit order values */
+#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
+
+/* Delete Validate order values */
+#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must be 
first */
+#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
+
+/* Delete Execute order values */
+#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
+
+/* Delete Commit order values */
+#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
+
+#endif /* _ACPI_SYS_HOTPLUG_H */
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[RFC PATCH v2 08/12] ACPI: Update processor driver for hotplug framework

[Toshi Kani]

Added acpi_processor_resource() for the .resource() interface,
which sets CPU information to a hotplug request.

Changed acpi_processor_hotplug_notify() to request a hotplug
operation by calling shp_submit_req().  It no longer initiates
hot-add or hot-delete operation by calling acpi_bus_add() or
acpi_bus_hot_remove_device() directly.

acpi_processor_handle_eject() is changed not to call cpu_down()
since .add() / .remove() may not online / offline a device.

Signed-off-by: Toshi Kani 
---
 drivers/acpi/processor_driver.c |  150 ++-
 1 file changed, 70 insertions(+), 80 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index e83311b..f630c2c 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -57,6 +57,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define PREFIX "ACPI: "
 
@@ -83,6 +84,8 @@ MODULE_LICENSE("GPL");
 static int acpi_processor_add(struct acpi_device *device);
 static int acpi_processor_remove(struct acpi_device *device, int type);
 static void acpi_processor_notify(struct acpi_device *device, u32 event);
+static int acpi_processor_resource(struct acpi_device *device,
+   struct shp_request *shp_req);
 static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr);
 static int acpi_processor_handle_eject(struct acpi_processor *pr);
 static int acpi_processor_start(struct acpi_processor *pr);
@@ -105,6 +108,7 @@ static struct acpi_driver acpi_processor_driver = {
.add = acpi_processor_add,
.remove = acpi_processor_remove,
.notify = acpi_processor_notify,
+   .resource = acpi_processor_resource,
},
.drv.pm = &acpi_processor_pm,
 };
@@ -649,6 +653,33 @@ free:
return 0;
 }
 
+static int
+acpi_processor_resource(struct acpi_device *device, struct shp_request 
*shp_req)
+{
+   struct acpi_processor *pr;
+   struct shp_device *shp_dev;
+
+   pr = acpi_driver_data(device);
+   if (!pr) {
+   dev_err(&device->dev, "Driver data missing\n");
+   return -EINVAL;
+   }
+
+   shp_dev = kzalloc(sizeof(*shp_dev), GFP_KERNEL);
+   if (!shp_dev) {
+   dev_err(&device->dev, "Failed to allocate shp_dev\n");
+   return -EINVAL;
+   }
+
+   shp_dev->device = &device->dev;
+   shp_dev->class = SHP_CLS_CPU;
+   shp_dev->info.cpu.cpu_id = pr->id;
+
+   shp_add_dev_info(shp_req, shp_dev);
+
+   return 0;
+}
+
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /
  * Acpi processor hotplug support  *
@@ -677,97 +708,68 @@ static int is_processor_present(acpi_handle handle)
return 0;
 }
 
-static
-int acpi_processor_device_add(acpi_handle handle, struct acpi_device **device)
-{
-   acpi_handle phandle;
-   struct acpi_device *pdev;
-
-
-   if (acpi_get_parent(handle, &phandle)) {
-   return -ENODEV;
-   }
-
-   if (acpi_bus_get_device(phandle, &pdev)) {
-   return -ENODEV;
-   }
-
-   if (acpi_bus_add(device, pdev, handle, ACPI_BUS_TYPE_PROCESSOR)) {
-   return -ENODEV;
-   }
-
-   return 0;
-}
-
 static void acpi_processor_hotplug_notify(acpi_handle handle,
  u32 event, void *data)
 {
struct acpi_device *device = NULL;
-   struct acpi_eject_event *ej_event = NULL;
+   struct shp_request *shp_req;
+   enum shp_operation shp_op;
u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */
-   int result;
 
switch (event) {
case ACPI_NOTIFY_BUS_CHECK:
case ACPI_NOTIFY_DEVICE_CHECK:
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-   "Processor driver received %s event\n",
-  (event == ACPI_NOTIFY_BUS_CHECK) ?
-  "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK"));
-
-   if (!is_processor_present(handle))
-   break;
-
-   if (!acpi_bus_get_device(handle, &device))
-   break;
+   if (!is_processor_present(handle)) {
+   acpi_handle_err(handle, "Device not enabled\n");
+   goto err;
+   }
 
-   result = acpi_processor_device_add(handle, &device);
-   if (result) {
-   acpi_handle_err(handle, "Unable to add the device\n");
-   break;
+   if (!acpi_bus_get_device(handle, &device)) {
+   acpi_handle_err(handle, "Device added already\n");
+   goto err;

[RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

Added include/linux/sys_hotplug.h, which defines the system device
hotplug framework interfaces used by the framework itself and
handlers.

The order values define the calling sequence of handlers.  For add
execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
CPU so that threads on new CPUs can start using their local memory.
The ordering of the delete execute is symmetric to the add execute.

struct shp_request defines a hot-plug request information.  The
device resource information is managed with a list so that a single
request may target to multiple devices.

Signed-off-by: Toshi Kani 
---
 include/linux/sys_hotplug.h |  181 +++
 1 file changed, 181 insertions(+)
 create mode 100644 include/linux/sys_hotplug.h

diff --git a/include/linux/sys_hotplug.h b/include/linux/sys_hotplug.h
new file mode 100644
index 000..86674dd
--- /dev/null
+++ b/include/linux/sys_hotplug.h
@@ -0,0 +1,181 @@
+/*
+ * sys_hotplug.h - System device hot-plug framework
+ *
+ * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
+ * Toshi Kani 
+ *
+ * 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.
+ */
+
+#ifndef _LINUX_SYS_HOTPLUG_H
+#define _LINUX_SYS_HOTPLUG_H
+
+#include 
+#include 
+
+/*
+ * System device hot-plug operation proceeds in the following order.
+ *   Validate phase -> Execute phase -> Commit phase
+ *
+ * The order values below define the calling sequence of platform
+ * neutral handlers for each phase in ascending order.  The order
+ * values of firmware-specific handlers are defined in sys_hotplug.h
+ * under firmware specific directories.
+ */
+
+/* All order values must be smaller than this value */
+#define SHP_ORDER_MAX  0xff
+
+/* Add Validate order values */
+
+/* Add Execute order values */
+#define SHP_MEM_ADD_EXECUTE_ORDER  100
+#define SHP_CPU_ADD_EXECUTE_ORDER  110
+
+/* Add Commit order values */
+
+/* Delete Validate order values */
+#define SHP_CPU_DEL_VALIDATE_ORDER 100
+#define SHP_MEM_DEL_VALIDATE_ORDER 110
+
+/* Delete Execute order values */
+#define SHP_CPU_DEL_EXECUTE_ORDER  10
+#define SHP_MEM_DEL_EXECUTE_ORDER  20
+
+/* Delete Commit order values */
+
+/*
+ * Hot-plug request types
+ */
+#define SHP_REQ_ADD0x00
+#define SHP_REQ_DELETE 0x01
+#define SHP_REQ_MASK   0xff
+
+/*
+ * Hot-plug phase types
+ */
+#define SHP_PH_VALIDATE0x00
+#define SHP_PH_EXECUTE 0x000100
+#define SHP_PH_COMMIT  0x000200
+#define SHP_PH_MASK0x00ff00
+
+/*
+ * Hot-plug operation types
+ */
+#define SHP_OP_HOTPLUG 0x00
+#define SHP_OP_ONLINE  0x01
+#define SHP_OP_MASK0xff
+
+/*
+ * Hot-plug phases
+ */
+enum shp_phase {
+   SHP_ADD_VALIDATE= (SHP_REQ_ADD|SHP_PH_VALIDATE),
+   SHP_ADD_EXECUTE = (SHP_REQ_ADD|SHP_PH_EXECUTE),
+   SHP_ADD_COMMIT  = (SHP_REQ_ADD|SHP_PH_COMMIT),
+   SHP_DEL_VALIDATE= (SHP_REQ_DELETE|SHP_PH_VALIDATE),
+   SHP_DEL_EXECUTE = (SHP_REQ_DELETE|SHP_PH_EXECUTE),
+   SHP_DEL_COMMIT  = (SHP_REQ_DELETE|SHP_PH_COMMIT)
+};
+
+/*
+ * Hot-plug operations
+ */
+enum shp_operation {
+   SHP_HOTPLUG_ADD = (SHP_OP_HOTPLUG|SHP_REQ_ADD),
+   SHP_HOTPLUG_DEL = (SHP_OP_HOTPLUG|SHP_REQ_DELETE),
+   SHP_ONLINE_ADD  = (SHP_OP_ONLINE|SHP_REQ_ADD),
+   SHP_ONLINE_DEL  = (SHP_OP_ONLINE|SHP_REQ_DELETE)
+};
+
+/*
+ * Hot-plug device classes
+ */
+enum shp_class {
+   SHP_CLS_INVALID = 0,
+   SHP_CLS_CPU = 1,
+   SHP_CLS_MEMORY  = 2,
+   SHP_CLS_HOSTBRIDGE  = 3,
+   SHP_CLS_CONTAINER   = 4,
+};
+
+/*
+ * Hot-plug device information
+ */
+union shp_dev_info {
+   struct shp_cpu {
+   u32 cpu_id;
+   } cpu;
+
+   struct shp_memory {
+   int node;
+   u64 start_addr;
+   u64 length;
+   } mem;
+
+   struct shp_hostbridge {
+   } hb;
+
+   struct shp_node {
+   } node;
+};
+
+struct shp_device {
+   struct list_headlist;
+   struct device   *device;
+   enum shp_class  class;
+   union shp_dev_info  info;
+};
+
+/*
+ * Hot-plug request
+ */
+struct shp_request {
+   /* common info */
+   enum shp_operation  operation;  /* operation */
+
+   /* hot-plug event info: only valid for hot-plug operations */
+   void*handle;/* FW handle */
+   u32 event;  /* FW event */
+
+   /* device resource info */
+   struct list_headdev_list;   /* shp

[RFC PATCH v2 06/12] ACPI: Add ACPI bus hotplug handlers

[Toshi Kani]

Added ACPI bus hotplug handlers.  acpi_add_execute() calls
acpi_bus_add() to construct new acpi_device objects for hot-add
operation, and acpi_del_execute() calls acpi_bus_trim() to destruct
them for hot-delete operation.  They are also used for rollback
as well.

acpi_del_commit() calls _EJ0 to eject a target object for hot-delete.

acpi_validate_ost() calls _OST to inform FW that a hot-plug operation
completed with error in case of rollback.

Signed-off-by: Toshi Kani 
---
 drivers/acpi/bus.c |  133 
 1 file changed, 133 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 1f0d457..31a1911 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "internal.h"
 
@@ -52,6 +53,9 @@ struct acpi_device *acpi_root;
 struct proc_dir_entry *acpi_root_dir;
 EXPORT_SYMBOL(acpi_root_dir);
 
+static int acpi_add_execute(struct shp_request *req, int rollback);
+static int acpi_del_execute(struct shp_request *req, int rollback);
+
 #define STRUCT_TO_INT(s)   (*((int*)&s))
 
 
@@ -859,6 +863,134 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, 
void *data)
 }
 
 /* --
+   Hot-plug Handling
+   -- 
*/
+
+static int acpi_validate_ost(struct shp_request *req, int rollback)
+{
+   /* If hotplug request failed, inform firmware with error */
+   if (rollback && shp_is_hotplug_op(req->operation))
+   (void) acpi_evaluate_hotplug_ost(req->handle, req->event,
+   ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL);
+
+   return 0;
+}
+
+static int acpi_add_execute(struct shp_request *req, int rollback)
+{
+   acpi_handle handle = (acpi_handle) req->handle;
+   acpi_handle phandle;
+   struct acpi_device *device = NULL;
+   struct acpi_device *pdev;
+   int ret;
+
+   if (rollback)
+   return acpi_del_execute(req, 0);
+
+   /* only handle hot-plug operation */
+   if (!shp_is_hotplug_op(req->operation))
+   return 0;
+
+   if (acpi_get_parent(handle, &phandle))
+   return -ENODEV;
+
+   if (acpi_bus_get_device(phandle, &pdev))
+   return -ENODEV;
+
+   ret = acpi_bus_add(&device, pdev, handle, ACPI_BUS_TYPE_DEVICE);
+
+   return ret;
+}
+
+static int acpi_add_commit(struct shp_request *req, int rollback)
+{
+   /* Inform firmware that the hotplug operation has completed */
+   (void) acpi_evaluate_hotplug_ost(req->handle, req->event,
+   ACPI_OST_SC_SUCCESS, NULL);
+
+   return 0;
+}
+
+static int acpi_del_execute(struct shp_request *req, int rollback)
+{
+   acpi_handle handle = (acpi_handle) req->handle;
+   struct acpi_device *device;
+
+   if (rollback)
+   return acpi_add_execute(req, 0);
+
+   /* only handle hot-plug operation */
+   if (!shp_is_hotplug_op(req->operation))
+   return 0;
+
+   if (acpi_bus_get_device(handle, &device)) {
+   acpi_handle_err(handle, "Failed to obtain device\n");
+   return -EINVAL;
+   }
+
+   if (acpi_bus_trim(device, 1)) {
+   dev_err(&device->dev, "Removing device failed\n");
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int acpi_del_commit(struct shp_request *req, int rollback)
+{
+   acpi_handle handle = (acpi_handle) req->handle;
+   acpi_handle temp;
+   struct acpi_object_list arg_list;
+   union acpi_object arg;
+   acpi_status status;
+
+   /* only handle hot-plug operation */
+   if (!shp_is_hotplug_op(req->operation))
+   return 0;
+
+   /* power off device */
+   status = acpi_evaluate_object(handle, "_PS3", NULL, NULL);
+   if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
+   acpi_handle_warn(handle, "Power-off device failed\n");
+
+   if (ACPI_SUCCESS(acpi_get_handle(handle, "_LCK", &temp))) {
+   arg_list.count = 1;
+   arg_list.pointer = &arg;
+   arg.type = ACPI_TYPE_INTEGER;
+   arg.integer.value = 0;
+   acpi_evaluate_object(handle, "_LCK", &arg_list, NULL);
+   }
+
+   arg_list.count = 1;
+   arg_list.pointer = &arg;
+   arg.type = ACPI_TYPE_INTEGER;
+   arg.integer.value = 1;
+
+   status = acpi_evaluate_object(handle, "_EJ0", &arg_list, NULL);
+   if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND))
+   acpi_handle_warn(handle, "Eject device failed\n");
+
+

[RFC PATCH v2 07/12] ACPI: Add ACPI resource hotplug handler

[Toshi Kani]

Added ACPI resource handler for hotplug operations.  The handler,
acpi_set_shp_device(), sets device resource information to a hotplug
request, which is consumed by the CPU and memory handlers.
For setting the device resource information, acpi_scan_shp_devices()
walks the acpi_device tree from a target device, and calls .resource
of ACPI drivers.

For hot-add, acpi_set_shp_device() is called right after the ACPI bus
handler so that it can walk through new acpi_device objects.  For
hot-delete, it is called at the begging of the validate phase so that
other validate handlers can use the device resource information for
their validations.

Signed-off-by: Toshi Kani 
---
 drivers/acpi/Makefile   |1 +
 drivers/acpi/bus.c  |1 +
 drivers/acpi/internal.h |1 +
 drivers/acpi/shp_resource.c |   86 +++
 include/acpi/acpi_bus.h |4 ++
 5 files changed, 93 insertions(+)
 create mode 100644 drivers/acpi/shp_resource.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 2a4502b..205be23 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -34,6 +34,7 @@ acpi-$(CONFIG_ACPI_SLEEP) += proc.o
 acpi-y += bus.o glue.o
 acpi-y += scan.o
 acpi-y += resource.o
+acpi-y += shp_resource.o
 acpi-y += processor_core.o
 acpi-y += ec.o
 acpi-$(CONFIG_ACPI_DOCK)   += dock.o
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 31a1911..69b5edb 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1236,6 +1236,7 @@ static int __init acpi_init(void)
acpi_sleep_proc_init();
acpi_wakeup_device_init();
acpi_shp_init();
+   acpi_shp_res_init();
return 0;
 }
 
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 3c407cd..51aa740 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -26,6 +26,7 @@
 int init_acpi_device_notify(void);
 int acpi_scan_init(void);
 int acpi_sysfs_init(void);
+void acpi_shp_res_init(void);
 
 #ifdef CONFIG_DEBUG_FS
 extern struct dentry *acpi_debugfs_dir;
diff --git a/drivers/acpi/shp_resource.c b/drivers/acpi/shp_resource.c
new file mode 100644
index 000..51ab968
--- /dev/null
+++ b/drivers/acpi/shp_resource.c
@@ -0,0 +1,86 @@
+/*
+ * shp_resource.c - Setup system device hot-plug resource information
+ *
+ * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
+ * Toshi Kani 
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "internal.h"
+
+static int
+acpi_set_shp_device(struct acpi_device *device, struct shp_request *req)
+{
+   int ret;
+
+   if (!device->driver) {
+   dev_dbg(&device->dev, "driver not bound\n");
+   return 0;
+   }
+
+   if (!device->driver->ops.resource)
+   return 0;
+
+   ret = device->driver->ops.resource(device, req);
+   if (ret) {
+   dev_err(&device->dev, "ops.resource failed (%d)\n", ret);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int
+acpi_scan_shp_devices(struct acpi_device *device, struct shp_request *req)
+{
+   struct acpi_device *child = NULL;
+
+   if (acpi_set_shp_device(device, req))
+   return 0;
+
+   list_for_each_entry(child, &device->children, node)
+   acpi_scan_shp_devices(child, req);
+
+   return 0;
+}
+
+static int acpi_set_shp_resources(struct shp_request *req, int rollback)
+{
+   acpi_handle handle = (acpi_handle) req->handle;
+   struct acpi_device *device = NULL;
+
+   if (rollback)
+   return 0;
+
+   /* only handle hot-plug operation */
+   if (!shp_is_hotplug_op(req->operation))
+   return 0;
+
+   if (acpi_bus_get_device(handle, &device)) {
+   acpi_handle_err(handle, "acpi_bus_get_device failed\n");
+   return -EINVAL;
+   }
+
+   acpi_scan_shp_devices(device, req);
+
+   return 0;
+}
+
+void __init acpi_shp_res_init(void)
+{
+   shp_register_handler(SHP_ADD_EXECUTE, acpi_set_shp_resources,
+   SHP_ACPI_RES_ADD_EXECUTE_ORDER);
+   shp_register_handler(SHP_DEL_VALIDATE, acpi_set_shp_resources,
+   SHP_ACPI_RES_DEL_VALIDATE_ORDER);
+}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 7ced5dc..6bf002e 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -27,6 +27,7 @@
 #define __ACPI_BUS_H__
 
 #include 
+#include 
 
 #include 
 
@@ -94,6 +95,8 @@ typedef int (*acpi_op_

[RFC PATCH v2 03/12] drivers/base: Add system device hotplug framework

[Toshi Kani]

Added sys_hotplug.c, which is the system device hotplug framework code.

shp_register_handler() allows modules to register their hotplug handlers
to the framework.  shp_submit_req() provides the interface to submit
a hotplug or online/offline request of system devices.  The request is
then put into hp_workqueue.  shp_start_req() calls all registered handlers
in ascending order for each phase.  If any handler failed in validate or
execute phase, shp_start_req() initiates its rollback procedure.

Signed-off-by: Toshi Kani 
---
 drivers/base/Makefile  |1 
 drivers/base/sys_hotplug.c |  313 
 2 files changed, 314 insertions(+)
 create mode 100644 drivers/base/sys_hotplug.c

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 5aa2d70..2e9b2f1 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -21,6 +21,7 @@ endif
 obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
 obj-$(CONFIG_REGMAP)   += regmap/
 obj-$(CONFIG_SOC_BUS) += soc.o
+obj-y  += sys_hotplug.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 
diff --git a/drivers/base/sys_hotplug.c b/drivers/base/sys_hotplug.c
new file mode 100644
index 000..c5f5285
--- /dev/null
+++ b/drivers/base/sys_hotplug.c
@@ -0,0 +1,313 @@
+/*
+ * sys_hotplug.c - System device hot-plug framework
+ *
+ * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
+ * Toshi Kani 
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Hot-plug handler list
+ */
+struct shp_handler {
+   struct list_headshp_list;
+   int shp_order;
+   shp_funcshp_func;
+};
+
+LIST_HEAD(shp_add_list_head);
+LIST_HEAD(shp_del_list_head);
+
+static DEFINE_MUTEX(shp_hdr_list_lock);
+
+#define SHP_VALIDATE_ORDER_BASE(SHP_ORDER_MAX+1)
+#define SHP_EXECUTE_ORDER_BASE ((SHP_ORDER_MAX+1) << 1)
+#define SHP_COMMIT_ORDER_BASE  ((SHP_ORDER_MAX+1) << 2)
+
+/*
+ * Hot-plug request work queue
+ */
+struct shp_work {
+   struct shp_request  *request;
+   struct work_struct  work;
+};
+
+static struct workqueue_struct *shp_workqueue;
+
+/* trace messages */
+static int shp_trace = 1;
+static char shp_ksym_buf[KSYM_NAME_LEN];
+module_param(shp_trace, int, 0644);
+MODULE_PARM_DESC(shp_trace, "Enable system device hot-plug trace messages");
+
+static char *shp_operation_string(enum shp_operation operation)
+{
+   switch (operation) {
+   case SHP_HOTPLUG_ADD:
+   return "Hot-Add";
+   case SHP_HOTPLUG_DEL:
+   return "Hot-Delete";
+   case SHP_ONLINE_ADD:
+   return "Online";
+   case SHP_ONLINE_DEL:
+   return "Offline";
+   }
+
+   return "n/a";
+}
+
+static u32 shp_get_order_base(enum shp_phase phase)
+{
+   switch (phase) {
+   case SHP_ADD_VALIDATE:
+   case SHP_DEL_VALIDATE:
+   return SHP_VALIDATE_ORDER_BASE;
+   case SHP_ADD_EXECUTE:
+   case SHP_DEL_EXECUTE:
+   return SHP_EXECUTE_ORDER_BASE;
+   case SHP_ADD_COMMIT:
+   case SHP_DEL_COMMIT:
+   return SHP_COMMIT_ORDER_BASE;
+   }
+
+   return 0;
+}
+
+/**
+ * shp_register_handler - register a hot-plug handler to the framework
+ * @phase: hot-plug phase
+ * @func: Hot-plug function
+ * @order: Pre-defined order value
+ */
+int shp_register_handler(enum shp_phase phase, shp_func func, u32 order)
+{
+   struct list_head *head;
+   struct shp_handler *hdr, *cur;
+   u32 order_base;
+   int insert = 0;
+
+   if (!func || order > SHP_ORDER_MAX)
+   return -EINVAL;
+
+   if (shp_is_add_phase(phase))
+   head = &shp_add_list_head;
+   else
+   head = &shp_del_list_head;
+
+   order_base = shp_get_order_base(phase);
+
+   hdr = kzalloc(sizeof(*hdr), GFP_KERNEL);
+   if (!hdr)
+   return -ENOMEM;
+
+   hdr->shp_order = order + order_base;
+   hdr->shp_func = func;
+
+   /*
+* Add this handler to the list in ascending order
+*/
+   mutex_lock(&shp_hdr_list_lock);
+   if (list_empty(head)) {
+   list_add(&hdr->shp_list, head);
+   } else {
+   list_for_each_entry(cur, head, shp_list)
+   if (cur->shp_order > hdr->shp_order) {
+   insert = 1;
+   break;
+   }
+
+   if (insert)
+   __list_add(&hdr->shp_list,
+   cur->shp_list.prev, &cur->shp_list);
+

[RFC PATCH v2 04/12] cpu: Add cpu hotplug handlers

[Toshi Kani]

Added cpu hotplug handlers.  cpu_add_execute() onlines requested
cpus for hot-add & online operations, and cpu_del_execute()
offlines them for hot-delete & offline operations.  They are
also used for rollback as well.

cpu_del_validate() fails a request if cpu0 is requested to delete.

Signed-off-by: Toshi Kani 
---
 drivers/base/cpu.c |  107 
 1 file changed, 107 insertions(+)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 6345294..05534ad 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -13,6 +13,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "base.h"
 
@@ -324,10 +326,115 @@ static void __init cpu_dev_register_generic(void)
 #endif
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+static int cpu_del_execute(struct shp_request *req, int rollback);
+
+static int cpu_add_execute(struct shp_request *req, int rollback)
+{
+   struct shp_device *shp_dev;
+   u32 cpu;
+   int ret;
+
+   if (rollback)
+   return cpu_del_execute(req, 0);
+
+   list_for_each_entry(shp_dev, &req->dev_list, list) {
+   if (shp_dev->class != SHP_CLS_CPU)
+   continue;
+
+   cpu = shp_dev->info.cpu.cpu_id;
+
+   if (cpu_online(cpu))
+   continue;
+
+   ret = cpu_up(cpu);
+   if (!ret) {
+   /* REVISIT: need a way to set a cpu dev for hot-plug */
+   if (shp_is_online_op(req->operation))
+   kobject_uevent(&shp_dev->device->kobj,
+   KOBJ_ONLINE);
+   } else {
+   pr_err("cpu: Failed to online cpu %d\n", cpu);
+   /* fall-thru */
+   }
+   }
+
+   return 0;
+}
+
+static int cpu_del_validate(struct shp_request *req, int rollback)
+{
+   struct shp_device *shp_dev;
+
+   if (rollback)
+   return 0;
+
+   list_for_each_entry(shp_dev, &req->dev_list, list) {
+   if (shp_dev->class != SHP_CLS_CPU)
+   continue;
+
+   /*
+* cpu 0 cannot be offlined.  This check can be removed when
+* cpu 0 offline is supported.
+*/
+   if (shp_dev->info.cpu.cpu_id == 0)
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int cpu_del_execute(struct shp_request *req, int rollback)
+{
+   struct shp_device *shp_dev;
+   u32 cpu;
+   int ret;
+
+   if (rollback)
+   return cpu_add_execute(req, 0);
+
+   list_for_each_entry(shp_dev, &req->dev_list, list) {
+   if (shp_dev->class != SHP_CLS_CPU)
+   continue;
+
+   cpu = shp_dev->info.cpu.cpu_id;
+
+   if (!cpu_online(cpu))
+   continue;
+
+   ret = cpu_down(cpu);
+   if (!ret) {
+   /* REVISIT: need a way to set a cpu dev for hot-plug */
+   if (shp_is_online_op(req->operation))
+   kobject_uevent(&shp_dev->device->kobj,
+   KOBJ_OFFLINE);
+   } else {
+   pr_err("cpu: Failed to offline cpu %d\n", cpu);
+   return ret;
+   }
+   }
+
+   return 0;
+}
+
+static void __init cpu_shp_init(void)
+{
+   shp_register_handler(SHP_ADD_EXECUTE, cpu_add_execute,
+   SHP_CPU_ADD_EXECUTE_ORDER);
+   shp_register_handler(SHP_DEL_VALIDATE, cpu_del_validate,
+   SHP_CPU_DEL_VALIDATE_ORDER);
+   shp_register_handler(SHP_DEL_EXECUTE, cpu_del_execute,
+   SHP_CPU_DEL_EXECUTE_ORDER);
+}
+#endif /* CONFIG_HOTPLUG_CPU */
+
 void __init cpu_dev_init(void)
 {
if (subsys_system_register(&cpu_subsys, cpu_root_attr_groups))
panic("Failed to register CPU subsystem");
 
cpu_dev_register_generic();
+#ifdef CONFIG_HOTPLUG_CPU
+   cpu_shp_init();
+#endif
 }
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[RFC PATCH v2 11/12] cpu: Update sysfs cpu/online for hotplug framework

[Toshi Kani]

Changed store_online() to request a cpu online or offline
operation by calling shp_submit_req().  It sets a target cpu
device information with shp_add_dev_info() for the request.

Signed-off-by: Toshi Kani 
---
 drivers/base/cpu.c |   40 
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 05534ad..cd1cbdc 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -41,27 +41,43 @@ static ssize_t __ref store_online(struct device *dev,
  const char *buf, size_t count)
 {
struct cpu *cpu = container_of(dev, struct cpu, dev);
-   ssize_t ret;
+   struct shp_request *shp_req;
+   struct shp_device *shp_dev;
+   enum shp_operation operation;
+   ssize_t ret = count;
 
-   cpu_hotplug_driver_lock();
switch (buf[0]) {
case '0':
-   ret = cpu_down(cpu->dev.id);
-   if (!ret)
-   kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
+   operation = SHP_ONLINE_DEL;
break;
case '1':
-   ret = cpu_up(cpu->dev.id);
-   if (!ret)
-   kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+   operation = SHP_ONLINE_ADD;
break;
default:
-   ret = -EINVAL;
+   return -EINVAL;
+   }
+
+   shp_req = shp_alloc_request(operation);
+   if (!shp_req)
+   return -ENOMEM;
+
+   shp_dev = kzalloc(sizeof(*shp_dev), GFP_KERNEL);
+   if (!shp_dev) {
+   kfree(shp_req);
+   return -ENOMEM;
+   }
+
+   shp_dev->device = dev;
+   shp_dev->class = SHP_CLS_CPU;
+   shp_dev->info.cpu.cpu_id = cpu->dev.id;
+   shp_add_dev_info(shp_req, shp_dev);
+
+   if (shp_submit_req(shp_req)) {
+   kfree(shp_dev);
+   kfree(shp_req);
+   return -EINVAL;
}
-   cpu_hotplug_driver_unlock();
 
-   if (ret >= 0)
-   ret = count;
return ret;
 }
 static DEVICE_ATTR(online, 0644, show_online, store_online);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[RFC PATCH v2 00/12] System device hot-plug framework

[Toshi Kani]

annot be rolled-back.
For instance, the ACPI module invokes _EJ0 for a hot-remove operation.

System Device Management Modules


CPU Handlers

CPU handlers are provided by the CPU driver in drivers/base/cpu.c, and
perform CPU online/offline procedures when CPU device(s) is added or
deleted during an operation.

Memory Handlers
---
Memory handlers are provided by the memory module in mm/memory_hotplug.c,
and perform Memory online/offline procedure when memory device(s) is
added or deleted during an operation.

FW-dependent Modules


ACPI Bus Handlers
-
ACPI bus handlers are provided by the ACPI core in drivers/acpi/bus.c,
and construct/destruct acpi_device object(s) during a hot-plug operation.

ACPI Resource Handlers
--
ACPI resource handlers are provided by the ACPI core in
drivers/acpi/hp_resource.c, and set device resource information to
a request during a hot-plug operation.  This device resource information
is then consumed by the system device management modules for their
online/offline procedure.

ACPI Drivers

ACPI drivers are called from the ACPI core during a hot-plug operation
through the following interfaces.  ACPI drivers are not called from the
framework directly, and remain internal to the ACPI core.  ACPI drivers
may not initiate online/offline of a device.

.add - Construct device-specific information to a given acpi_device.
Called at boot, hot-add and sysfs bind.

.remove - Destruct device-specific information to a given acpi_device.
Called at hot-remove and sysfs unbind.

.resource - Set device-specific resource information to a given hot-plug
request.  Called at hot-add and hot-remove.

---
v2:
 - Documented that system devices may not be initialized through the driver
   structures.
 - Clarified that the framework is for "system device" hotplug by changing
   file name, prefix and documentation.
 - Removed the use of CONFIG_HOTPLUG.
 - Moved ACPI specific definitions to include/acpi/sys_hotplug.h.
 - Implemented shp_unregister_handler() and added locking.
 - Added module parameters, shp_trace and del_movable_only.

---
Toshi Kani (12):
 Add sys_hotplug.h for system device hotplug framework
 ACPI: Add sys_hotplug.h for system device hotplug framework
 drivers/base: Add system device hotplug framework 
 cpu: Add cpu hotplug handlers
 mm: Add memory hotplug handlers
 ACPI: Add ACPI bus hotplug handlers
 ACPI: Add ACPI resource hotplug handler
 ACPI: Update processor driver for hotplug framework
 ACPI: Update memory driver for hotplug framework
 ACPI: Update container driver for hotplug framework
 cpu: Update sysfs cpu/online for hotplug framework
 ACPI: Update sysfs eject for hotplug framework

---
 drivers/acpi/Makefile   |   1 +
 drivers/acpi/acpi_memhotplug.c  | 271 --
 drivers/acpi/bus.c  | 134 +
 drivers/acpi/container.c|  95 +---
 drivers/acpi/internal.h |   1 +
 drivers/acpi/processor_driver.c | 150 +--
 drivers/acpi/scan.c | 122 +++-
 drivers/acpi/shp_resource.c |  86 +++
 drivers/base/Makefile   |   1 +
 drivers/base/cpu.c  | 147 +--
 drivers/base/sys_hotplug.c  | 313 
 include/acpi/acpi_bus.h |   8 +-
 include/acpi/sys_hotplug.h  |  48 ++
 include/linux/sys_hotplug.h | 181 +++
 mm/memory_hotplug.c | 101 +
 15 files changed, 1224 insertions(+), 435 deletions(-)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[RFC PATCH v2 05/12] mm: Add memory hotplug handlers

[Toshi Kani]

Added memory hotplug handlers.  mm_add_execute() onlines requested
memory ranges for hot-add & online operations, and mm_del_execute()
offlines them for hot-delete & offline operations.  They are also
used for rollback as well.

mm_del_validate() fails a hot-delete request if a requested memory
range is non-movable when del_movable_only is set.

Signed-off-by: Toshi Kani 
---
 mm/memory_hotplug.c |  101 +++
 1 file changed, 101 insertions(+)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index d04ed87..ed3d829 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -29,6 +29,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 
@@ -45,6 +47,13 @@ static void generic_online_page(struct page *page);
 
 static online_page_callback_t online_page_callback = generic_online_page;
 
+static int mm_add_execute(struct shp_request *req, int rollback);
+static int mm_del_execute(struct shp_request *req, int rollback);
+
+static int del_movable_only = 0;
+module_param(del_movable_only, int, 0644);
+MODULE_PARM_DESC(del_movable_only, "Restrict hot-remove to movable memory 
only");
+
 DEFINE_MUTEX(mem_hotplug_mutex);
 
 void lock_memory_hotplug(void)
@@ -1431,3 +1440,95 @@ int remove_memory(u64 start, u64 size)
 }
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 EXPORT_SYMBOL_GPL(remove_memory);
+
+static int mm_add_execute(struct shp_request *req, int rollback)
+{
+   struct shp_device *shp_dev;
+   struct shp_memory *shp_mem;
+   int ret;
+
+   if (rollback)
+   return mm_del_execute(req, 0);
+
+   list_for_each_entry(shp_dev, &req->dev_list, list) {
+   if (shp_dev->class != SHP_CLS_MEMORY)
+   continue;
+
+   shp_mem = &shp_dev->info.mem;
+
+   ret = add_memory(shp_mem->node,
+   shp_mem->start_addr, shp_mem->length);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+
+static int mm_del_validate(struct shp_request *req, int rollback)
+{
+   struct shp_device *shp_dev;
+   struct shp_memory *shp_mem;
+   unsigned long start_pfn, nr_pages;
+
+   if (rollback || !del_movable_only)
+   return 0;
+
+   list_for_each_entry(shp_dev, &req->dev_list, list) {
+   if (shp_dev->class != SHP_CLS_MEMORY)
+   continue;
+
+   shp_mem = &shp_dev->info.mem;
+   start_pfn = shp_mem->start_addr >> PAGE_SHIFT;
+   nr_pages = PAGE_ALIGN(shp_mem->length) >> PAGE_SHIFT;
+
+   /*
+* Check if this memory range is removable.  This check is
+* enabled when del_movable_only is set.
+*/
+   if (is_mem_section_removable(start_pfn, nr_pages)) {
+   pr_info("Memory [%#010llx-%#010llx] not removable\n",
+   shp_mem->start_addr,
+   shp_mem->start_addr + shp_mem->length-1);
+   return -EINVAL;
+   }
+   }
+
+   return 0;
+}
+
+static int mm_del_execute(struct shp_request *req, int rollback)
+{
+   struct shp_device *shp_dev;
+   struct shp_memory *shp_mem;
+   int ret;
+
+   if (rollback)
+   return mm_add_execute(req, 0);
+
+   list_for_each_entry(shp_dev, &req->dev_list, list) {
+   if (shp_dev->class != SHP_CLS_MEMORY)
+   continue;
+
+   shp_mem = &shp_dev->info.mem;
+
+   ret = remove_memory(shp_mem->start_addr, shp_mem->length);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+
+static int __init mm_shp_init(void)
+{
+   shp_register_handler(SHP_ADD_EXECUTE, mm_add_execute,
+   SHP_MEM_ADD_EXECUTE_ORDER);
+   shp_register_handler(SHP_DEL_VALIDATE, mm_del_validate,
+   SHP_MEM_DEL_VALIDATE_ORDER);
+   shp_register_handler(SHP_DEL_EXECUTE, mm_del_execute,
+   SHP_MEM_DEL_EXECUTE_ORDER);
+
+   return 0;
+}
+module_init(mm_shp_init);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Fri, 2013-01-11 at 22:23 +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 04:40:19 PM Toshi Kani wrote:
> > Added include/linux/sys_hotplug.h, which defines the system device
> > hotplug framework interfaces used by the framework itself and
> > handlers.
> > 
> > The order values define the calling sequence of handlers.  For add
> > execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
> > CPU so that threads on new CPUs can start using their local memory.
> > The ordering of the delete execute is symmetric to the add execute.
> > 
> > struct shp_request defines a hot-plug request information.  The
> > device resource information is managed with a list so that a single
> > request may target to multiple devices.
> > 
 :
> > +
> > +struct shp_device {
> > +   struct list_headlist;
> > +   struct device   *device;
> > +   enum shp_class  class;
> > +   union shp_dev_info  info;
> > +};
> > +
> > +/*
> > + * Hot-plug request
> > + */
> > +struct shp_request {
> > +   /* common info */
> > +   enum shp_operation  operation;  /* operation */
> > +
> > +   /* hot-plug event info: only valid for hot-plug operations */
> > +   void*handle;/* FW handle */
> 
> What's the role of handle here?

On ACPI-based platforms, the handle keeps a notified ACPI handle when a
hot-plug request is made.  ACPI bus handlers, acpi_add_execute() /
acpi_del_execute(), then scans / trims ACPI devices from the handle.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > device hotplug header and defines the order values of ACPI-specific
> > handlers.
> > 
> > Signed-off-by: Toshi Kani 
> > ---
> >  include/acpi/sys_hotplug.h |   48 
> > 
> >  1 file changed, 48 insertions(+)
> >  create mode 100644 include/acpi/sys_hotplug.h
> > 
> > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > new file mode 100644
> > index 000..ad80f61
> > --- /dev/null
> > +++ b/include/acpi/sys_hotplug.h
> > @@ -0,0 +1,48 @@
> > +/*
> > + * sys_hotplug.h - ACPI System device hot-plug framework
> > + *
> > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > + * Toshi Kani 
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef _ACPI_SYS_HOTPLUG_H
> > +#define _ACPI_SYS_HOTPLUG_H
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/*
> > + * System device hot-plug operation proceeds in the following order.
> > + *   Validate phase -> Execute phase -> Commit phase
> > + *
> > + * The order values below define the calling sequence of ACPI-specific
> > + * handlers for each phase in ascending order.  The order value of
> > + * platform-neutral handlers are defined in .
> > + */
> > +
> > +/* Add Validate order values */
> > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must be 
> > first */
> > +
> > +/* Add Execute order values */
> > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > +
> > +/* Add Commit order values */
> > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > +
> > +/* Delete Validate order values */
> > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must be 
> > first */
> > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
> > +
> > +/* Delete Execute order values */
> > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
> > +
> > +/* Delete Commit order values */
> > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
> > +
> > +#endif /* _ACPI_SYS_HOTPLUG_H */
> > --
> 
> Why did you use the particular values above?

The ordering values above are used to define the relative order among
handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
potentially be 21 since it is still larger than 20 for
SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
so that more platform-neutral handlers can be added in between 20 and
100 in future.

Thanks,
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > device hotplug header and defines the order values of ACPI-specific
> > > > handlers.
> > > > 
> > > > Signed-off-by: Toshi Kani 
> > > > ---
> > > >  include/acpi/sys_hotplug.h |   48 
> > > > 
> > > >  1 file changed, 48 insertions(+)
> > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > 
> > > > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > > > new file mode 100644
> > > > index 000..ad80f61
> > > > --- /dev/null
> > > > +++ b/include/acpi/sys_hotplug.h
> > > > @@ -0,0 +1,48 @@
> > > > +/*
> > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > + *
> > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > + * Toshi Kani 
> > > > + *
> > > > + * 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.
> > > > + */
> > > > +
> > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +/*
> > > > + * System device hot-plug operation proceeds in the following order.
> > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > + *
> > > > + * The order values below define the calling sequence of ACPI-specific
> > > > + * handlers for each phase in ascending order.  The order value of
> > > > + * platform-neutral handlers are defined in .
> > > > + */
> > > > +
> > > > +/* Add Validate order values */
> > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must 
> > > > be first */
> > > > +
> > > > +/* Add Execute order values */
> > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > > > +
> > > > +/* Add Commit order values */
> > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > > > +
> > > > +/* Delete Validate order values */
> > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must 
> > > > be first */
> > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
> > > > +
> > > > +/* Delete Execute order values */
> > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
> > > > +
> > > > +/* Delete Commit order values */
> > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
> > > > +
> > > > +#endif /* _ACPI_SYS_HOTPLUG_H */
> > > > --
> > > 
> > > Why did you use the particular values above?
> > 
> > The ordering values above are used to define the relative order among
> > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
> > potentially be 21 since it is still larger than 20 for
> > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
> > so that more platform-neutral handlers can be added in between 20 and
> > 100 in future.
> 
> I thought so, but I don't think it's a good idea to add gaps like this.

OK, I will use an equal gap of 10 for all values.  So, the 100 in the
above example will be changed to 30.   

Thanks,
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-01-14 at 19:48 +0100, Rafael J. Wysocki wrote:
> On Monday, January 14, 2013 08:33:48 AM Toshi Kani wrote:
> > On Fri, 2013-01-11 at 22:23 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 10, 2013 04:40:19 PM Toshi Kani wrote:
> > > > Added include/linux/sys_hotplug.h, which defines the system device
> > > > hotplug framework interfaces used by the framework itself and
> > > > handlers.
> > > > 
> > > > The order values define the calling sequence of handlers.  For add
> > > > execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
> > > > CPU so that threads on new CPUs can start using their local memory.
> > > > The ordering of the delete execute is symmetric to the add execute.
> > > > 
> > > > struct shp_request defines a hot-plug request information.  The
> > > > device resource information is managed with a list so that a single
> > > > request may target to multiple devices.
> > > > 
> >  :
> > > > +
> > > > +struct shp_device {
> > > > +   struct list_headlist;
> > > > +   struct device   *device;
> > > > +   enum shp_class  class;
> > > > +   union shp_dev_info  info;
> > > > +};
> > > > +
> > > > +/*
> > > > + * Hot-plug request
> > > > + */
> > > > +struct shp_request {
> > > > +   /* common info */
> > > > +   enum shp_operation  operation;  /* operation */
> > > > +
> > > > +   /* hot-plug event info: only valid for hot-plug operations */
> > > > +   void*handle;/* FW handle */
> > > 
> > > What's the role of handle here?
> > 
> > On ACPI-based platforms, the handle keeps a notified ACPI handle when a
> > hot-plug request is made.  ACPI bus handlers, acpi_add_execute() /
> > acpi_del_execute(), then scans / trims ACPI devices from the handle.
> 
> OK, so this is ACPI-specific and should be described as such.

Other FW interface I know is parisc, which has mod_index (module index)
to identify a unique object, just like what ACPI handle does.  The
handle can keep the mod_index as an opaque value as well.  But as you
said, I do not know if the handle works for all other FWs.  So, I will
add descriptions, such that the hot-plug event info is modeled after
ACPI and may need to be revisited when supporting other FW.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-01-14 at 20:07 +0100, Rafael J. Wysocki wrote:
> On Monday, January 14, 2013 11:42:09 AM Toshi Kani wrote:
> > On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> > > On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > > > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > > > device hotplug header and defines the order values of ACPI-specific
> > > > > > handlers.
> > > > > > 
> > > > > > Signed-off-by: Toshi Kani 
> > > > > > ---
> > > > > >  include/acpi/sys_hotplug.h |   48 
> > > > > > 
> > > > > >  1 file changed, 48 insertions(+)
> > > > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > > > 
> > > > > > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > > > > > new file mode 100644
> > > > > > index 000..ad80f61
> > > > > > --- /dev/null
> > > > > > +++ b/include/acpi/sys_hotplug.h
> > > > > > @@ -0,0 +1,48 @@
> > > > > > +/*
> > > > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > > > + *
> > > > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > > > + * Toshi Kani 
> > > > > > + *
> > > > > > + * 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.
> > > > > > + */
> > > > > > +
> > > > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > > > +
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +
> > > > > > +/*
> > > > > > + * System device hot-plug operation proceeds in the following 
> > > > > > order.
> > > > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > > > + *
> > > > > > + * The order values below define the calling sequence of 
> > > > > > ACPI-specific
> > > > > > + * handlers for each phase in ascending order.  The order value of
> > > > > > + * platform-neutral handlers are defined in .
> > > > > > + */
> > > > > > +
> > > > > > +/* Add Validate order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must 
> > > > > > be first */
> > > > > > +
> > > > > > +/* Add Execute order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > > > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > > > > > +
> > > > > > +/* Add Commit order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > > > > > +
> > > > > > +/* Delete Validate order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must 
> > > > > > be first */
> > > > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
> > > > > > +
> > > > > > +/* Delete Execute order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
> > > > > > +
> > > > > > +/* Delete Commit order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
> > > > > > +
> > > > > > +#endif /* _ACPI_SYS_HOTPLUG_H */
> > > > > > --
> > > > > 
> > > > > Why did you use the particular values above?
> > > > 
> > > > The ordering values above are used to define the relative order among
> > > > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
> > > > potentially be 21 since it is still larger than 20 for
> > > > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
> > > > so that more platform-neutral handlers can be added in between 20 and
> > > > 100 in future.
> > > 
> > > I thought so, but I don't think it's a good idea to add gaps like this.
> > 
> > OK, I will use an equal gap of 10 for all values.  So, the 100 in the
> > above example will be changed to 30.  
> 
> I wonder why you want to have those gaps at all.

Oh, I see.  I think some gap is helpful since it allows a new handler to
come between without recompiling other modules.  For instance, OEM
vendors may want to add their own handlers with loadable modules after
the kernel is distributed.

> Anyway, this is just a small detail and it doesn't mean I don't have more
> comments.  I just need some more time to get the big picture idea of how this
> is supposed to work and perhaps Greg will have some remarks too.

Yes, I am well-aware of that. :-)  Please let me know if you have any
questions.  I'd be happy to explain any details.

Thanks a lot for reviewing!
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-01-14 at 11:21 -0800, Greg KH wrote:
> On Mon, Jan 14, 2013 at 08:07:35PM +0100, Rafael J. Wysocki wrote:
> > On Monday, January 14, 2013 11:42:09 AM Toshi Kani wrote:
> > > On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> > > > On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > > > > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > > > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > > > > device hotplug header and defines the order values of 
> > > > > > > ACPI-specific
> > > > > > > handlers.
> > > > > > > 
> > > > > > > Signed-off-by: Toshi Kani 
> > > > > > > ---
> > > > > > >  include/acpi/sys_hotplug.h |   48 
> > > > > > > 
> > > > > > >  1 file changed, 48 insertions(+)
> > > > > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > > > > 
> > > > > > > diff --git a/include/acpi/sys_hotplug.h 
> > > > > > > b/include/acpi/sys_hotplug.h
> > > > > > > new file mode 100644
> > > > > > > index 000..ad80f61
> > > > > > > --- /dev/null
> > > > > > > +++ b/include/acpi/sys_hotplug.h
> > > > > > > @@ -0,0 +1,48 @@
> > > > > > > +/*
> > > > > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > > > > + *
> > > > > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > > > > + *   Toshi Kani 
> > > > > > > + *
> > > > > > > + * 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.
> > > > > > > + */
> > > > > > > +
> > > > > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > > > > +
> > > > > > > +#include 
> > > > > > > +#include 
> > > > > > > +#include 
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * System device hot-plug operation proceeds in the following 
> > > > > > > order.
> > > > > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > > > > + *
> > > > > > > + * The order values below define the calling sequence of 
> > > > > > > ACPI-specific
> > > > > > > + * handlers for each phase in ascending order.  The order value 
> > > > > > > of
> > > > > > > + * platform-neutral handlers are defined in 
> > > > > > > .
> > > > > > > + */
> > > > > > > +
> > > > > > > +/* Add Validate order values */
> > > > > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER  0   /* must 
> > > > > > > be first */
> > > > > > > +
> > > > > > > +/* Add Execute order values */
> > > > > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER   10
> > > > > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER   20
> > > > > > > +
> > > > > > > +/* Add Commit order values */
> > > > > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER10
> > > > > > > +
> > > > > > > +/* Delete Validate order values */
> > > > > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER  0   /* must 
> > > > > > > be first */
> > > > > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER  10
> > > > > > > +
> > > > > > > +/* Delete Execute order values */
> > > > > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER   100
> > > > > > > +
> > > > > > > +/* Delete Commit order values */
> > > > > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER100
> > > > > > > +
> > > > > > > +#endif   /* _ACPI_SYS_HOTPLUG_H */
> > > > > > > --
> > > > > > 
> > > > > > Why did you use the particular values above?
> > > > > 
> > > > > The ordering values above are used to define the relative order among
> > > > > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER 
> > > > > can
> > > > > potentially be 21 since it is still larger than 20 for
> > > > > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 
> > > > > 100
> > > > > so that more platform-neutral handlers can be added in between 20 and
> > > > > 100 in future.
> > > > 
> > > > I thought so, but I don't think it's a good idea to add gaps like this.
> > > 
> > > OK, I will use an equal gap of 10 for all values.  So, the 100 in the
> > > above example will be changed to 30.  
> > 
> > I wonder why you want to have those gaps at all.
> > 
> > Anyway, this is just a small detail and it doesn't mean I don't have more
> > comments.  I just need some more time to get the big picture idea of how 
> > this
> > is supposed to work and perhaps Greg will have some remarks too.
> 
> Yes, give me a few days to catch up on other patches before I get the
> chance to review these.

That's great!  Thanks Greg!
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 00/12] System device hot-plug framework

[Toshi Kani]

On Thu, 2013-01-17 at 01:50 +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 04:40:18 PM Toshi Kani wrote:
> > This patchset is a prototype of proposed system device hot-plug framework
> > for design review.  Unlike other hot-plug environments, such as USB and
> > PCI, there is no common framework for system device hot-plug [1].
> > Therefore, this patchset is designed to provide a common framework for
> > hot-plugging and online/offline operations of system devices, such as CPU,
> > Memory and Node.  While this patchset only supports ACPI-based hot-plug
> > operations, the framework itself is designed to be platform-neural and
> > can support other FW architectures as necessary.
 :
> At this point I'd like to clearly understand how the code is supposed to work.

Thanks for reviewing!

> From what I can say at the moment it all boils down to having two (ordered)
> lists of notifiers (shp_add_list, shp_del_list) that can be added to or 
> removed
> from with shp_register_handler() and shp_unregister_handler(), respectively

Yes.

> (BTW, the abbreviation "hdr" makes me think about a "header" rather than a
> "handler", but maybe that's just me :-)), 

Well, it makes me think that way as well. :)  How about "hdlr"?

> and a workqueue for requests (why do
> we need a separate workqueue for that?).

This workqueue needs to be platform-neutral and max_active set to 1, and
preferably is dedicated for hotplug operations.  kacpi_hotplug_wq is
close, but is ACPI-specific.  So, I decided to create a new workqueue
for this framework.

> Whoever needs to carry out a hotplug operation is supposed to prepare a 
> request
> and then put it into the workqueue with shp_submit_request().  The framework
> will then execute all of the notifier callbacks from the appropriate notifier
> list (depending on whether the operation is a hot-add or a hot-remove).  If 
> any
> of those callbacks returns an error code and it is not too late (the order of
> the failing notifier is not too high), the already executed notifier callbacks
> will be run again with the "rollback" argument set to 1 (why not to use bool?)

Agreed.  I will change the rollback to bool.

> to indicate that they are supposed to bring things back to the initial state.
> Error codes returned in that stage only cause messages to be printed.
>
> Is the description above correct?

Yes.  It's very good summary!

> If so, it looks like subsystems are supposed to register notifiers (handlers)
> for hotplug/hot-remove operations of the devices they handle.  They are
> supposed to use predefined order values to indicate what kinds of devices
> those are.  Then, hopefully, if they do everything correctly, and the
> initiator of a hotplug/hot-remove operation prepares the request correctly,
> the callbacks will be executed in the right order, they will find their
> devices in the list attached to the request object and they will do what's
> necessary with them.
> 
> Am I still on the right track?

Yes.

> If that's the case, I have a few questions.

Well, there are more than a few :), but they all are excellent
questions!

> (1) Why is this limited to system devices?

It could be extended to other devices, but is specifically designed for
system devices as follows.  So, I think it is best to keep it in that
way.

a) Work with multiple subsystems without bus dependency.  Other hot-plug
frameworks are designed and implemented for a particular bus and a
subsystem.  Therefore, they work best for their targeted environment as
well.

b) Sequence with pre-defined order.  This allows hot-add operation and
the boot sequence to be consistent.  Other non-system devices are
initialized within a subsystem, and do not depend on the boot-up
sequence.

> (2) What's the guarantee that the ordering of hot-removal (for example) of CPU
> cores with respect to memory and host bridges will always be the same?
> What if the CPU cores themselves need to be hot-removed in a specific
> order?

When devices are added in the order of A->B->C, their dependency model
is:
 - B may depend on A (but A may not depend on B)
 - C may depend on A and B (but A and B may not depend on C)

Therefore, they can be deleted in the order of C->B->A.

The boot sequence defines the order for add.  So, it is important to
make sure that we hot-add devices in the same order with the boot
sequence.  Of course, if there is an issue in the order, we need to fix
it.  But the point is that the add order should be consistent between
the boot sequence and hot-add.

In your example, the boot sequence adds them in the order of
memory->CPU->host bridge.  I think this makes sense because cpu may need
its local memory, and host bridge may need its loca

Re: [PATCH Bug fix 0/5] Bug fix for physical memory hot-remove.

[Toshi Kani]

On Tue, 2013-01-22 at 19:42 +0800, Tang Chen wrote:
> Here are some bug fix patches for physical memory hot-remove. All these
> patches are based on the latest -mm tree.
> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git akpm
> 
> And patch1 and patch3 are very important.
> patch1: free compound pages when freeing memmap, otherwise the kernel
> will panic the next time memory is hot-added.
> patch3: the old way of freeing pagetable pages was wrong. We should never
> split larger pages into small ones.
> 
> 
> Lai Jiangshan (1):
>   Bug-fix: mempolicy: fix is_valid_nodemask()
> 
> Tang Chen (3):
>   Bug fix: Do not split pages when freeing pagetable pages.
>   Bug fix: Fix section mismatch problem of
> release_firmware_map_entry().
>   Bug fix: Fix the doc format in drivers/firmware/memmap.c
> 
> Wen Congyang (1):
>   Bug fix: consider compound pages when free memmap
> 
>  arch/x86/mm/init_64.c |  148 
> ++---
>  drivers/firmware/memmap.c |   16 +++---
>  mm/mempolicy.c|   36 +++
>  mm/sparse.c   |2 +-
>  4 files changed, 77 insertions(+), 125 deletions(-)

This patchset fixed a blocker panic I was hitting in my memory hot-plug
testing.  Memory hotplug works fine with this patchset (for testing my
hotplug framework patchset :).  For the series:

Tested-by: Toshi Kani 

Thanks,
-Toshi



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Tue, 2013-01-29 at 23:48 -0500, Greg KH wrote:
> On Mon, Jan 14, 2013 at 12:02:04PM -0700, Toshi Kani wrote:
> > On Mon, 2013-01-14 at 19:48 +0100, Rafael J. Wysocki wrote:
> > > On Monday, January 14, 2013 08:33:48 AM Toshi Kani wrote:
> > > > On Fri, 2013-01-11 at 22:23 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, January 10, 2013 04:40:19 PM Toshi Kani wrote:
> > > > > > Added include/linux/sys_hotplug.h, which defines the system device
> > > > > > hotplug framework interfaces used by the framework itself and
> > > > > > handlers.
> > > > > > 
> > > > > > The order values define the calling sequence of handlers.  For add
> > > > > > execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
> > > > > > CPU so that threads on new CPUs can start using their local memory.
> > > > > > The ordering of the delete execute is symmetric to the add execute.
> > > > > > 
> > > > > > struct shp_request defines a hot-plug request information.  The
> > > > > > device resource information is managed with a list so that a single
> > > > > > request may target to multiple devices.
> > > > > > 
> > > >  :
> > > > > > +
> > > > > > +struct shp_device {
> > > > > > +   struct list_headlist;
> > > > > > +   struct device   *device;
> > > > > > +   enum shp_class  class;
> > > > > > +   union shp_dev_info  info;
> > > > > > +};
> > > > > > +
> > > > > > +/*
> > > > > > + * Hot-plug request
> > > > > > + */
> > > > > > +struct shp_request {
> > > > > > +   /* common info */
> > > > > > +   enum shp_operation  operation;  /* operation */
> > > > > > +
> > > > > > +   /* hot-plug event info: only valid for hot-plug operations */
> > > > > > +   void*handle;/* FW handle */
> > > > > 
> > > > > What's the role of handle here?
> > > > 
> > > > On ACPI-based platforms, the handle keeps a notified ACPI handle when a
> > > > hot-plug request is made.  ACPI bus handlers, acpi_add_execute() /
> > > > acpi_del_execute(), then scans / trims ACPI devices from the handle.
> > > 
> > > OK, so this is ACPI-specific and should be described as such.
> > 
> > Other FW interface I know is parisc, which has mod_index (module index)
> > to identify a unique object, just like what ACPI handle does.  The
> > handle can keep the mod_index as an opaque value as well.  But as you
> > said, I do not know if the handle works for all other FWs.  So, I will
> > add descriptions, such that the hot-plug event info is modeled after
> > ACPI and may need to be revisited when supporting other FW.
> 
> Please make it a "real" pointer, and not a void *, those shouldn't be
> used at all if possible.

How about changing the "void *handle" to acpi_dev_node below?   

   struct acpi_dev_nodeacpi_node;

Basically, it has the same challenge as struct device, which uses
acpi_dev_node as well.  We can add other FW node when needed (just like
device also has *of_node).

Thanks,
-Toshi



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Tue, 2013-01-29 at 23:51 -0500, Greg KH wrote:
> On Mon, Jan 14, 2013 at 12:21:30PM -0700, Toshi Kani wrote:
> > On Mon, 2013-01-14 at 20:07 +0100, Rafael J. Wysocki wrote:
> > > On Monday, January 14, 2013 11:42:09 AM Toshi Kani wrote:
> > > > On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> > > > > On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > > > > > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > > > > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > > > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > > > > > device hotplug header and defines the order values of 
> > > > > > > > ACPI-specific
> > > > > > > > handlers.
> > > > > > > > 
> > > > > > > > Signed-off-by: Toshi Kani 
> > > > > > > > ---
> > > > > > > >  include/acpi/sys_hotplug.h |   48 
> > > > > > > > 
> > > > > > > >  1 file changed, 48 insertions(+)
> > > > > > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > > > > > 
> > > > > > > > diff --git a/include/acpi/sys_hotplug.h 
> > > > > > > > b/include/acpi/sys_hotplug.h
> > > > > > > > new file mode 100644
> > > > > > > > index 000..ad80f61
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/include/acpi/sys_hotplug.h
> > > > > > > > @@ -0,0 +1,48 @@
> > > > > > > > +/*
> > > > > > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > > > > > + *
> > > > > > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > > > > > + * Toshi Kani 
> > > > > > > > + *
> > > > > > > > + * 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.
> > > > > > > > + */
> > > > > > > > +
> > > > > > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > > > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > > > > > +
> > > > > > > > +#include 
> > > > > > > > +#include 
> > > > > > > > +#include 
> > > > > > > > +
> > > > > > > > +/*
> > > > > > > > + * System device hot-plug operation proceeds in the following 
> > > > > > > > order.
> > > > > > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > > > > > + *
> > > > > > > > + * The order values below define the calling sequence of 
> > > > > > > > ACPI-specific
> > > > > > > > + * handlers for each phase in ascending order.  The order 
> > > > > > > > value of
> > > > > > > > + * platform-neutral handlers are defined in 
> > > > > > > > .
> > > > > > > > + */
> > > > > > > > +
> > > > > > > > +/* Add Validate order values */
> > > > > > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   
> > > > > > > > /* must be first */
> > > > > > > > +
> > > > > > > > +/* Add Execute order values */
> > > > > > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > > > > > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > > > > > > > +
> > > > > > > > +/* Add Commit order values */
> > > > > > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > > > > > > > +
> > > > > > > > +/* Delete Validate order values */
> > > > > > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   
> > > > > > > > /* mus

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Tue, 2013-01-29 at 23:53 -0500, Greg KH wrote:
> On Thu, Jan 10, 2013 at 04:40:19PM -0700, Toshi Kani wrote:
> > Added include/linux/sys_hotplug.h, which defines the system device
> > hotplug framework interfaces used by the framework itself and
> > handlers.
> > 
> > The order values define the calling sequence of handlers.  For add
> > execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
> > CPU so that threads on new CPUs can start using their local memory.
> > The ordering of the delete execute is symmetric to the add execute.
> > 
> > struct shp_request defines a hot-plug request information.  The
> > device resource information is managed with a list so that a single
> > request may target to multiple devices.
> > 
> > Signed-off-by: Toshi Kani 
> > ---
> >  include/linux/sys_hotplug.h |  181 
> > +++
> >  1 file changed, 181 insertions(+)
> >  create mode 100644 include/linux/sys_hotplug.h
> > 
> > diff --git a/include/linux/sys_hotplug.h b/include/linux/sys_hotplug.h
> > new file mode 100644
> > index 000..86674dd
> > --- /dev/null
> > +++ b/include/linux/sys_hotplug.h
> > @@ -0,0 +1,181 @@
> > +/*
> > + * sys_hotplug.h - System device hot-plug framework
> > + *
> > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > + * Toshi Kani 
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef _LINUX_SYS_HOTPLUG_H
> > +#define _LINUX_SYS_HOTPLUG_H
> > +
> > +#include 
> > +#include 
> > +
> > +/*
> > + * System device hot-plug operation proceeds in the following order.
> > + *   Validate phase -> Execute phase -> Commit phase
> > + *
> > + * The order values below define the calling sequence of platform
> > + * neutral handlers for each phase in ascending order.  The order
> > + * values of firmware-specific handlers are defined in sys_hotplug.h
> > + * under firmware specific directories.
> > + */
> > +
> > +/* All order values must be smaller than this value */
> > +#define SHP_ORDER_MAX  0xff
> > +
> > +/* Add Validate order values */
> > +
> > +/* Add Execute order values */
> > +#define SHP_MEM_ADD_EXECUTE_ORDER  100
> > +#define SHP_CPU_ADD_EXECUTE_ORDER  110
> > +
> > +/* Add Commit order values */
> > +
> > +/* Delete Validate order values */
> > +#define SHP_CPU_DEL_VALIDATE_ORDER 100
> > +#define SHP_MEM_DEL_VALIDATE_ORDER 110
> > +
> > +/* Delete Execute order values */
> > +#define SHP_CPU_DEL_EXECUTE_ORDER  10
> > +#define SHP_MEM_DEL_EXECUTE_ORDER  20
> > +
> > +/* Delete Commit order values */
> > +
> 
> Empty value?

Yes, in this version, all the delete commit order values are defined in
.

> Anyway, as I said before, don't use "values", just call things directly
> in the order you need to.
> 
> This isn't like other operating systems, we don't need to be so
> "flexible", we can modify the core code as much as we want and need to
> if future things come along :)

Understood.  As described in the previous email, I will define them with
enum and avoid using values.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 03/12] drivers/base: Add system device hotplug framework

[Toshi Kani]

On Tue, 2013-01-29 at 23:54 -0500, Greg KH wrote:
> On Thu, Jan 10, 2013 at 04:40:21PM -0700, Toshi Kani wrote:
> > Added sys_hotplug.c, which is the system device hotplug framework code.
> > 
> > shp_register_handler() allows modules to register their hotplug handlers
> > to the framework.  shp_submit_req() provides the interface to submit
> > a hotplug or online/offline request of system devices.  The request is
> > then put into hp_workqueue.  shp_start_req() calls all registered handlers
> > in ascending order for each phase.  If any handler failed in validate or
> > execute phase, shp_start_req() initiates its rollback procedure.
> > 
> > Signed-off-by: Toshi Kani 
> > ---
> >  drivers/base/Makefile  |1 
> >  drivers/base/sys_hotplug.c |  313 
> > 
> >  2 files changed, 314 insertions(+)
> >  create mode 100644 drivers/base/sys_hotplug.c
> > 
> > diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> > index 5aa2d70..2e9b2f1 100644
> > --- a/drivers/base/Makefile
> > +++ b/drivers/base/Makefile
> > @@ -21,6 +21,7 @@ endif
> >  obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
> >  obj-$(CONFIG_REGMAP)   += regmap/
> >  obj-$(CONFIG_SOC_BUS) += soc.o
> > +obj-y  += sys_hotplug.o
> 
> No option to select this for systems that don't need it?  If not, then
> put it up higher with all of the other code for the core.

It used to have CONFIG_HOTPLUG, but I removed it as you suggested.  Yes,
I will put it up higher.  

Thanks,
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Tue, 2013-01-29 at 23:58 -0500, Greg KH wrote:
> On Thu, Jan 10, 2013 at 04:40:19PM -0700, Toshi Kani wrote:
> > +/*
> > + * Hot-plug device information
> > + */
> 
> Again, stop it with the "generic" hotplug term here, and everywhere
> else.  You are doing a very _specific_ type of hotplug devices, so spell
> it out.  We've worked hard to hotplug _everything_ in Linux, you are
> going to confuse a lot of people with this type of terms.

Agreed.  I will clarify in all places.

> > +union shp_dev_info {
> > +   struct shp_cpu {
> > +   u32 cpu_id;
> > +   } cpu;
> 
> What is this?  Why not point to the system device for the cpu?

This info is used to on-line a new CPU and create its system/cpu device.
In other word, a system/cpu device is created as a result of CPU
hotplug.

> > +   struct shp_memory {
> > +   int node;
> > +   u64 start_addr;
> > +   u64 length;
> > +   } mem;
> 
> Same here, why not point to the system device?

Same as above.

> > +   struct shp_hostbridge {
> > +   } hb;
> > +
> > +   struct shp_node {
> > +   } node;
> 
> What happened here with these?  Empty structures?  Huh?

They are place holders for now.  PCI bridge hot-plug and node hot-plug
are still very much work in progress, so I have not integrated them into
this framework yet.

> > +};
> > +
> > +struct shp_device {
> > +   struct list_headlist;
> > +   struct device   *device;
> 
> No, make it a "real" device, embed the device into it.

This device pointer is used to send KOBJ_ONLINE/OFFLINE event during CPU
online/offline operation in order to maintain the current behavior.  CPU
online/offline operation only changes the state of CPU, so its
system/cpu device continues to be present before and after an operation.
(Whereas, CPU hot-add/delete operation creates or removes a system/cpu
device.)  So, this "*device" needs to be a pointer to reference an
existing device that is to be on-lined/off-lined.

> But, again, I'm going to ask why you aren't using the existing cpu /
> memory / bridge / node devices that we have in the kernel.  Please use
> them, or give me a _really_ good reason why they will not work.

We cannot use the existing system devices or ACPI devices here.  During
hot-plug, ACPI handler sets this shp_device info, so that cpu and memory
handlers (drivers/cpu.c and mm/memory_hotplug.c) can obtain their target
device information in a platform-neutral way.  During hot-add, we first
creates an ACPI device node (i.e. device under /sys/bus/acpi/devices),
but platform-neutral modules cannot use them as they are ACPI-specific.
Also, its system device (i.e. device under /sys/devices/system) has not
been created until the hot-add operation completes.

> > +   enum shp_class  class;
> > +   union shp_dev_info  info;
> > +};
> > +
> > +/*
> > + * Hot-plug request
> > + */
> > +struct shp_request {
> > +   /* common info */
> > +   enum shp_operation  operation;  /* operation */
> > +
> > +   /* hot-plug event info: only valid for hot-plug operations */
> > +   void*handle;/* FW handle */
> > +   u32 event;  /* FW event */
> 
> What is this?

The shp_request describes a hotplug or online/offline operation that is
requested.  In case of hot-plug request, the "*handle" describes a
target device (which is an ACPI device object) and the "event" describes
a type of request, such as hot-add or hot-delete.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Thu, 2013-01-31 at 05:24 +, Greg KH wrote:
> On Wed, Jan 30, 2013 at 06:15:12PM -0700, Toshi Kani wrote:
> > > Please make it a "real" pointer, and not a void *, those shouldn't be
> > > used at all if possible.
> > 
> > How about changing the "void *handle" to acpi_dev_node below?   
> > 
> >struct acpi_dev_nodeacpi_node;
> > 
> > Basically, it has the same challenge as struct device, which uses
> > acpi_dev_node as well.  We can add other FW node when needed (just like
> > device also has *of_node).
> 
> That sounds good to me.

Great!  Thanks Greg,
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Thu, 2013-01-31 at 21:54 +0100, Rafael J. Wysocki wrote:
> On Wednesday, January 30, 2013 07:57:45 PM Toshi Kani wrote:
> > On Tue, 2013-01-29 at 23:58 -0500, Greg KH wrote:
> > > On Thu, Jan 10, 2013 at 04:40:19PM -0700, Toshi Kani wrote:
 :
> > > > +};
> > > > +
> > > > +struct shp_device {
> > > > +   struct list_headlist;
> > > > +   struct device   *device;
> > > 
> > > No, make it a "real" device, embed the device into it.
> > 
> > This device pointer is used to send KOBJ_ONLINE/OFFLINE event during CPU
> > online/offline operation in order to maintain the current behavior.  CPU
> > online/offline operation only changes the state of CPU, so its
> > system/cpu device continues to be present before and after an operation.
> > (Whereas, CPU hot-add/delete operation creates or removes a system/cpu
> > device.)  So, this "*device" needs to be a pointer to reference an
> > existing device that is to be on-lined/off-lined.
> > 
> > > But, again, I'm going to ask why you aren't using the existing cpu /
> > > memory / bridge / node devices that we have in the kernel.  Please use
> > > them, or give me a _really_ good reason why they will not work.
> > 
> > We cannot use the existing system devices or ACPI devices here.  During
> > hot-plug, ACPI handler sets this shp_device info, so that cpu and memory
> > handlers (drivers/cpu.c and mm/memory_hotplug.c) can obtain their target
> > device information in a platform-neutral way.  During hot-add, we first
> > creates an ACPI device node (i.e. device under /sys/bus/acpi/devices),
> > but platform-neutral modules cannot use them as they are ACPI-specific.
> 
> But suppose we're smart and have ACPI scan handlers that will create
> "physical" device nodes for those devices during the ACPI namespace scan.
> Then, the platform-neutral nodes will be able to bind to those "physical"
> nodes.  Moreover, it should be possible to get a hierarchy of device objects
> this way that will reflect all of the dependencies we need to take into
> account during hot-add and hot-remove operations.  That may not be what we
> have today, but I don't see any *fundamental* obstacles preventing us from
> using this approach.

I misstated in my previous email.  system/cpu device is actually created
by ACPI driver during ACPI scan in case of hot-add.  This is done by 
acpi_processor_hotadd_init(), which I consider as a hack but can be
done.  system/memory device is created in add_memory() by the mm module.

> This is already done for PCI host bridges and platform devices and I don't
> see why we can't do that for the other types of devices too.
> 
> The only missing piece I see is a way to handle the "eject" problem, i.e.
> when we try do eject a device at the top of a subtree and need to tear down
> the entire subtree below it, but if that's going to lead to a system crash,
> for example, we want to cancel the eject.  It seems to me that we'll need some
> help from the driver core here.

There are three different approaches suggested for system device
hot-plug:
 A. Proceed within system device bus scan.
 B. Proceed within ACPI bus scan.
 C. Proceed with a sequence (as a mini-boot).

Option A uses system devices as tokens, option B uses acpi devices as
tokens, and option C uses resource tables as tokens, for their handlers.

Here is summary of key questions & answers so far.  I hope this
clarifies why I am suggesting option 3.

1. What are the system devices?
System devices provide system-wide core computing resources, which are
essential to compose a computer system.  System devices are not
connected to any particular standard buses.

2. Why are the system devices special?
The system devices are initialized during early boot-time, by multiple
subsystems, from the boot-up sequence, in pre-defined order.  They
provide low-level services to enable other subsystems to come up.

3. Why can't initialize the system devices from the driver structure at
boot?
The driver structure is initialized at the end of the boot sequence and
requires the low-level services from the system devices initialized
beforehand.

4. Why do we need a new common framework?
Sysfs CPU and memory on-lining/off-lining are performed within the CPU
and memory modules.  They are common code and do not depend on ACPI.
Therefore, a new common framework is necessary to integrate both
on-lining/off-lining operation and hot-plugging operation of system
devices into a single framework.

5. Why can't do everything with ACPI bus scan?
Software dependency among system devices may not be dictated by the ACPI
hierarchy.  For instance, memory s

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Fri, 2013-02-01 at 07:30 +, Greg KH wrote:
> On Thu, Jan 31, 2013 at 06:32:18PM -0700, Toshi Kani wrote:
>  > This is already done for PCI host bridges and platform devices and I don't
> > > see why we can't do that for the other types of devices too.
> > > 
> > > The only missing piece I see is a way to handle the "eject" problem, i.e.
> > > when we try do eject a device at the top of a subtree and need to tear 
> > > down
> > > the entire subtree below it, but if that's going to lead to a system 
> > > crash,
> > > for example, we want to cancel the eject.  It seems to me that we'll need 
> > > some
> > > help from the driver core here.
> > 
> > There are three different approaches suggested for system device
> > hot-plug:
> >  A. Proceed within system device bus scan.
> >  B. Proceed within ACPI bus scan.
> >  C. Proceed with a sequence (as a mini-boot).
> > 
> > Option A uses system devices as tokens, option B uses acpi devices as
> > tokens, and option C uses resource tables as tokens, for their handlers.
> > 
> > Here is summary of key questions & answers so far.  I hope this
> > clarifies why I am suggesting option 3.
> > 
> > 1. What are the system devices?
> > System devices provide system-wide core computing resources, which are
> > essential to compose a computer system.  System devices are not
> > connected to any particular standard buses.
> 
> Not a problem, lots of devices are not connected to any "particular
> standard busses".  All this means is that system devices are connected
> to the "system" bus, nothing more.

Can you give me a few examples of other devices that support hotplug and
are not connected to any particular buses?  I will investigate them to
see how they are managed to support hotplug.

> > 2. Why are the system devices special?
> > The system devices are initialized during early boot-time, by multiple
> > subsystems, from the boot-up sequence, in pre-defined order.  They
> > provide low-level services to enable other subsystems to come up.
> 
> Sorry, no, that doesn't mean they are special, nothing here is unique
> for the point of view of the driver model from any other device or bus.

I think system devices are unique in a sense that they are initialized
before drivers run.

> > 3. Why can't initialize the system devices from the driver structure at
> > boot?
> > The driver structure is initialized at the end of the boot sequence and
> > requires the low-level services from the system devices initialized
> > beforehand.
> 
> Wait, what "driver structure"?  

Sorry it was not clear.  cpu_dev_init() and memory_dev_init() are called
from driver_init() at the end of the boot sequence, and initialize
system/cpu and system/memory devices.  I assume they are the system bus
you are referring with option A.

> If you need to initialize the driver
> core earlier, then do so.  Or, even better, just wait until enough of
> the system has come up and then go initialize all of the devices you
> have found so far as part of your boot process.

They are pseudo drivers that provide sysfs entry points of cpu and
memory.  They do not actually initialize cpu and memory.  I do not think
initializing cpu and memory fits into the driver model either, since
drivers should run after cpu and memory are initialized.

> None of the above things you have stated seem to have anything to do
> with your proposed patch, so I don't understand why you have mentioned
> them...

You suggested option A before, which uses system bus scan to initialize
all system devices at boot time as well as hot-plug.  I tried to say
that this option would not be doable.

> > 4. Why do we need a new common framework?
> > Sysfs CPU and memory on-lining/off-lining are performed within the CPU
> > and memory modules.  They are common code and do not depend on ACPI.
> > Therefore, a new common framework is necessary to integrate both
> > on-lining/off-lining operation and hot-plugging operation of system
> > devices into a single framework.
> 
> {sigh}
> 
> Removing and adding devices and handling hotplug operations is what the
> driver core was written for, almost 10 years ago.  To somehow think that
> your devices are "special" just because they don't use ACPI is odd,
> because the driver core itself has nothing to do with ACPI.  Don't get
> the current mix of x86 system code tied into ACPI confused with an
> driver core issues here please.

CPU online/offline operation is performed within the CPU module.  Memory
online/offline operation is performed within the 

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Fri, 2013-02-01 at 23:21 +0100, Rafael J. Wysocki wrote:
> On Friday, February 01, 2013 01:40:10 PM Toshi Kani wrote:
> > On Fri, 2013-02-01 at 07:30 +, Greg KH wrote:
> > > On Thu, Jan 31, 2013 at 06:32:18PM -0700, Toshi Kani wrote:
> > >  > This is already done for PCI host bridges and platform devices and I 
> > > don't
> > > > > see why we can't do that for the other types of devices too.
> > > > > 
> > > > > The only missing piece I see is a way to handle the "eject" problem, 
> > > > > i.e.
> > > > > when we try do eject a device at the top of a subtree and need to 
> > > > > tear down
> > > > > the entire subtree below it, but if that's going to lead to a system 
> > > > > crash,
> > > > > for example, we want to cancel the eject.  It seems to me that we'll 
> > > > > need some
> > > > > help from the driver core here.
> > > > 
> > > > There are three different approaches suggested for system device
> > > > hot-plug:
> > > >  A. Proceed within system device bus scan.
> > > >  B. Proceed within ACPI bus scan.
> > > >  C. Proceed with a sequence (as a mini-boot).
> > > > 
> > > > Option A uses system devices as tokens, option B uses acpi devices as
> > > > tokens, and option C uses resource tables as tokens, for their handlers.
> > > > 
> > > > Here is summary of key questions & answers so far.  I hope this
> > > > clarifies why I am suggesting option 3.
> > > > 
> > > > 1. What are the system devices?
> > > > System devices provide system-wide core computing resources, which are
> > > > essential to compose a computer system.  System devices are not
> > > > connected to any particular standard buses.
> > > 
> > > Not a problem, lots of devices are not connected to any "particular
> > > standard busses".  All this means is that system devices are connected
> > > to the "system" bus, nothing more.
> > 
> > Can you give me a few examples of other devices that support hotplug and
> > are not connected to any particular buses?  I will investigate them to
> > see how they are managed to support hotplug.
> > 
> > > > 2. Why are the system devices special?
> > > > The system devices are initialized during early boot-time, by multiple
> > > > subsystems, from the boot-up sequence, in pre-defined order.  They
> > > > provide low-level services to enable other subsystems to come up.
> > > 
> > > Sorry, no, that doesn't mean they are special, nothing here is unique
> > > for the point of view of the driver model from any other device or bus.
> > 
> > I think system devices are unique in a sense that they are initialized
> > before drivers run.
> > 
> > > > 3. Why can't initialize the system devices from the driver structure at
> > > > boot?
> > > > The driver structure is initialized at the end of the boot sequence and
> > > > requires the low-level services from the system devices initialized
> > > > beforehand.
> > > 
> > > Wait, what "driver structure"?  
> > 
> > Sorry it was not clear.  cpu_dev_init() and memory_dev_init() are called
> > from driver_init() at the end of the boot sequence, and initialize
> > system/cpu and system/memory devices.  I assume they are the system bus
> > you are referring with option A.
> > 
> > > If you need to initialize the driver
> > > core earlier, then do so.  Or, even better, just wait until enough of
> > > the system has come up and then go initialize all of the devices you
> > > have found so far as part of your boot process.
> > 
> > They are pseudo drivers that provide sysfs entry points of cpu and
> > memory.  They do not actually initialize cpu and memory.  I do not think
> > initializing cpu and memory fits into the driver model either, since
> > drivers should run after cpu and memory are initialized.
> > 
> > > None of the above things you have stated seem to have anything to do
> > > with your proposed patch, so I don't understand why you have mentioned
> > > them...
> > 
> > You suggested option A before, which uses system bus scan to initialize
> > all system devices at boot time as well as hot-plug.  I tried to say
> > that this option would not be doable.
> > 
> > &g

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Sat, 2013-02-02 at 16:01 +0100, Greg KH wrote:
> On Fri, Feb 01, 2013 at 01:40:10PM -0700, Toshi Kani wrote:
> > On Fri, 2013-02-01 at 07:30 +, Greg KH wrote:
> > > On Thu, Jan 31, 2013 at 06:32:18PM -0700, Toshi Kani wrote:
> > >  > This is already done for PCI host bridges and platform devices and I 
> > > don't
> > > > > see why we can't do that for the other types of devices too.
> > > > > 
> > > > > The only missing piece I see is a way to handle the "eject" problem, 
> > > > > i.e.
> > > > > when we try do eject a device at the top of a subtree and need to 
> > > > > tear down
> > > > > the entire subtree below it, but if that's going to lead to a system 
> > > > > crash,
> > > > > for example, we want to cancel the eject.  It seems to me that we'll 
> > > > > need some
> > > > > help from the driver core here.
> > > > 
> > > > There are three different approaches suggested for system device
> > > > hot-plug:
> > > >  A. Proceed within system device bus scan.
> > > >  B. Proceed within ACPI bus scan.
> > > >  C. Proceed with a sequence (as a mini-boot).
> > > > 
> > > > Option A uses system devices as tokens, option B uses acpi devices as
> > > > tokens, and option C uses resource tables as tokens, for their handlers.
> > > > 
> > > > Here is summary of key questions & answers so far.  I hope this
> > > > clarifies why I am suggesting option 3.
> > > > 
> > > > 1. What are the system devices?
> > > > System devices provide system-wide core computing resources, which are
> > > > essential to compose a computer system.  System devices are not
> > > > connected to any particular standard buses.
> > > 
> > > Not a problem, lots of devices are not connected to any "particular
> > > standard busses".  All this means is that system devices are connected
> > > to the "system" bus, nothing more.
> > 
> > Can you give me a few examples of other devices that support hotplug and
> > are not connected to any particular buses?  I will investigate them to
> > see how they are managed to support hotplug.
> 
> Any device that is attached to any bus in the driver model can be
> hotunplugged from userspace by telling it to be "unbound" from the
> driver controlling it.  Try it for any platform device in your system to
> see how it happens.

The unbind operation, as I understand from you, is to detach a driver
from a device.  Yes, unbinding can be done for any devices.  It is
however different from hot-plug operation, which unplugs a device.

Today, the unbind operation to an ACPI cpu/memory devices causes
hot-unplug (offline) operation to them, which is one of the major issues
for us since unbind cannot fail.  This patchset addresses this issue by
making the unbind operation of ACPI cpu/memory devices to do the
unbinding only.  ACPI drivers no longer control cpu and memory as they
are supposed to be controlled by their drivers, cpu and memory modules.
The current hotplug code requires putting all device control stuff into
ACPI, which this patchset is trying to fix it.


> > > > 2. Why are the system devices special?
> > > > The system devices are initialized during early boot-time, by multiple
> > > > subsystems, from the boot-up sequence, in pre-defined order.  They
> > > > provide low-level services to enable other subsystems to come up.
> > > 
> > > Sorry, no, that doesn't mean they are special, nothing here is unique
> > > for the point of view of the driver model from any other device or bus.
> > 
> > I think system devices are unique in a sense that they are initialized
> > before drivers run.
> 
> No, most all devices are "initialized" before a driver runs on it, USB
> is one such example, PCI another, and I'm pretty sure that there are
> others.

USB devices can be initialized after the USB bus driver is initialized.
Similarly, PCI devices can be initialized after the PCI bus driver is
initialized.  However, CPU and memory are initialized without any
dependency to their bus driver since there is no such thing.

In addition, CPU and memory have two drivers -- their actual
drivers/subsystems and their ACPI drivers.  Their actual
drivers/subsystems initialize cpu and memory.  The ACPI drivers
initialize driver-specific data of their ACPI nodes.  During hot-plug
operation, however, the current code requires these ACPI drivers to do
their hot-plug

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-02-04 at 14:41 +0100, Rafael J. Wysocki wrote:
> On Sunday, February 03, 2013 07:23:49 PM Greg KH wrote:
> > On Sat, Feb 02, 2013 at 09:15:37PM +0100, Rafael J. Wysocki wrote:
> > > On Saturday, February 02, 2013 03:58:01 PM Greg KH wrote:
  :
> > > Yes, but those are just remove events and we can only see how destructive 
> > > they
> > > were after the removal.  The point is to be able to figure out whether or 
> > > not
> > > we *want* to do the removal in the first place.
> > 
> > Yes, but, you will always race if you try to test to see if you can shut
> > down a device and then trying to do it.  So walking the bus ahead of
> > time isn't a good idea.
> >
> > And, we really don't have a viable way to recover if disconnect() fails,
> > do we.  What do we do in that situation, restore the other devices we
> > disconnected successfully?  How do we remember/know what they were?
> > 
> > PCI hotplug almost had this same problem until the designers finally
> > realized that they just had to accept the fact that removing a PCI
> > device could either happen by:
> > - a user yanking out the device, at which time the OS better
> >   clean up properly no matter what happens
> > - the user asked nicely to remove a device, and the OS can take
> >   as long as it wants to complete that action, including
> >   stalling for noticable amounts of time before eventually,
> >   always letting the action succeed.
> > 
> > I think the second thing is what you have to do here.  If a user tells
> > the OS it wants to remove these devices, you better do it.  If you
> > can't, because memory is being used by someone else, either move them
> > off, or just hope that nothing bad happens, before the user gets
> > frustrated and yanks out the CPU/memory module themselves physically :)
> 
> Well, that we can't help, but sometimes users really *want* the OS to tell 
> them
> if it is safe to unplug something at this particualr time (think about the
> Windows' "safe remove" feature for USB sticks, for example; that came out of
> users' demand AFAIR).
> 
> So in my opinion it would be good to give them an option to do "safe eject" or
> "forcible eject", whichever they prefer.

For system device hot-plug, it always needs to be "safe eject".  This
feature will be implemented on mission critical servers, which are
managed by professional IT folks.  Crashing a server causes serious
money to the business.

A user yanking out a system device won't happen, and it immediately
crashes the system if it is done.  So, we have nothing to do with this
case.  The 2nd case can hang the operation, waiting forever to proceed,
which is still a serious issue for enterprise customers.


> > > Say you have a computing node which signals a hardware problem in a 
> > > processor
> > > package (the container with CPU cores, memory, PCI host bridge etc.).  You
> > > may want to eject that package, but you don't want to kill the system this
> > > way.  So if the eject is doable, it is very much desirable to do it, but 
> > > if it
> > > is not doable, you'd rather shut the box down and do the replacement 
> > > afterward.
> > > That may be costly, however (maybe weeks of computations), so it should be
> > > avoided if possible, but not at the expense of crashing the box if the 
> > > eject
> > > doesn't work out.
> > 
> > These same "situations" came up for PCI hotplug, and I still say the
> > same resolution there holds true, as described above.  The user wants to
> > remove something, so let them do it.  They always know best, and get mad
> > at us if we think otherwise :)
> 
> Well, not necessarily.  Users sometimes really don't know what they are doing
> and want us to give them a hint.  My opinion is that if we can give them a
> hint, there's no reason not to.
> 
> > What does the ACPI spec say about this type of thing?  Surely the same
> > people that did the PCI Hotplug spec were consulted when doing this part
> > of the spec, right?  Yeah, I know, I can dream...
> 
> It's not very specific (as usual), but it gives hints. :-)
> 
> For example, there is the _OST method (Section 6.3.5 of ACPI 5) that we are
> supposed to use to notify the platform of ejection failures and there are
> status codes like "0x81: Device in use by application" or "0x82: Device busy"
> that can be used in there.  So definitely the authors took ejection failures
> for software-related reasons into consideration.

That is correct.  Also, ACPI spec deliberately does not define
implementation details, so we defined DIG64 hotplug spec below (which I
contributed to the spec.)

http://www.dig64.org/home/DIG64_HPPF_R1_0.pdf

For example, Figure 2 in page 14 states memory hot-remove flow.  The
operation needs to either succeed or fail.  Crash or hang is not an
option.


Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-02-04 at 15:21 +0100, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 04:48:10 AM Greg KH wrote:
> > On Sun, Feb 03, 2013 at 09:44:39PM +0100, Rafael J. Wysocki wrote:
> > > > Yes, but those are just remove events and we can only see how 
> > > > destructive they
> > > > were after the removal.  The point is to be able to figure out whether 
> > > > or not
> > > > we *want* to do the removal in the first place.
> > > > 
> > > > Say you have a computing node which signals a hardware problem in a 
> > > > processor
> > > > package (the container with CPU cores, memory, PCI host bridge etc.).  
> > > > You
> > > > may want to eject that package, but you don't want to kill the system 
> > > > this
> > > > way.  So if the eject is doable, it is very much desirable to do it, 
> > > > but if it
> > > > is not doable, you'd rather shut the box down and do the replacement 
> > > > afterward.
> > > > That may be costly, however (maybe weeks of computations), so it should 
> > > > be
> > > > avoided if possible, but not at the expense of crashing the box if the 
> > > > eject
> > > > doesn't work out.
> > > 
> > > It seems to me that we could handle that with the help of a new flag, say
> > > "no_eject", in struct device, a global mutex, and a function that will 
> > > walk
> > > the given subtree of the device hierarchy and check if "no_eject" is set 
> > > for
> > > any devices in there.  Plus a global "no_eject" switch, perhaps.
> > 
> > I think this will always be racy, or at worst, slow things down on
> > normal device operations as you will always be having to grab this flag
> > whenever you want to do something new.
> 
> I don't see why this particular scheme should be racy, at least I don't see 
> any
> obvious races in it (although I'm not that good at races detection in general,
> admittedly).
> 
> Also, I don't expect that flag to be used for everything, just for things 
> known
> to seriously break if forcible eject is done.  That may be not precise enough,
> so that's a matter of defining its purpose more precisely.
> 
> We can do something like that on the ACPI level (ie. introduce a no_eject flag
> in struct acpi_device and provide an iterface for the layers above ACPI to
> manipulate it) but then devices without ACPI namespace objects won't be
> covered.  That may not be a big deal, though.

I am afraid that bringing the device status management into the ACPI
level would not a good idea.  acpi_device should only reflect ACPI
device object information, not how its actual device is being used.

I like your initiative of acpi_scan_driver and I think scanning /
trimming of ACPI object info is what the ACPI drivers should do.


Thanks,
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-02-04 at 04:46 -0800, Greg KH wrote:
> On Sun, Feb 03, 2013 at 05:28:09PM -0700, Toshi Kani wrote:
> > On Sat, 2013-02-02 at 16:01 +0100, Greg KH wrote:
> > > On Fri, Feb 01, 2013 at 01:40:10PM -0700, Toshi Kani wrote:
> > > > On Fri, 2013-02-01 at 07:30 +, Greg KH wrote:
> > > > > On Thu, Jan 31, 2013 at 06:32:18PM -0700, Toshi Kani wrote:
> > > > >  > This is already done for PCI host bridges and platform devices and 
> > > > > I don't
> > > > > > > see why we can't do that for the other types of devices too.
> > > > > > > 
> > > > > > > The only missing piece I see is a way to handle the "eject" 
> > > > > > > problem, i.e.
> > > > > > > when we try do eject a device at the top of a subtree and need to 
> > > > > > > tear down
> > > > > > > the entire subtree below it, but if that's going to lead to a 
> > > > > > > system crash,
> > > > > > > for example, we want to cancel the eject.  It seems to me that 
> > > > > > > we'll need some
> > > > > > > help from the driver core here.
> > > > > > 
> > > > > > There are three different approaches suggested for system device
> > > > > > hot-plug:
> > > > > >  A. Proceed within system device bus scan.
> > > > > >  B. Proceed within ACPI bus scan.
> > > > > >  C. Proceed with a sequence (as a mini-boot).
> > > > > > 
> > > > > > Option A uses system devices as tokens, option B uses acpi devices 
> > > > > > as
> > > > > > tokens, and option C uses resource tables as tokens, for their 
> > > > > > handlers.
> > > > > > 
> > > > > > Here is summary of key questions & answers so far.  I hope this
> > > > > > clarifies why I am suggesting option 3.
> > > > > > 
> > > > > > 1. What are the system devices?
> > > > > > System devices provide system-wide core computing resources, which 
> > > > > > are
> > > > > > essential to compose a computer system.  System devices are not
> > > > > > connected to any particular standard buses.
> > > > > 
> > > > > Not a problem, lots of devices are not connected to any "particular
> > > > > standard busses".  All this means is that system devices are connected
> > > > > to the "system" bus, nothing more.
> > > > 
> > > > Can you give me a few examples of other devices that support hotplug and
> > > > are not connected to any particular buses?  I will investigate them to
> > > > see how they are managed to support hotplug.
> > > 
> > > Any device that is attached to any bus in the driver model can be
> > > hotunplugged from userspace by telling it to be "unbound" from the
> > > driver controlling it.  Try it for any platform device in your system to
> > > see how it happens.
> > 
> > The unbind operation, as I understand from you, is to detach a driver
> > from a device.  Yes, unbinding can be done for any devices.  It is
> > however different from hot-plug operation, which unplugs a device.
> 
> Physically, yes, but to the driver involved, and the driver core, there
> is no difference.  That was one of the primary goals of the driver core
> creation so many years ago.
> 
> > Today, the unbind operation to an ACPI cpu/memory devices causes
> > hot-unplug (offline) operation to them, which is one of the major issues
> > for us since unbind cannot fail.  This patchset addresses this issue by
> > making the unbind operation of ACPI cpu/memory devices to do the
> > unbinding only.  ACPI drivers no longer control cpu and memory as they
> > are supposed to be controlled by their drivers, cpu and memory modules.
> 
> I think that's the problem right there, solve that, please.

We cannot eliminate the ACPI drivers since we have to scan ACPI.  But we
can limit the ACPI drivers to do the scanning stuff only.   This is
precisely the intend of this patchset.  The real stuff, removing actual
devices, is done by the system device drivers/modules.


> > > > > > 2. Why are the system devices special?
> > > > > > The system devices are initialized during early boot-time, by 
> > > > > > multiple
> > > > > 

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-02-04 at 20:48 +0100, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 09:02:46 AM Toshi Kani wrote:
> > On Mon, 2013-02-04 at 14:41 +0100, Rafael J. Wysocki wrote:
> > > On Sunday, February 03, 2013 07:23:49 PM Greg KH wrote:
> > > > On Sat, Feb 02, 2013 at 09:15:37PM +0100, Rafael J. Wysocki wrote:
> > > > > On Saturday, February 02, 2013 03:58:01 PM Greg KH wrote:
> >   :
> > > > > Yes, but those are just remove events and we can only see how 
> > > > > destructive they
> > > > > were after the removal.  The point is to be able to figure out 
> > > > > whether or not
> > > > > we *want* to do the removal in the first place.
> > > > 
> > > > Yes, but, you will always race if you try to test to see if you can shut
> > > > down a device and then trying to do it.  So walking the bus ahead of
> > > > time isn't a good idea.
> > > >
> > > > And, we really don't have a viable way to recover if disconnect() fails,
> > > > do we.  What do we do in that situation, restore the other devices we
> > > > disconnected successfully?  How do we remember/know what they were?
> > > > 
> > > > PCI hotplug almost had this same problem until the designers finally
> > > > realized that they just had to accept the fact that removing a PCI
> > > > device could either happen by:
> > > > - a user yanking out the device, at which time the OS better
> > > >   clean up properly no matter what happens
> > > > - the user asked nicely to remove a device, and the OS can take
> > > >   as long as it wants to complete that action, including
> > > >   stalling for noticable amounts of time before eventually,
> > > >   always letting the action succeed.
> > > > 
> > > > I think the second thing is what you have to do here.  If a user tells
> > > > the OS it wants to remove these devices, you better do it.  If you
> > > > can't, because memory is being used by someone else, either move them
> > > > off, or just hope that nothing bad happens, before the user gets
> > > > frustrated and yanks out the CPU/memory module themselves physically :)
> > > 
> > > Well, that we can't help, but sometimes users really *want* the OS to 
> > > tell them
> > > if it is safe to unplug something at this particualr time (think about the
> > > Windows' "safe remove" feature for USB sticks, for example; that came out 
> > > of
> > > users' demand AFAIR).
> > > 
> > > So in my opinion it would be good to give them an option to do "safe 
> > > eject" or
> > > "forcible eject", whichever they prefer.
> > 
> > For system device hot-plug, it always needs to be "safe eject".  This
> > feature will be implemented on mission critical servers, which are
> > managed by professional IT folks.  Crashing a server causes serious
> > money to the business.
> 
> Well, "always" is a bit too strong a word as far as human behavior is 
> concerned
> in my opinion.
> 
> That said I would be perfectly fine with not supporting the "forcible eject" 
> to
> start with and waiting for the first request to add support for it.  I also
> would be fine with taking bets on how much time it's going to take for such a
> request to appear. :-)

Sounds good.  In my experience, though, it actually takes a LONG time to
convince customers that "safe eject" is actually safe.  Enterprise
customers are so afraid of doing anything risky that might cause the
system to crash or hang due to some defect.  I would be very surprised
to see a customer asking for a force operation when we do not guarantee
its outcome.  I have not seen such enterprise customers yet.

Thanks,
-Toshi 


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-02-04 at 21:12 +0100, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 12:46:24 PM Toshi Kani wrote:
> > On Mon, 2013-02-04 at 20:48 +0100, Rafael J. Wysocki wrote:
> > > On Monday, February 04, 2013 09:02:46 AM Toshi Kani wrote:
> > > > On Mon, 2013-02-04 at 14:41 +0100, Rafael J. Wysocki wrote:
> > > > > On Sunday, February 03, 2013 07:23:49 PM Greg KH wrote:
> > > > > > On Sat, Feb 02, 2013 at 09:15:37PM +0100, Rafael J. Wysocki wrote:
> > > > > > > On Saturday, February 02, 2013 03:58:01 PM Greg KH wrote:
> > > >   :
> > > > > > > Yes, but those are just remove events and we can only see how 
> > > > > > > destructive they
> > > > > > > were after the removal.  The point is to be able to figure out 
> > > > > > > whether or not
> > > > > > > we *want* to do the removal in the first place.
> > > > > > 
> > > > > > Yes, but, you will always race if you try to test to see if you can 
> > > > > > shut
> > > > > > down a device and then trying to do it.  So walking the bus ahead of
> > > > > > time isn't a good idea.
> > > > > >
> > > > > > And, we really don't have a viable way to recover if disconnect() 
> > > > > > fails,
> > > > > > do we.  What do we do in that situation, restore the other devices 
> > > > > > we
> > > > > > disconnected successfully?  How do we remember/know what they were?
> > > > > > 
> > > > > > PCI hotplug almost had this same problem until the designers finally
> > > > > > realized that they just had to accept the fact that removing a PCI
> > > > > > device could either happen by:
> > > > > > - a user yanking out the device, at which time the OS better
> > > > > >   clean up properly no matter what happens
> > > > > > - the user asked nicely to remove a device, and the OS can take
> > > > > >   as long as it wants to complete that action, including
> > > > > >   stalling for noticable amounts of time before eventually,
> > > > > >   always letting the action succeed.
> > > > > > 
> > > > > > I think the second thing is what you have to do here.  If a user 
> > > > > > tells
> > > > > > the OS it wants to remove these devices, you better do it.  If you
> > > > > > can't, because memory is being used by someone else, either move 
> > > > > > them
> > > > > > off, or just hope that nothing bad happens, before the user gets
> > > > > > frustrated and yanks out the CPU/memory module themselves 
> > > > > > physically :)
> > > > > 
> > > > > Well, that we can't help, but sometimes users really *want* the OS to 
> > > > > tell them
> > > > > if it is safe to unplug something at this particualr time (think 
> > > > > about the
> > > > > Windows' "safe remove" feature for USB sticks, for example; that came 
> > > > > out of
> > > > > users' demand AFAIR).
> > > > > 
> > > > > So in my opinion it would be good to give them an option to do "safe 
> > > > > eject" or
> > > > > "forcible eject", whichever they prefer.
> > > > 
> > > > For system device hot-plug, it always needs to be "safe eject".  This
> > > > feature will be implemented on mission critical servers, which are
> > > > managed by professional IT folks.  Crashing a server causes serious
> > > > money to the business.
> > > 
> > > Well, "always" is a bit too strong a word as far as human behavior is 
> > > concerned
> > > in my opinion.
> > > 
> > > That said I would be perfectly fine with not supporting the "forcible 
> > > eject" to
> > > start with and waiting for the first request to add support for it.  I 
> > > also
> > > would be fine with taking bets on how much time it's going to take for 
> > > such a
> > > request to appear. :-)
> > 
> > Sounds good.  In my experience, though, it actually takes a LONG time to
> > convince customers that "safe eject" is actually safe.  Enterprise
> > customers are so afraid of doing anything risky that might cause the
> > system to crash or hang due to some defect.  I would be very surprised
> > to see a customer asking for a force operation when we do not guarantee
> > its outcome.  I have not seen such enterprise customers yet.
> 
> But we're talking about a kernel that is supposed to run on mobile phones too,
> among other things.

I think using this feature for RAS i.e. replacing a faulty device
on-line, will continue to be limited for high-end systems.  For low-end
systems, it does not make sense for customers to pay much $$ for this
feature.  They can just shut the system down for replacement, or they
can simply buy a new system instead of repairing.

That said, using this feature on VM for workload balancing does not
require any special hardware.  So, I can see someone willing to try out
to see how it goes with a force option on VM for personal use.   

Thanks,
-Toshi


 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-02-04 at 20:45 +0100, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 09:46:18 AM Toshi Kani wrote:
> > On Mon, 2013-02-04 at 04:46 -0800, Greg KH wrote:
> > > On Sun, Feb 03, 2013 at 05:28:09PM -0700, Toshi Kani wrote:
> > > > On Sat, 2013-02-02 at 16:01 +0100, Greg KH wrote:
> > > > > On Fri, Feb 01, 2013 at 01:40:10PM -0700, Toshi Kani wrote:
> > > > > > On Fri, 2013-02-01 at 07:30 +, Greg KH wrote:
> > > > > > > On Thu, Jan 31, 2013 at 06:32:18PM -0700, Toshi Kani wrote:
> > > > > > >  > This is already done for PCI host bridges and platform devices 
> > > > > > > and I don't
> > > > > > > > > see why we can't do that for the other types of devices too.
> > > > > > > > > 
> > > > > > > > > The only missing piece I see is a way to handle the "eject" 
> > > > > > > > > problem, i.e.
> > > > > > > > > when we try do eject a device at the top of a subtree and 
> > > > > > > > > need to tear down
> > > > > > > > > the entire subtree below it, but if that's going to lead to a 
> > > > > > > > > system crash,
> > > > > > > > > for example, we want to cancel the eject.  It seems to me 
> > > > > > > > > that we'll need some
> > > > > > > > > help from the driver core here.
> > > > > > > > 
> > > > > > > > There are three different approaches suggested for system device
> > > > > > > > hot-plug:
> > > > > > > >  A. Proceed within system device bus scan.
> > > > > > > >  B. Proceed within ACPI bus scan.
> > > > > > > >  C. Proceed with a sequence (as a mini-boot).
> > > > > > > > 
> > > > > > > > Option A uses system devices as tokens, option B uses acpi 
> > > > > > > > devices as
> > > > > > > > tokens, and option C uses resource tables as tokens, for their 
> > > > > > > > handlers.
> > > > > > > > 
> > > > > > > > Here is summary of key questions & answers so far.  I hope this
> > > > > > > > clarifies why I am suggesting option 3.
> > > > > > > > 
> > > > > > > > 1. What are the system devices?
> > > > > > > > System devices provide system-wide core computing resources, 
> > > > > > > > which are
> > > > > > > > essential to compose a computer system.  System devices are not
> > > > > > > > connected to any particular standard buses.
> > > > > > > 
> > > > > > > Not a problem, lots of devices are not connected to any 
> > > > > > > "particular
> > > > > > > standard busses".  All this means is that system devices are 
> > > > > > > connected
> > > > > > > to the "system" bus, nothing more.
> > > > > > 
> > > > > > Can you give me a few examples of other devices that support 
> > > > > > hotplug and
> > > > > > are not connected to any particular buses?  I will investigate them 
> > > > > > to
> > > > > > see how they are managed to support hotplug.
> > > > > 
> > > > > Any device that is attached to any bus in the driver model can be
> > > > > hotunplugged from userspace by telling it to be "unbound" from the
> > > > > driver controlling it.  Try it for any platform device in your system 
> > > > > to
> > > > > see how it happens.
> > > > 
> > > > The unbind operation, as I understand from you, is to detach a driver
> > > > from a device.  Yes, unbinding can be done for any devices.  It is
> > > > however different from hot-plug operation, which unplugs a device.
> > > 
> > > Physically, yes, but to the driver involved, and the driver core, there
> > > is no difference.  That was one of the primary goals of the driver core
> > > creation so many years ago.
> > > 
> > > > Today, the unbind operation to an ACPI cpu/memory devices causes
> > > > hot-unplug (offline) operation to them, which is one of the major issues
> > > > for us since unbind cannot fail.  This patchset addresses this issue by
> > > > making the unbind operation of ACPI cpu/memory devices to do the
> > > > unbinding only.  ACPI drivers no longer control cpu and memory as they
> > > > are supposed to be controlled by their drivers, cpu and memory modules.
> > > 
> > > I think that's the problem right there, solve that, please.
> > 
> > We cannot eliminate the ACPI drivers since we have to scan ACPI.  But we
> > can limit the ACPI drivers to do the scanning stuff only.   This is
> > precisely the intend of this patchset.  The real stuff, removing actual
> > devices, is done by the system device drivers/modules.
> 
> In case you haven't realized that yet, the $subject patchset has no future.

That's really disappointing, esp. the fact that this basic approach has
been proven to work on other OS for years...


> Let's just talk about how we can get what we need in more general terms.

So, are we heading to an approach of doing everything in ACPI?  I am not
clear about which direction we have agreed with or disagreed with.

As for the eject flag approach, I agree with Greg.


Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Mon, 2013-02-04 at 21:07 +0100, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 06:33:52 AM Greg KH wrote:
> > On Mon, Feb 04, 2013 at 03:21:22PM +0100, Rafael J. Wysocki wrote:
> > > On Monday, February 04, 2013 04:48:10 AM Greg KH wrote:
> > > > On Sun, Feb 03, 2013 at 09:44:39PM +0100, Rafael J. Wysocki wrote:
> > > > > > Yes, but those are just remove events and we can only see how 
> > > > > > destructive they
> > > > > > were after the removal.  The point is to be able to figure out 
> > > > > > whether or not
> > > > > > we *want* to do the removal in the first place.
> > > > > > 
> > > > > > Say you have a computing node which signals a hardware problem in a 
> > > > > > processor
> > > > > > package (the container with CPU cores, memory, PCI host bridge 
> > > > > > etc.).  You
> > > > > > may want to eject that package, but you don't want to kill the 
> > > > > > system this
> > > > > > way.  So if the eject is doable, it is very much desirable to do 
> > > > > > it, but if it
> > > > > > is not doable, you'd rather shut the box down and do the 
> > > > > > replacement afterward.
> > > > > > That may be costly, however (maybe weeks of computations), so it 
> > > > > > should be
> > > > > > avoided if possible, but not at the expense of crashing the box if 
> > > > > > the eject
> > > > > > doesn't work out.
> > > > > 
> > > > > It seems to me that we could handle that with the help of a new flag, 
> > > > > say
> > > > > "no_eject", in struct device, a global mutex, and a function that 
> > > > > will walk
> > > > > the given subtree of the device hierarchy and check if "no_eject" is 
> > > > > set for
> > > > > any devices in there.  Plus a global "no_eject" switch, perhaps.
> > > > 
> > > > I think this will always be racy, or at worst, slow things down on
> > > > normal device operations as you will always be having to grab this flag
> > > > whenever you want to do something new.
> > > 
> > > I don't see why this particular scheme should be racy, at least I don't 
> > > see any
> > > obvious races in it (although I'm not that good at races detection in 
> > > general,
> > > admittedly).
> > > 
> > > Also, I don't expect that flag to be used for everything, just for things 
> > > known
> > > to seriously break if forcible eject is done.  That may be not precise 
> > > enough,
> > > so that's a matter of defining its purpose more precisely.
> > > 
> > > We can do something like that on the ACPI level (ie. introduce a no_eject 
> > > flag
> > > in struct acpi_device and provide an iterface for the layers above ACPI to
> > > manipulate it) but then devices without ACPI namespace objects won't be
> > > covered.  That may not be a big deal, though.
> > > 
> > > So say dev is about to be used for something incompatible with ejecting, 
> > > so to
> > > speak.  Then, one would do platform_lock_eject(dev), which would check if 
> > > dev
> > > has an ACPI handle and then take acpi_eject_lock (if so).  The return 
> > > value of
> > > platform_lock_eject(dev) would need to be checked to see if the device is 
> > > not
> > > gone.  If it returns success (0), one would do something to the device and
> > > call platform_no_eject(dev) and then platform_unlock_eject(dev).
> > 
> > How does a device "know" it is doing something that is incompatible with
> > ejecting?  That's a non-trivial task from what I can tell.
> 
> I agree that this is complicated in general.  But.
> 
> There are devices known to have software "offline" and "online" operations
> such that after the "offline" the given device is guaranteed to be not used
> until "online".  We have that for CPU cores, for example, and user space can
> do it via /sys/devices/system/cpu/cpuX/online .  So, why don't we make the
> "online" set the no_eject flag (under the lock as appropriate) and the
> "offline" clear it?  And why don't we define such "online" and "offline" for
> all of the other "system" stuff, like memory, PCI host bridges etc. and make 
> it
> behave analogously?
> 
> Then, it is quite simple to say which devices should use the no_eject flag:
> devices that have "online" and "offline" exported to user space.  And guess
> who's responsible for "offlining" all of those things before trying to eject
> them: user space is.  From the kernel's point of view it is all clear.  Hands
> clean. :-)
> 
> Now, there's a different problem how to expose all of the relevant information
> to user space so that it knows what to "offline" for the specific eject
> operation to succeed, but that's kind of separate and worth addressing
> anyway.

So, the idea is to run a user space program that off-lines all relevant
devices before trimming ACPI devices.  Is that right?  That sounds like
a worth idea to consider with.  This basically moves the "sequencer"
part into user space instead of the kernel space in my proposal.  I
agree that how to expose all of the relevant info to user space is an
issue.  Also, we will need to make sure that the user program

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Tue, 2013-02-05 at 00:23 +0100, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 01:59:27 PM Toshi Kani wrote:
> > On Mon, 2013-02-04 at 20:45 +0100, Rafael J. Wysocki wrote:
> > > On Monday, February 04, 2013 09:46:18 AM Toshi Kani wrote:
> > > > On Mon, 2013-02-04 at 04:46 -0800, Greg KH wrote:
> > > > > On Sun, Feb 03, 2013 at 05:28:09PM -0700, Toshi Kani wrote:
> > > > > > On Sat, 2013-02-02 at 16:01 +0100, Greg KH wrote:
> > > > > > > On Fri, Feb 01, 2013 at 01:40:10PM -0700, Toshi Kani wrote:
> > > > > > > > On Fri, 2013-02-01 at 07:30 +, Greg KH wrote:
> > > > > > > > > On Thu, Jan 31, 2013 at 06:32:18PM -0700, Toshi Kani wrote:
> > > > > > > > >  > This is already done for PCI host bridges and platform 
> > > > > > > > > devices and I don't
> > > > > > > > > > > see why we can't do that for the other types of devices 
> > > > > > > > > > > too.
> > > > > > > > > > > 
> > > > > > > > > > > The only missing piece I see is a way to handle the 
> > > > > > > > > > > "eject" problem, i.e.
> > > > > > > > > > > when we try do eject a device at the top of a subtree and 
> > > > > > > > > > > need to tear down
> > > > > > > > > > > the entire subtree below it, but if that's going to lead 
> > > > > > > > > > > to a system crash,
> > > > > > > > > > > for example, we want to cancel the eject.  It seems to me 
> > > > > > > > > > > that we'll need some
> > > > > > > > > > > help from the driver core here.
> > > > > > > > > > 
> > > > > > > > > > There are three different approaches suggested for system 
> > > > > > > > > > device
> > > > > > > > > > hot-plug:
> > > > > > > > > >  A. Proceed within system device bus scan.
> > > > > > > > > >  B. Proceed within ACPI bus scan.
> > > > > > > > > >  C. Proceed with a sequence (as a mini-boot).
> > > > > > > > > > 
> > > > > > > > > > Option A uses system devices as tokens, option B uses acpi 
> > > > > > > > > > devices as
> > > > > > > > > > tokens, and option C uses resource tables as tokens, for 
> > > > > > > > > > their handlers.
> > > > > > > > > > 
> > > > > > > > > > Here is summary of key questions & answers so far.  I hope 
> > > > > > > > > > this
> > > > > > > > > > clarifies why I am suggesting option 3.
> > > > > > > > > > 
> > > > > > > > > > 1. What are the system devices?
> > > > > > > > > > System devices provide system-wide core computing 
> > > > > > > > > > resources, which are
> > > > > > > > > > essential to compose a computer system.  System devices are 
> > > > > > > > > > not
> > > > > > > > > > connected to any particular standard buses.
> > > > > > > > > 
> > > > > > > > > Not a problem, lots of devices are not connected to any 
> > > > > > > > > "particular
> > > > > > > > > standard busses".  All this means is that system devices are 
> > > > > > > > > connected
> > > > > > > > > to the "system" bus, nothing more.
> > > > > > > > 
> > > > > > > > Can you give me a few examples of other devices that support 
> > > > > > > > hotplug and
> > > > > > > > are not connected to any particular buses?  I will investigate 
> > > > > > > > them to
> > > > > > > > see how they are managed to support hotplug.
> > > > > > > 
> > > > > > > Any device that is attached to any bus in the driver model can be
> > > > > > > hotunplugged from userspace by telling i

Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

[Toshi Kani]

On Tue, 2013-02-05 at 00:52 +0100, Rafael J. Wysocki wrote:
> On Monday, February 04, 2013 03:13:29 PM Toshi Kani wrote:
> > On Mon, 2013-02-04 at 21:07 +0100, Rafael J. Wysocki wrote:
> > > On Monday, February 04, 2013 06:33:52 AM Greg KH wrote:
> > > > On Mon, Feb 04, 2013 at 03:21:22PM +0100, Rafael J. Wysocki wrote:
> > > > > On Monday, February 04, 2013 04:48:10 AM Greg KH wrote:
> > > > > > On Sun, Feb 03, 2013 at 09:44:39PM +0100, Rafael J. Wysocki wrote:
> > > > > > > > Yes, but those are just remove events and we can only see how 
> > > > > > > > destructive they
> > > > > > > > were after the removal.  The point is to be able to figure out 
> > > > > > > > whether or not
> > > > > > > > we *want* to do the removal in the first place.
> > > > > > > > 
> > > > > > > > Say you have a computing node which signals a hardware problem 
> > > > > > > > in a processor
> > > > > > > > package (the container with CPU cores, memory, PCI host bridge 
> > > > > > > > etc.).  You
> > > > > > > > may want to eject that package, but you don't want to kill the 
> > > > > > > > system this
> > > > > > > > way.  So if the eject is doable, it is very much desirable to 
> > > > > > > > do it, but if it
> > > > > > > > is not doable, you'd rather shut the box down and do the 
> > > > > > > > replacement afterward.
> > > > > > > > That may be costly, however (maybe weeks of computations), so 
> > > > > > > > it should be
> > > > > > > > avoided if possible, but not at the expense of crashing the box 
> > > > > > > > if the eject
> > > > > > > > doesn't work out.
> > > > > > > 
> > > > > > > It seems to me that we could handle that with the help of a new 
> > > > > > > flag, say
> > > > > > > "no_eject", in struct device, a global mutex, and a function that 
> > > > > > > will walk
> > > > > > > the given subtree of the device hierarchy and check if "no_eject" 
> > > > > > > is set for
> > > > > > > any devices in there.  Plus a global "no_eject" switch, perhaps.
> > > > > > 
> > > > > > I think this will always be racy, or at worst, slow things down on
> > > > > > normal device operations as you will always be having to grab this 
> > > > > > flag
> > > > > > whenever you want to do something new.
> > > > > 
> > > > > I don't see why this particular scheme should be racy, at least I 
> > > > > don't see any
> > > > > obvious races in it (although I'm not that good at races detection in 
> > > > > general,
> > > > > admittedly).
> > > > > 
> > > > > Also, I don't expect that flag to be used for everything, just for 
> > > > > things known
> > > > > to seriously break if forcible eject is done.  That may be not 
> > > > > precise enough,
> > > > > so that's a matter of defining its purpose more precisely.
> > > > > 
> > > > > We can do something like that on the ACPI level (ie. introduce a 
> > > > > no_eject flag
> > > > > in struct acpi_device and provide an iterface for the layers above 
> > > > > ACPI to
> > > > > manipulate it) but then devices without ACPI namespace objects won't 
> > > > > be
> > > > > covered.  That may not be a big deal, though.
> > > > > 
> > > > > So say dev is about to be used for something incompatible with 
> > > > > ejecting, so to
> > > > > speak.  Then, one would do platform_lock_eject(dev), which would 
> > > > > check if dev
> > > > > has an ACPI handle and then take acpi_eject_lock (if so).  The return 
> > > > > value of
> > > > > platform_lock_eject(dev) would need to be checked to see if the 
> > > > > device is not
> > > > > gone.  If it returns success (0), one would do something to the 
> > > > > device and
> > > > > ca

Re: [patch] mm, hotplug: avoid compiling memory hotremove functions when disabled

[Toshi Kani]

On Tue, 2013-04-09 at 23:07 -0700, David Rientjes wrote:
> __remove_pages() is only necessary for CONFIG_MEMORY_HOTREMOVE.  PowerPC
> pseries will return -EOPNOTSUPP if unsupported.
> 
> Adding an #ifdef causes several other functions it depends on to also
> become unnecessary, which saves in .text when disabled (it's disabled in
> most defconfigs besides powerpc, including x86).  remove_memory_block()
> becomes static since it is not referenced outside of
> drivers/base/memory.c.
> 
> Build tested on x86 and powerpc with CONFIG_MEMORY_HOTREMOVE both enabled
> and disabled.
> 
> Signed-off-by: David Rientjes 

Acked-by: Toshi Kani 

Thanks,
-Toshi


> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c | 12 +
>  drivers/base/memory.c   | 44 +++
>  include/linux/memory.h  |  3 +-
>  include/linux/memory_hotplug.h  |  4 +-
>  mm/memory_hotplug.c | 68 +++
>  mm/sparse.c | 72 
> +
>  6 files changed, 113 insertions(+), 90 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
> b/arch/powerpc/platforms/pseries/hotplug-memory.c
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -72,6 +72,7 @@ unsigned long memory_block_size_bytes(void)
>   return get_memblock_size();
>  }
>  
> +#ifdef CONFIG_MEMORY_HOTREMOVE
>  static int pseries_remove_memblock(unsigned long base, unsigned int 
> memblock_size)
>  {
>   unsigned long start, start_pfn;
> @@ -153,6 +154,17 @@ static int pseries_remove_memory(struct device_node *np)
>   ret = pseries_remove_memblock(base, lmb_size);
>   return ret;
>  }
> +#else
> +static inline int pseries_remove_memblock(unsigned long base,
> +   unsigned int memblock_size)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int pseries_remove_memory(struct device_node *np)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_MEMORY_HOTREMOVE */
>  
>  static int pseries_add_memory(struct device_node *np)
>  {
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -93,16 +93,6 @@ int register_memory(struct memory_block *memory)
>   return error;
>  }
>  
> -static void
> -unregister_memory(struct memory_block *memory)
> -{
> - BUG_ON(memory->dev.bus != &memory_subsys);
> -
> - /* drop the ref. we got in remove_memory_block() */
> - kobject_put(&memory->dev.kobj);
> - device_unregister(&memory->dev);
> -}
> -
>  unsigned long __weak memory_block_size_bytes(void)
>  {
>   return MIN_MEMORY_BLOCK_SIZE;
> @@ -637,8 +627,28 @@ static int add_memory_section(int nid, struct 
> mem_section *section,
>   return ret;
>  }
>  
> -int remove_memory_block(unsigned long node_id, struct mem_section *section,
> - int phys_device)
> +/*
> + * need an interface for the VM to add new memory regions,
> + * but without onlining it.
> + */
> +int register_new_memory(int nid, struct mem_section *section)
> +{
> + return add_memory_section(nid, section, NULL, MEM_OFFLINE, HOTPLUG);
> +}
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +static void
> +unregister_memory(struct memory_block *memory)
> +{
> + BUG_ON(memory->dev.bus != &memory_subsys);
> +
> + /* drop the ref. we got in remove_memory_block() */
> + kobject_put(&memory->dev.kobj);
> + device_unregister(&memory->dev);
> +}
> +
> +static int remove_memory_block(unsigned long node_id,
> +struct mem_section *section, int phys_device)
>  {
>   struct memory_block *mem;
>  
> @@ -661,15 +671,6 @@ int remove_memory_block(unsigned long node_id, struct 
> mem_section *section,
>   return 0;
>  }
>  
> -/*
> - * need an interface for the VM to add new memory regions,
> - * but without onlining it.
> - */
> -int register_new_memory(int nid, struct mem_section *section)
> -{
> - return add_memory_section(nid, section, NULL, MEM_OFFLINE, HOTPLUG);
> -}
> -
>  int unregister_memory_section(struct mem_section *section)
>  {
>   if (!present_section(section))
> @@ -677,6 +678,7 @@ int unregister_memory_section(struct mem_section *section)
>  
>   return remove_memory_block(0, section, 0);
>  }
> +#endif /* CONFIG_MEMORY_HOTREMOVE */
>  
>  /*
>   * offline one memory block. If the memory block has been offlined, do 
> nothing.

Re: [RFC PATCH 00/11] Hot-plug and Online/Offline framework

[Toshi Kani]

On Thu, 2012-12-13 at 10:30 -0800, Greg KH wrote:
> On Thu, Dec 13, 2012 at 09:03:54AM -0700, Toshi Kani wrote:
> > On Thu, 2012-12-13 at 04:16 +, Greg KH wrote:
> > > On Wed, Dec 12, 2012 at 08:37:44PM -0700, Toshi Kani wrote:
> > > > On Wed, 2012-12-12 at 16:55 -0800, Greg KH wrote:
> > > > > On Wed, Dec 12, 2012 at 05:39:36PM -0700, Toshi Kani wrote:
> > > > > > On Wed, 2012-12-12 at 15:56 -0800, Greg KH wrote:
> > > > > > > On Wed, Dec 12, 2012 at 04:17:12PM -0700, Toshi Kani wrote:
> > > > > > > > This patchset is an initial prototype of proposed hot-plug 
> > > > > > > > framework
> > > > > > > > for design review.  The hot-plug framework is designed to 
> > > > > > > > provide 
> > > > > > > > the common framework for hot-plugging and online/offline 
> > > > > > > > operations
> > > > > > > > of system devices, such as CPU, Memory and Node.  While this 
> > > > > > > > patchset
> > > > > > > > only supports ACPI-based hot-plug operations, the framework 
> > > > > > > > itself is
> > > > > > > > designed to be platform-neural and can support other FW 
> > > > > > > > architectures
> > > > > > > > as necessary.
> > > > > > > > 
> > > > > > > > The patchset has not been fully tested yet, esp. for memory 
> > > > > > > > hot-plug.
> > > > > > > > Any help for testing will be very appreciated since my test 
> > > > > > > > setup
> > > > > > > > is limited.
> > > > > > > > 
> > > > > > > > The patchset is based on the linux-next branch of linux-pm.git 
> > > > > > > > tree.
> > > > > > > > 
> > > > > > > > Overview of the Framework
> > > > > > > > =
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > Why all the new framework, doesn't the existing bus infrastructure
> > > > > > > provide everything you need here?  Shouldn't you just be putting 
> > > > > > > your
> > > > > > > cpus and memory sticks on a bus and handle stuff that way?  What 
> > > > > > > makes
> > > > > > > these types of devices so unique from all other devices that 
> > > > > > > Linux has
> > > > > > > been handling in a dynamic manner (i.e. hotplugging them) for 
> > > > > > > many many
> > > > > > > years?
> > > > > > > 
> > > > > > > Why are you reinventing the wheel?
> > > > > > 
> > > > > > Good question.  Yes, USB and PCI hotplug operate based on their bus
> > > > > > structures.  USB and PCI cards only work under USB and PCI bus
> > > > > > controllers.  So, their framework can be composed within the bus
> > > > > > structures as you pointed out.
> > > > > > 
> > > > > > However, system devices such CPU and memory do not have their 
> > > > > > standard
> > > > > > bus.  ACPI allows these system devices to be enumerated, but it 
> > > > > > does not
> > > > > > make ACPI as the HW bus hierarchy for CPU and memory, unlike PCI and
> > > > > > USB.  Therefore, CPU and memory modules manage CPU and memory 
> > > > > > outside of
> > > > > > ACPI.  This makes sense because CPU and memory can be used without 
> > > > > > ACPI.
> > > > > > 
> > > > > > This leads us an issue when we try to manage system device hotplug
> > > > > > within ACPI, because ACPI does not control everything.  This 
> > > > > > patchset
> > > > > > provides a common hotplug framework for system devices, which both 
> > > > > > ACPI
> > > > > > and non-ACPI modules (i.e. CPU and memory modules) can participate 
> > > > > > and
> > > > > > are coordinated for their hotplug operations.  This is analogous to 
> > > > > > the
> > > > > > boot-up sequence, which ACPI and n

[PATCH 1/4] hotplug, x86: Fix online state in cpu0 debug interface

[Toshi Kani]

_debug_hotplug_cpu() is a debug interface that puts cpu0 offline during
boot-up when CONFIG_DEBUG_HOTPLUG_CPU0 is set.  After cpu0 is put offline
in this interface, however, /sys/devices/system/cpu/cpu0/online still
shows 1 (online).

This patch fixes _debug_hotplug_cpu() to update dev->offline when CPU
online/offline operation succeeded.

Signed-off-by: Toshi Kani 
---
 arch/x86/kernel/topology.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 6e60b5f..5823bbd 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -72,16 +72,19 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
ret = cpu_down(cpu);
if (!ret) {
pr_info("CPU %u is now offline\n", cpu);
+   dev->offline = true;
kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
} else
pr_debug("Can't offline CPU%d.\n", cpu);
break;
case 1:
ret = cpu_up(cpu);
-   if (!ret)
+   if (!ret) {
+   dev->offline = false;
kobject_uevent(&dev->kobj, KOBJ_ONLINE);
-   else
+   } else {
pr_debug("Can't online CPU%d.\n", cpu);
+   }
break;
default:
ret = -EINVAL;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

[Toshi Kani]

cpu_hotplug_driver_lock() serializes CPU online/offline operations
when ARCH_CPU_PROBE_RELEASE is set.  This lock interface is no longer
necessary with the following reason:

 - lock_device_hotplug() now protects CPU online/offline operations,
   including the probe & release interfaces enabled by
   ARCH_CPU_PROBE_RELEASE.  The use of cpu_hotplug_driver_lock() is
   redundant.
 - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
   is defined, which is misleading and is only enabled on powerpc.

This patch removes the cpu_hotplug_driver_lock() interface.  As
a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
probe & release interface as intended.  There is no functional change
in this patch.

Signed-off-by: Toshi Kani 
---
Performed build test only on powerpc.
---
 arch/powerpc/kernel/smp.c  |   12 --
 arch/powerpc/platforms/pseries/dlpar.c |   40 
 arch/x86/kernel/topology.c |2 --
 drivers/base/cpu.c |   10 +---
 include/linux/cpu.h|   13 --
 5 files changed, 16 insertions(+), 61 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 38b0ba6..1667269 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -763,18 +763,6 @@ void __cpu_die(unsigned int cpu)
smp_ops->cpu_die(cpu);
 }
 
-static DEFINE_MUTEX(powerpc_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock()
-{
-   mutex_lock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock()
-{
-   mutex_unlock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
 void cpu_die(void)
 {
if (ppc_md.cpu_die)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index a1a7b9a..e39325d 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -387,18 +387,13 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
char *cpu_name;
int rc;
 
-   cpu_hotplug_driver_lock();
rc = strict_strtoul(buf, 0, &drc_index);
-   if (rc) {
-   rc = -EINVAL;
-   goto out;
-   }
+   if (rc)
+   return -EINVAL;
 
dn = dlpar_configure_connector(drc_index);
-   if (!dn) {
-   rc = -EINVAL;
-   goto out;
-   }
+   if (!dn)
+   return -EINVAL;
 
/* configure-connector reports cpus as living in the base
 * directory of the device tree.  CPUs actually live in the
@@ -407,8 +402,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
if (!cpu_name) {
dlpar_free_cc_nodes(dn);
-   rc = -ENOMEM;
-   goto out;
+   return -ENOMEM;
}
 
kfree(dn->full_name);
@@ -417,22 +411,21 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
rc = dlpar_acquire_drc(drc_index);
if (rc) {
dlpar_free_cc_nodes(dn);
-   rc = -EINVAL;
-   goto out;
+   return -EINVAL;
}
 
rc = dlpar_attach_node(dn);
if (rc) {
dlpar_release_drc(drc_index);
dlpar_free_cc_nodes(dn);
-   goto out;
+   return rc;
}
 
rc = dlpar_online_cpu(dn);
-out:
-   cpu_hotplug_driver_unlock();
+   if (rc)
+   return rc;
 
-   return rc ? rc : count;
+   return count;
 }
 
 static int dlpar_offline_cpu(struct device_node *dn)
@@ -505,30 +498,27 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   cpu_hotplug_driver_lock();
rc = dlpar_offline_cpu(dn);
if (rc) {
of_node_put(dn);
-   rc = -EINVAL;
-   goto out;
+   return -EINVAL;
}
 
rc = dlpar_release_drc(*drc_index);
if (rc) {
of_node_put(dn);
-   goto out;
+   return rc;
}
 
rc = dlpar_detach_node(dn);
if (rc) {
dlpar_acquire_drc(*drc_index);
-   goto out;
+   return rc;
}
 
of_node_put(dn);
-out:
-   cpu_hotplug_driver_unlock();
-   return rc ? rc : count;
+
+   return count;
 }
 
 static int __init pseries_dlpar_init(void)
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index a3f35eb..649b010 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -66,7 +66,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
return -EINVAL;
 
lock_device_hotplug();
-   cpu_hotplug_driver_lock();
 
switch (action) {
case 0:
@@ -91,7 +90,6 @@ int __ref _debug_hotplug_cpu(i

[PATCH 2/4] hotplug, x86: Add hotplug lock to missing places

[Toshi Kani]

lock_device_hotplug() serializes CPU & Memory online/offline and hotplug
operations.  However, this lock is not held in the debug interfaces below
that initiate CPU online/offline operations.

 - _debug_hotplug_cpu(), cpu0 hotplug test interface enabled by
   CONFIG_DEBUG_HOTPLUG_CPU0.
 - cpu_probe_store() and cpu_release_store(), cpu hotplug test interface
   enabled by CONFIG_ARCH_CPU_PROBE_RELEASE.

This patch changes the above interfaces to hold lock_device_hotplug().

Signed-off-by: Toshi Kani 
---
 arch/x86/kernel/topology.c |2 ++
 drivers/base/cpu.c |   16 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 5823bbd..a3f35eb 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -65,6 +65,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
if (!cpu_is_hotpluggable(cpu))
return -EINVAL;
 
+   lock_device_hotplug();
cpu_hotplug_driver_lock();
 
switch (action) {
@@ -91,6 +92,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
}
 
cpu_hotplug_driver_unlock();
+   unlock_device_hotplug();
 
return ret;
 }
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 4c358bc..4cc6928 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -88,7 +88,13 @@ static ssize_t cpu_probe_store(struct device *dev,
   const char *buf,
   size_t count)
 {
-   return arch_cpu_probe(buf, count);
+   ssize_t ret;
+
+   lock_device_hotplug();
+   ret = arch_cpu_probe(buf, count);
+   unlock_device_hotplug();
+
+   return ret;
 }
 
 static ssize_t cpu_release_store(struct device *dev,
@@ -96,7 +102,13 @@ static ssize_t cpu_release_store(struct device *dev,
 const char *buf,
 size_t count)
 {
-   return arch_cpu_release(buf, count);
+   ssize_t ret;
+
+   lock_device_hotplug();
+   ret = arch_cpu_release(buf, count);
+   unlock_device_hotplug();
+
+   return ret;
 }
 
 static DEVICE_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/4] Unify CPU hotplug lock interface

[Toshi Kani]

lock_device_hotplug() was recently introduced to serialize CPU & Memory
online/offline and hotplug operations, along with sysfs online interface
restructure (commit 4f3549d7).  With this new locking scheme,
cpu_hotplug_driver_lock() is redundant and is no longer necessary.

This patchset makes sure that lock_device_hotplug() covers all CPU online/
offline interfaces, and then removes cpu_hotplug_driver_lock().

The patchset is based on Linus's tree, 3.11.0-rc5.

---
Toshi Kani (4):
  hotplug, x86: Fix online state in cpu0 debug interface
  hotplug, x86: Add hotplug lock to missing places
  hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
  hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

---
 arch/powerpc/kernel/smp.c  | 12 --
 arch/powerpc/platforms/pseries/dlpar.c | 40 +-
 arch/x86/Kconfig   |  4 
 arch/x86/kernel/smpboot.c  | 21 --
 arch/x86/kernel/topology.c | 11 ++
 drivers/base/cpu.c | 26 --
 include/linux/cpu.h| 13 ---
 7 files changed, 37 insertions(+), 90 deletions(-)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/4] hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86

[Toshi Kani]

Commit d7c53c9e enabled ARCH_CPU_PROBE_RELEASE on x86 in order to
serialize CPU online/offline operations.  Although it is the config
option to enable CPU hotplug test interfaces, probe & release, it is
also the option to enable cpu_hotplug_driver_lock() as well.  Therefore,
this option had to be enabled on x86 with dummy arch_cpu_probe() and
arch_cpu_release().

Since then, lock_device_hotplug() was introduced to serialize CPU
online/offline & hotplug operations.  Therefore, this config option
is no longer required for the serialization.  This patch disables
this config option on x86 and revert the changes made by commit
d7c53c9e.

Signed-off-by: Toshi Kani 
---
 arch/x86/Kconfig  |4 
 arch/x86/kernel/smpboot.c |   21 -
 2 files changed, 25 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b32ebf9..c87e49a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -255,10 +255,6 @@ config ARCH_HWEIGHT_CFLAGS
default "-fcall-saved-ecx -fcall-saved-edx" if X86_32
default "-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11" if X86_64
 
-config ARCH_CPU_PROBE_RELEASE
-   def_bool y
-   depends on HOTPLUG_CPU
-
 config ARCH_SUPPORTS_UPROBES
def_bool y
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index aecc98a..5b24a9d 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -82,27 +82,6 @@
 /* State of each CPU */
 DEFINE_PER_CPU(int, cpu_state) = { 0 };
 
-#ifdef CONFIG_HOTPLUG_CPU
-/*
- * We need this for trampoline_base protection from concurrent accesses when
- * off- and onlining cores wildly.
- */
-static DEFINE_MUTEX(x86_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock(void)
-{
-   mutex_lock(&x86_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock(void)
-{
-   mutex_unlock(&x86_cpu_hotplug_driver_mutex);
-}
-
-ssize_t arch_cpu_probe(const char *buf, size_t count) { return -1; }
-ssize_t arch_cpu_release(const char *buf, size_t count) { return -1; }
-#endif
-
 /* Number of siblings per CPU package */
 int smp_num_siblings = 1;
 EXPORT_SYMBOL(smp_num_siblings);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/4] hotplug, x86: Add hotplug lock to missing places

[Toshi Kani]

On Sat, 2013-08-17 at 16:15 -0700, Greg KH wrote:
> On Sat, Aug 17, 2013 at 01:46:57PM -0600, Toshi Kani wrote:
> > lock_device_hotplug() serializes CPU & Memory online/offline and hotplug
> > operations.  However, this lock is not held in the debug interfaces below
> > that initiate CPU online/offline operations.
> > 
> >  - _debug_hotplug_cpu(), cpu0 hotplug test interface enabled by
> >CONFIG_DEBUG_HOTPLUG_CPU0.
> >  - cpu_probe_store() and cpu_release_store(), cpu hotplug test interface
> >enabled by CONFIG_ARCH_CPU_PROBE_RELEASE.
> > 
> > This patch changes the above interfaces to hold lock_device_hotplug().
> > 
> > Signed-off-by: Toshi Kani 
> > ---
> >  arch/x86/kernel/topology.c |2 ++
> >  drivers/base/cpu.c |   16 ++--
> >  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> Acked-by: Greg Kroah-Hartman 

Thanks Greg!
-Toshi



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/4] Unify CPU hotplug lock interface

[Toshi Kani]

On Sun, 2013-08-18 at 03:02 +0200, Rafael J. Wysocki wrote:
> On Saturday, August 17, 2013 01:46:55 PM Toshi Kani wrote:
> > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > online/offline and hotplug operations, along with sysfs online interface
> > restructure (commit 4f3549d7).  With this new locking scheme,
> > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > 
> > This patchset makes sure that lock_device_hotplug() covers all CPU online/
> > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > 
> > The patchset is based on Linus's tree, 3.11.0-rc5.
> 
> Nice series, thanks a lot for taking care of this!

Thanks Rafael!
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

[Toshi Kani]

On Mon, 2013-08-19 at 10:20 -0500, Nathan Fontenot wrote:
> On 08/17/2013 02:46 PM, Toshi Kani wrote:
> > cpu_hotplug_driver_lock() serializes CPU online/offline operations
> > when ARCH_CPU_PROBE_RELEASE is set.  This lock interface is no longer
> > necessary with the following reason:
> > 
> >  - lock_device_hotplug() now protects CPU online/offline operations,
> >including the probe & release interfaces enabled by
> >ARCH_CPU_PROBE_RELEASE.  The use of cpu_hotplug_driver_lock() is
> >redundant.
> >  - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
> >is defined, which is misleading and is only enabled on powerpc.
> > 
> > This patch removes the cpu_hotplug_driver_lock() interface.  As
> > a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
> > probe & release interface as intended.  There is no functional change
> > in this patch.
> > 
> > Signed-off-by: Toshi Kani 
> 
> Reviewed-by: Nathan Fontenot 

Thanks Nathan!
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/4] Unify CPU hotplug lock interface

[Toshi Kani]

On Sun, 2013-08-18 at 03:02 +0200, Rafael J. Wysocki wrote:
> On Saturday, August 17, 2013 01:46:55 PM Toshi Kani wrote:
> > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > online/offline and hotplug operations, along with sysfs online interface
> > restructure (commit 4f3549d7).  With this new locking scheme,
> > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > 
> > This patchset makes sure that lock_device_hotplug() covers all CPU online/
> > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > 
> > The patchset is based on Linus's tree, 3.11.0-rc5.
> 
> Nice series, thanks a lot for taking care of this!

Hi Rafael,

Per the recent your changes in lock_device_hotplug(), do you think it
makes sense to integrate this patchset into your tree?  I am also
considering to add one more patch to use lock_device_hotplug_sysfs() in
cpu_probe_store().  I will rebase to your tree and send them today if it
makes sense to you.

Thanks,
-Toshi


> 
> Rafael
> 
> 
> > ---
> > Toshi Kani (4):
> >   hotplug, x86: Fix online state in cpu0 debug interface
> >   hotplug, x86: Add hotplug lock to missing places
> >   hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
> >   hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
> > 
> > ---
> >  arch/powerpc/kernel/smp.c  | 12 --
> >  arch/powerpc/platforms/pseries/dlpar.c | 40 
> > +-
> >  arch/x86/Kconfig   |  4 
> >  arch/x86/kernel/smpboot.c  | 21 --
> >  arch/x86/kernel/topology.c | 11 ++
> >  drivers/base/cpu.c | 26 --
> >  include/linux/cpu.h| 13 ---
> >  7 files changed, 37 insertions(+), 90 deletions(-)
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/4] Unify CPU hotplug lock interface

[Toshi Kani]

On Fri, 2013-08-30 at 02:06 +0200, Rafael J. Wysocki wrote:
> On Thursday, August 29, 2013 11:15:10 AM Toshi Kani wrote:
> > On Sun, 2013-08-18 at 03:02 +0200, Rafael J. Wysocki wrote:
> > > On Saturday, August 17, 2013 01:46:55 PM Toshi Kani wrote:
> > > > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > > > online/offline and hotplug operations, along with sysfs online interface
> > > > restructure (commit 4f3549d7).  With this new locking scheme,
> > > > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > > > 
> > > > This patchset makes sure that lock_device_hotplug() covers all CPU 
> > > > online/
> > > > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > > > 
> > > > The patchset is based on Linus's tree, 3.11.0-rc5.
> > > 
> > > Nice series, thanks a lot for taking care of this!
> > 
> > Hi Rafael,
> > 
> > Per the recent your changes in lock_device_hotplug(), do you think it
> > makes sense to integrate this patchset into your tree?  I am also
> > considering to add one more patch to use lock_device_hotplug_sysfs() in
> > cpu_probe_store().  I will rebase to your tree and send them today if it
> > makes sense to you.
> 
> Yes, it does to me.

Great!  I will send them shortly.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 1/4] hotplug, x86: Fix online state in cpu0 debug interface

[Toshi Kani]

_debug_hotplug_cpu() is a debug interface that puts cpu0 offline during
boot-up when CONFIG_DEBUG_HOTPLUG_CPU0 is set.  After cpu0 is put offline
in this interface, however, /sys/devices/system/cpu/cpu0/online still
shows 1 (online).

This patch fixes _debug_hotplug_cpu() to update dev->offline when CPU
online/offline operation succeeded.

Signed-off-by: Toshi Kani 
Acked-by: Rafael J. Wysocki 
---
 arch/x86/kernel/topology.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 6e60b5f..5823bbd 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -72,16 +72,19 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
ret = cpu_down(cpu);
if (!ret) {
pr_info("CPU %u is now offline\n", cpu);
+   dev->offline = true;
kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
} else
pr_debug("Can't offline CPU%d.\n", cpu);
break;
case 1:
ret = cpu_up(cpu);
-   if (!ret)
+   if (!ret) {
+   dev->offline = false;
kobject_uevent(&dev->kobj, KOBJ_ONLINE);
-   else
+   } else {
pr_debug("Can't online CPU%d.\n", cpu);
+   }
break;
default:
ret = -EINVAL;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 2/4] hotplug, x86: Add hotplug lock to missing places

[Toshi Kani]

lock_device_hotplug[_sysfs]() serializes CPU & Memory online/offline
and hotplug operations.  However, this lock is not held in the debug
interfaces below that initiate CPU online/offline operations.

 - _debug_hotplug_cpu(), cpu0 hotplug test interface enabled by
   CONFIG_DEBUG_HOTPLUG_CPU0.
 - cpu_probe_store() and cpu_release_store(), cpu hotplug test interface
   enabled by CONFIG_ARCH_CPU_PROBE_RELEASE.

This patch changes the above interfaces to hold lock_device_hotplug().

Signed-off-by: Toshi Kani 
Acked-by: Greg Kroah-Hartman 
Acked-by: Rafael J. Wysocki 
---
 arch/x86/kernel/topology.c |2 ++
 drivers/base/cpu.c |   24 ++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 5823bbd..a3f35eb 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -65,6 +65,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
if (!cpu_is_hotpluggable(cpu))
return -EINVAL;
 
+   lock_device_hotplug();
cpu_hotplug_driver_lock();
 
switch (action) {
@@ -91,6 +92,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
}
 
cpu_hotplug_driver_unlock();
+   unlock_device_hotplug();
 
return ret;
 }
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 4cf0717..dc78e45 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -89,7 +89,17 @@ static ssize_t cpu_probe_store(struct device *dev,
   const char *buf,
   size_t count)
 {
-   return arch_cpu_probe(buf, count);
+   ssize_t cnt;
+   int ret;
+
+   ret = lock_device_hotplug_sysfs();
+   if (ret)
+   return ret;
+
+   cnt = arch_cpu_probe(buf, count);
+
+   unlock_device_hotplug();
+   return cnt;
 }
 
 static ssize_t cpu_release_store(struct device *dev,
@@ -97,7 +107,17 @@ static ssize_t cpu_release_store(struct device *dev,
 const char *buf,
 size_t count)
 {
-   return arch_cpu_release(buf, count);
+   ssize_t cnt;
+   int ret;
+
+   ret = lock_device_hotplug_sysfs();
+   if (ret)
+   return ret;
+
+   cnt = arch_cpu_release(buf, count);
+
+   unlock_device_hotplug();
+   return cnt;
 }
 
 static DEVICE_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 3/4] hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86

[Toshi Kani]

Commit d7c53c9e enabled ARCH_CPU_PROBE_RELEASE on x86 in order to
serialize CPU online/offline operations.  Although it is the config
option to enable CPU hotplug test interfaces, probe & release, it is
also the option to enable cpu_hotplug_driver_lock() as well.  Therefore,
this option had to be enabled on x86 with dummy arch_cpu_probe() and
arch_cpu_release().

Since then, lock_device_hotplug() was introduced to serialize CPU
online/offline & hotplug operations.  Therefore, this config option
is no longer required for the serialization.  This patch disables
this config option on x86 and revert the changes made by commit
d7c53c9e.

Signed-off-by: Toshi Kani 
Acked-by: Rafael J. Wysocki 
---
 arch/x86/Kconfig  |4 
 arch/x86/kernel/smpboot.c |   21 -
 2 files changed, 25 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b32ebf9..c87e49a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -255,10 +255,6 @@ config ARCH_HWEIGHT_CFLAGS
default "-fcall-saved-ecx -fcall-saved-edx" if X86_32
default "-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11" if X86_64
 
-config ARCH_CPU_PROBE_RELEASE
-   def_bool y
-   depends on HOTPLUG_CPU
-
 config ARCH_SUPPORTS_UPROBES
def_bool y
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index aecc98a..5b24a9d 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -82,27 +82,6 @@
 /* State of each CPU */
 DEFINE_PER_CPU(int, cpu_state) = { 0 };
 
-#ifdef CONFIG_HOTPLUG_CPU
-/*
- * We need this for trampoline_base protection from concurrent accesses when
- * off- and onlining cores wildly.
- */
-static DEFINE_MUTEX(x86_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock(void)
-{
-   mutex_lock(&x86_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock(void)
-{
-   mutex_unlock(&x86_cpu_hotplug_driver_mutex);
-}
-
-ssize_t arch_cpu_probe(const char *buf, size_t count) { return -1; }
-ssize_t arch_cpu_release(const char *buf, size_t count) { return -1; }
-#endif
-
 /* Number of siblings per CPU package */
 int smp_num_siblings = 1;
 EXPORT_SYMBOL(smp_num_siblings);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

[Toshi Kani]

cpu_hotplug_driver_lock() serializes CPU online/offline operations
when ARCH_CPU_PROBE_RELEASE is set.  This lock interface is no longer
necessary with the following reason:

 - lock_device_hotplug() now protects CPU online/offline operations,
   including the probe & release interfaces enabled by
   ARCH_CPU_PROBE_RELEASE.  The use of cpu_hotplug_driver_lock() is
   redundant.
 - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
   is defined, which is misleading and is only enabled on powerpc.

This patch removes the cpu_hotplug_driver_lock() interface.  As
a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
probe & release interface as intended.  There is no functional change
in this patch.

Signed-off-by: Toshi Kani 
Acked-by: Rafael J. Wysocki 
Reviewed-by: Nathan Fontenot 
---
Performed build test only on powerpc.
---
 arch/powerpc/kernel/smp.c  |   12 --
 arch/powerpc/platforms/pseries/dlpar.c |   40 
 arch/x86/kernel/topology.c |2 --
 drivers/base/cpu.c |   10 +---
 include/linux/cpu.h|   13 --
 5 files changed, 16 insertions(+), 61 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 38b0ba6..1667269 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -763,18 +763,6 @@ void __cpu_die(unsigned int cpu)
smp_ops->cpu_die(cpu);
 }
 
-static DEFINE_MUTEX(powerpc_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock()
-{
-   mutex_lock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock()
-{
-   mutex_unlock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
 void cpu_die(void)
 {
if (ppc_md.cpu_die)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index a1a7b9a..e39325d 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -387,18 +387,13 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
char *cpu_name;
int rc;
 
-   cpu_hotplug_driver_lock();
rc = strict_strtoul(buf, 0, &drc_index);
-   if (rc) {
-   rc = -EINVAL;
-   goto out;
-   }
+   if (rc)
+   return -EINVAL;
 
dn = dlpar_configure_connector(drc_index);
-   if (!dn) {
-   rc = -EINVAL;
-   goto out;
-   }
+   if (!dn)
+   return -EINVAL;
 
/* configure-connector reports cpus as living in the base
 * directory of the device tree.  CPUs actually live in the
@@ -407,8 +402,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
if (!cpu_name) {
dlpar_free_cc_nodes(dn);
-   rc = -ENOMEM;
-   goto out;
+   return -ENOMEM;
}
 
kfree(dn->full_name);
@@ -417,22 +411,21 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
rc = dlpar_acquire_drc(drc_index);
if (rc) {
dlpar_free_cc_nodes(dn);
-   rc = -EINVAL;
-   goto out;
+   return -EINVAL;
}
 
rc = dlpar_attach_node(dn);
if (rc) {
dlpar_release_drc(drc_index);
dlpar_free_cc_nodes(dn);
-   goto out;
+   return rc;
}
 
rc = dlpar_online_cpu(dn);
-out:
-   cpu_hotplug_driver_unlock();
+   if (rc)
+   return rc;
 
-   return rc ? rc : count;
+   return count;
 }
 
 static int dlpar_offline_cpu(struct device_node *dn)
@@ -505,30 +498,27 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   cpu_hotplug_driver_lock();
rc = dlpar_offline_cpu(dn);
if (rc) {
of_node_put(dn);
-   rc = -EINVAL;
-   goto out;
+   return -EINVAL;
}
 
rc = dlpar_release_drc(*drc_index);
if (rc) {
of_node_put(dn);
-   goto out;
+   return rc;
}
 
rc = dlpar_detach_node(dn);
if (rc) {
dlpar_acquire_drc(*drc_index);
-   goto out;
+   return rc;
}
 
of_node_put(dn);
-out:
-   cpu_hotplug_driver_unlock();
-   return rc ? rc : count;
+
+   return count;
 }
 
 static int __init pseries_dlpar_init(void)
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index a3f35eb..649b010 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -66,7 +66,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
return -EINVAL;
 
lock_device_hotplug();
-   cpu_hotplug_driver_lock();
 
switch (action) {

[PATCH v2 0/4] Unify CPU hotplug lock interface

[Toshi Kani]

lock_device_hotplug() was recently introduced to serialize CPU & Memory
online/offline and hotplug operations, along with sysfs online interface
restructure (commit 4f3549d7).  With this new locking scheme,
cpu_hotplug_driver_lock() is redundant and is no longer necessary.

This patchset makes sure that lock_device_hotplug() covers all CPU online/
offline interfaces, and then removes cpu_hotplug_driver_lock().

v2:
 - Rebased to the pm tree, bleeding-edge.
 - Changed patch 2/4 to use lock_device_hotplug_sysfs().

---
Toshi Kani (4):
  hotplug, x86: Fix online state in cpu0 debug interface
  hotplug, x86: Add hotplug lock to missing places
  hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
  hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

---
 arch/powerpc/kernel/smp.c  | 12 --
 arch/powerpc/platforms/pseries/dlpar.c | 40 +-
 arch/x86/Kconfig   |  4 
 arch/x86/kernel/smpboot.c  | 21 --
 arch/x86/kernel/topology.c | 11 ++
 drivers/base/cpu.c | 34 +++--
 include/linux/cpu.h| 13 ---
 7 files changed, 45 insertions(+), 90 deletions(-)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 0/4] Unify CPU hotplug lock interface

[Toshi Kani]

On Fri, 2013-08-30 at 11:44 +0900, Yasuaki Ishimatsu wrote:
> (2013/08/30 9:22), Toshi Kani wrote:
> > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > online/offline and hotplug operations, along with sysfs online interface
> > restructure (commit 4f3549d7).  With this new locking scheme,
> > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > 
> > This patchset makes sure that lock_device_hotplug() covers all CPU online/
> > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > 
> > v2:
> >   - Rebased to the pm tree, bleeding-edge.
> >   - Changed patch 2/4 to use lock_device_hotplug_sysfs().
> > 
> > ---
> > Toshi Kani (4):
> >hotplug, x86: Fix online state in cpu0 debug interface
> >hotplug, x86: Add hotplug lock to missing places
> >hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
> >hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
> > 
> > ---
> The patch-set looks good to me.
> 
> Acked-by: Yasuaki Ishimatsu 

Thanks Yasuaki!
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 0/4] Unify CPU hotplug lock interface

[Toshi Kani]

On Fri, 2013-08-30 at 14:14 +0200, Rafael J. Wysocki wrote:
> On Friday, August 30, 2013 11:44:06 AM Yasuaki Ishimatsu wrote:
> > (2013/08/30 9:22), Toshi Kani wrote:
> > > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > > online/offline and hotplug operations, along with sysfs online interface
> > > restructure (commit 4f3549d7).  With this new locking scheme,
> > > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > > 
> > > This patchset makes sure that lock_device_hotplug() covers all CPU online/
> > > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > > 
> > > v2:
> > >   - Rebased to the pm tree, bleeding-edge.
> > >   - Changed patch 2/4 to use lock_device_hotplug_sysfs().
> > > 
> > > ---
> > > Toshi Kani (4):
> > >hotplug, x86: Fix online state in cpu0 debug interface
> > >hotplug, x86: Add hotplug lock to missing places
> > >hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
> > >hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
> > > 
> > > ---
> > The patch-set looks good to me.
> > 
> > Acked-by: Yasuaki Ishimatsu 
> 
> Thanks!  I'll tentatively queue up this series for 3.12 (for the second
> half of the merge window).

Thanks Rafael!
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

[Toshi Kani]

On Wed, 2013-09-25 at 08:27 +, Rafael J. Wysocki wrote:
> Hi,
> 
> On Thursday, August 29, 2013 06:22:09 PM Toshi Kani wrote:
> > cpu_hotplug_driver_lock() serializes CPU online/offline operations
> > when ARCH_CPU_PROBE_RELEASE is set.  This lock interface is no longer
> > necessary with the following reason:
> > 
> >  - lock_device_hotplug() now protects CPU online/offline operations,
> >including the probe & release interfaces enabled by
> >ARCH_CPU_PROBE_RELEASE.  The use of cpu_hotplug_driver_lock() is
> >redundant.
> >  - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
> >is defined, which is misleading and is only enabled on powerpc.
> > 
> > This patch removes the cpu_hotplug_driver_lock() interface.  As
> > a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
> > probe & release interface as intended.  There is no functional change
> > in this patch.
> > 
> > Signed-off-by: Toshi Kani 
> > Acked-by: Rafael J. Wysocki 
> > Reviewed-by: Nathan Fontenot 
> 
> Can you please rebase this patch on top of 3.12-rc2?  It doesn't apply for
> me any more.

Yes, I will send this patch on top of 3.12-rc2.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH RESEND 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()

[Toshi Kani]

cpu_hotplug_driver_lock() serializes CPU online/offline operations
when ARCH_CPU_PROBE_RELEASE is set.  This lock interface is no longer
necessary with the following reason:

 - lock_device_hotplug() now protects CPU online/offline operations,
   including the probe & release interfaces enabled by
   ARCH_CPU_PROBE_RELEASE.  The use of cpu_hotplug_driver_lock() is
   redundant.
 - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
   is defined, which is misleading and is only enabled on powerpc.

This patch removes the cpu_hotplug_driver_lock() interface.  As
a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
probe & release interface as intended.  There is no functional change
in this patch.

Signed-off-by: Toshi Kani 
Acked-by: Rafael J. Wysocki 
Reviewed-by: Nathan Fontenot 
---
Performed build test only on powerpc.
Resend: Rebased to linux-next of the PM tree (3.12-rc2).
---
 arch/powerpc/kernel/smp.c  |   12 -
 arch/powerpc/platforms/pseries/dlpar.c |   43 
 arch/x86/kernel/topology.c |2 -
 drivers/base/cpu.c |   15 ++-
 include/linux/cpu.h|   13 --
 5 files changed, 19 insertions(+), 66 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 8e59abc..930cd8a 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -844,18 +844,6 @@ void __cpu_die(unsigned int cpu)
smp_ops->cpu_die(cpu);
 }
 
-static DEFINE_MUTEX(powerpc_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock()
-{
-   mutex_lock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock()
-{
-   mutex_unlock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
 void cpu_die(void)
 {
if (ppc_md.cpu_die)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index 7cfdaae..a8fe5aa 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -404,46 +404,38 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
unsigned long drc_index;
int rc;
 
-   cpu_hotplug_driver_lock();
rc = strict_strtoul(buf, 0, &drc_index);
-   if (rc) {
-   rc = -EINVAL;
-   goto out;
-   }
+   if (rc)
+   return -EINVAL;
 
parent = of_find_node_by_path("/cpus");
-   if (!parent) {
-   rc = -ENODEV;
-   goto out;
-   }
+   if (!parent)
+   return -ENODEV;
 
dn = dlpar_configure_connector(drc_index, parent);
-   if (!dn) {
-   rc = -EINVAL;
-   goto out;
-   }
+   if (!dn)
+   return -EINVAL;
 
of_node_put(parent);
 
rc = dlpar_acquire_drc(drc_index);
if (rc) {
dlpar_free_cc_nodes(dn);
-   rc = -EINVAL;
-   goto out;
+   return -EINVAL;
}
 
rc = dlpar_attach_node(dn);
if (rc) {
dlpar_release_drc(drc_index);
dlpar_free_cc_nodes(dn);
-   goto out;
+   return rc;
}
 
rc = dlpar_online_cpu(dn);
-out:
-   cpu_hotplug_driver_unlock();
+   if (rc)
+   return rc;
 
-   return rc ? rc : count;
+   return count;
 }
 
 static int dlpar_offline_cpu(struct device_node *dn)
@@ -516,30 +508,27 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   cpu_hotplug_driver_lock();
rc = dlpar_offline_cpu(dn);
if (rc) {
of_node_put(dn);
-   rc = -EINVAL;
-   goto out;
+   return -EINVAL;
}
 
rc = dlpar_release_drc(*drc_index);
if (rc) {
of_node_put(dn);
-   goto out;
+   return rc;
}
 
rc = dlpar_detach_node(dn);
if (rc) {
dlpar_acquire_drc(*drc_index);
-   goto out;
+   return rc;
}
 
of_node_put(dn);
-out:
-   cpu_hotplug_driver_unlock();
-   return rc ? rc : count;
+
+   return count;
 }
 
 static int __init pseries_dlpar_init(void)
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index a3f35eb..649b010 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -66,7 +66,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
return -EINVAL;
 
lock_device_hotplug();
-   cpu_hotplug_driver_lock();
 
switch (action) {
case 0:
@@ -91,7 +90,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
ret = -EINVAL;
}
 
-   cpu_hotplug_driver_unlock();
unlock_device_hotplug();
 
return ret;
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
in