The memory device can be removed by 2 ways:
1. send eject request by SCI
2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
In the 1st case, acpi_memory_disable_device() will be called.
In the 2nd case, acpi_memory_device_remove() will be called.
acpi_memory_device_remove() will also be called when we unbind the
memory device from the driver acpi_memhotplug or a driver initialization
fails.
acpi_memory_disable_device() has already implemented a code which
offlines memory and releases acpi_memory_info struct . But
acpi_memory_device_remove() has not implemented it yet.
So the patch prepares the framework for hot removing memory and
adds the framework into acpi_memory_device_remove().
The last version of this patchset is here:
https://lkml.org/lkml/2012/10/26/175
Note: patch1-2 are in pm tree now. And there is a bug in patch1, so I resend
them. The commit in pm tree is:
patch1: 85fcb3758c10e063a2a30dfad75017097999deed
patch2: d0fbb400b6f3a6191bdc5024f8733b2e2b86338f
Changes from v3 to v4:
1. patch1: unlock list_lock when removing memory fails.
2. patch2: just rebase them
3. patch3-7: these patches are in -mm tree, and they conflict with this
patchset, so Adrew Morton drop them from -mm tree. I rebase and merge
them into this patchset.
Wen Congyang (6):
acpi,memory-hotplug: introduce a mutex lock to protect the list in
acpi_memory_device
acpi_memhotplug.c: fix memory leak when memory device is unbound from
the module acpi_memhotplug
acpi_memhotplug.c: free memory device if acpi_memory_enable_device()
failed
acpi_memhotplug.c: don't allow to eject the memory device if it is
being used
acpi_memhotplug.c: bind the memory device when the driver is being
loaded
acpi_memhotplug.c: auto bind the memory device which is hotplugged
before the driver is loaded
Yasuaki Ishimatsu (1):
acpi,memory-hotplug : add memory offline code to
acpi_memory_device_remove()
drivers/acpi/acpi_memhotplug.c | 168 ++++++++++++++++++++++++++++++++---------
1 file changed, 133 insertions(+), 35 deletions(-)
--
1.8.0
From: Yasuaki Ishimatsu <[email protected]>
The memory device can be removed by 2 ways:
1. send eject request by SCI
2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
In the 1st case, acpi_memory_disable_device() will be called.
In the 2nd case, acpi_memory_device_remove() will be called.
acpi_memory_device_remove() will also be called when we unbind the
memory device from the driver acpi_memhotplug or a driver initialization
fails.
acpi_memory_disable_device() has already implemented a code which
offlines memory and releases acpi_memory_info struct. But
acpi_memory_device_remove() has not implemented it yet.
So the patch move offlining memory and releasing acpi_memory_info struct
codes to a new function acpi_memory_remove_memory(). And it is used by both
acpi_memory_device_remove() and acpi_memory_disable_device().
CC: David Rientjes <[email protected]>
CC: Jiang Liu <[email protected]>
CC: Len Brown <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Christoph Lameter <[email protected]>
Cc: Minchan Kim <[email protected]>
CC: Andrew Morton <[email protected]>
CC: KOSAKI Motohiro <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Yasuaki Ishimatsu <[email protected]>
Signed-off-by: Wen Congyang <[email protected]>
---
The commit for pm tree is d0fbb400
drivers/acpi/acpi_memhotplug.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 4c18ee3..e573e87 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -316,16 +316,11 @@ static int acpi_memory_powerdown_device(struct acpi_memory_device *mem_device)
return 0;
}
-static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
+static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
{
int result;
struct acpi_memory_info *info, *n;
-
- /*
- * Ask the VM to offline this memory range.
- * Note: Assume that this function returns zero on success
- */
mutex_lock(&mem_device->list_lock);
list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
if (info->enabled) {
@@ -335,10 +330,27 @@ static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
return result;
}
}
+
+ list_del(&info->list);
kfree(info);
}
mutex_unlock(&mem_device->list_lock);
+ return 0;
+}
+
+static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
+{
+ int result;
+
+ /*
+ * Ask the VM to offline this memory range.
+ * Note: Assume that this function returns zero on success
+ */
+ result = acpi_memory_remove_memory(mem_device);
+ if (result)
+ return result;
+
/* Power-off and eject the device */
result = acpi_memory_powerdown_device(mem_device);
if (result) {
@@ -489,12 +501,17 @@ static int acpi_memory_device_add(struct acpi_device *device)
static int acpi_memory_device_remove(struct acpi_device *device, int type)
{
struct acpi_memory_device *mem_device = NULL;
-
+ int result;
if (!device || !acpi_driver_data(device))
return -EINVAL;
mem_device = acpi_driver_data(device);
+
+ result = acpi_memory_remove_memory(mem_device);
+ if (result)
+ return result;
+
kfree(mem_device);
return 0;
--
1.8.0
The memory device can be removed by 2 ways:
1. send eject request by SCI
2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
This 2 events may happen at the same time, so we may touch
acpi_memory_device.res_list at the same time. This patch
introduce a lock to protect this list.
CC: David Rientjes <[email protected]>
CC: Jiang Liu <[email protected]>
CC: Len Brown <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Christoph Lameter <[email protected]>
Cc: Minchan Kim <[email protected]>
CC: Andrew Morton <[email protected]>
CC: KOSAKI Motohiro <[email protected]>
CC: Yasuaki Ishimatsu <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Wen Congyang <[email protected]>
---
The commit in pm tree is 85fcb375
drivers/acpi/acpi_memhotplug.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 1e90e8f..4c18ee3 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -83,7 +83,8 @@ struct acpi_memory_info {
struct acpi_memory_device {
struct acpi_device * device;
unsigned int state; /* State of the memory device */
- struct list_head res_list;
+ struct mutex list_lock;
+ struct list_head res_list; /* protected by list_lock */
};
static int acpi_hotmem_initialized;
@@ -101,19 +102,23 @@ acpi_memory_get_resource(struct acpi_resource *resource, void *context)
(address64.resource_type != ACPI_MEMORY_RANGE))
return AE_OK;
+ mutex_lock(&mem_device->list_lock);
list_for_each_entry(info, &mem_device->res_list, list) {
/* Can we combine the resource range information? */
if ((info->caching == address64.info.mem.caching) &&
(info->write_protect == address64.info.mem.write_protect) &&
(info->start_addr + info->length == address64.minimum)) {
info->length += address64.address_length;
+ mutex_unlock(&mem_device->list_lock);
return AE_OK;
}
}
new = kzalloc(sizeof(struct acpi_memory_info), GFP_KERNEL);
- if (!new)
+ if (!new) {
+ mutex_unlock(&mem_device->list_lock);
return AE_ERROR;
+ }
INIT_LIST_HEAD(&new->list);
new->caching = address64.info.mem.caching;
@@ -121,6 +126,7 @@ acpi_memory_get_resource(struct acpi_resource *resource, void *context)
new->start_addr = address64.minimum;
new->length = address64.address_length;
list_add_tail(&new->list, &mem_device->res_list);
+ mutex_unlock(&mem_device->list_lock);
return AE_OK;
}
@@ -138,9 +144,11 @@ acpi_memory_get_device_resources(struct acpi_memory_device *mem_device)
status = acpi_walk_resources(mem_device->device->handle, METHOD_NAME__CRS,
acpi_memory_get_resource, mem_device);
if (ACPI_FAILURE(status)) {
+ mutex_lock(&mem_device->list_lock);
list_for_each_entry_safe(info, n, &mem_device->res_list, list)
kfree(info);
INIT_LIST_HEAD(&mem_device->res_list);
+ mutex_unlock(&mem_device->list_lock);
return -EINVAL;
}
@@ -236,6 +244,7 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
* We don't have memory-hot-add rollback function,now.
* (i.e. memory-hot-remove function)
*/
+ mutex_lock(&mem_device->list_lock);
list_for_each_entry(info, &mem_device->res_list, list) {
if (info->enabled) { /* just sanity check...*/
num_enabled++;
@@ -256,6 +265,7 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
info->enabled = 1;
num_enabled++;
}
+ mutex_unlock(&mem_device->list_lock);
if (!num_enabled) {
printk(KERN_ERR PREFIX "add_memory failed\n");
mem_device->state = MEMORY_INVALID_STATE;
@@ -316,14 +326,18 @@ static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
* Ask the VM to offline this memory range.
* Note: Assume that this function returns zero on success
*/
+ mutex_lock(&mem_device->list_lock);
list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
if (info->enabled) {
result = remove_memory(info->start_addr, info->length);
- if (result)
+ if (result) {
+ mutex_unlock(&mem_device->list_lock);
return result;
+ }
}
kfree(info);
}
+ mutex_unlock(&mem_device->list_lock);
/* Power-off and eject the device */
result = acpi_memory_powerdown_device(mem_device);
@@ -438,6 +452,7 @@ static int acpi_memory_device_add(struct acpi_device *device)
mem_device->device = device;
sprintf(acpi_device_name(device), "%s", ACPI_MEMORY_DEVICE_NAME);
sprintf(acpi_device_class(device), "%s", ACPI_MEMORY_DEVICE_CLASS);
+ mutex_init(&mem_device->list_lock);
device->driver_data = mem_device;
/* Get the range from the _CRS */
--
1.8.0
We eject the memory device even if it is in use. It is very dangerous,
and it will cause the kernel to be panicked.
CC: David Rientjes <[email protected]>
CC: Jiang Liu <[email protected]>
CC: Len Brown <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Christoph Lameter <[email protected]>
Cc: Minchan Kim <[email protected]>
CC: Andrew Morton <[email protected]>
CC: KOSAKI Motohiro <[email protected]>
CC: Yasuaki Ishimatsu <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Wen Congyang <[email protected]>
---
drivers/acpi/acpi_memhotplug.c | 46 +++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 8914399..1fb1342 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -78,6 +78,7 @@ struct acpi_memory_info {
unsigned short caching; /* memory cache attribute */
unsigned short write_protect; /* memory read/write attribute */
unsigned int enabled:1;
+ unsigned int failed:1;
};
struct acpi_memory_device {
@@ -266,9 +267,23 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
node = memory_add_physaddr_to_nid(info->start_addr);
result = add_memory(node, info->start_addr, info->length);
- if (result)
+
+ /*
+ * If the memory block has been used by the kernel, add_memory()
+ * returns -EEXIST. If add_memory() returns the other error, it
+ * means that this memory block is not used by the kernel.
+ */
+ if (result && result != -EEXIST) {
+ info->failed = 1;
continue;
- info->enabled = 1;
+ }
+
+ if (!result)
+ info->enabled = 1;
+ /*
+ * Add num_enable even if add_memory() returns -EEXIST, so the
+ * device is bound to this driver.
+ */
num_enabled++;
}
mutex_unlock(&mem_device->list_lock);
@@ -324,25 +339,36 @@ static int acpi_memory_powerdown_device(struct acpi_memory_device *mem_device)
static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
{
- int result;
+ int result = 0;
struct acpi_memory_info *info, *n;
mutex_lock(&mem_device->list_lock);
list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
- if (info->enabled) {
- result = remove_memory(info->start_addr, info->length);
- if (result) {
- mutex_unlock(&mem_device->list_lock);
- return result;
- }
+ if (info->failed)
+ /* The kernel does not use this memory block */
+ continue;
+
+ if (!info->enabled) {
+ /*
+ * The kernel uses this memory block, but it may be not
+ * managed by us.
+ */
+ result = -EBUSY;
+ goto out;
}
+ result = remove_memory(info->start_addr, info->length);
+ if (result)
+ goto out;
+
list_del(&info->list);
kfree(info);
}
+
+out:
mutex_unlock(&mem_device->list_lock);
- return 0;
+ return result;
}
static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
--
1.8.0
If the memory device is hotplugged before the driver is loaded, the user
cannot see this device under the directory /sys/bus/acpi/devices/, and the
user cannot bind it by hand after the driver is loaded. This patch
introduces a new feature to bind such device when the driver is being
loaded.
CC: David Rientjes <[email protected]>
CC: Jiang Liu <[email protected]>
CC: Len Brown <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Christoph Lameter <[email protected]>
Cc: Minchan Kim <[email protected]>
CC: Andrew Morton <[email protected]>
CC: KOSAKI Motohiro <[email protected]>
CC: Yasuaki Ishimatsu <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Wen Congyang <[email protected]>
---
drivers/acpi/acpi_memhotplug.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 8a8716f..24bfa6e 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -52,6 +52,9 @@ MODULE_LICENSE("GPL");
#define MEMORY_POWER_ON_STATE 1
#define MEMORY_POWER_OFF_STATE 2
+static bool auto_probe;
+module_param(auto_probe, bool, S_IRUGO | S_IWUSR);
+
static int acpi_memory_device_add(struct acpi_device *device);
static int acpi_memory_device_remove(struct acpi_device *device, int type);
@@ -581,12 +584,44 @@ acpi_memory_register_notify_handler(acpi_handle handle,
u32 level, void *ctxt, void **retv)
{
acpi_status status;
-
+ struct acpi_memory_device *mem_device = NULL;
+ unsigned long long current_status;
status = is_memory_device(handle);
if (ACPI_FAILURE(status))
return AE_OK; /* continue */
+ if (auto_probe) {
+ /* Get device present/absent information from the _STA */
+ status = acpi_evaluate_integer(handle, "_STA", NULL,
+ ¤t_status);
+ if (ACPI_FAILURE(status))
+ goto install;
+
+ /*
+ * Check for device status. Device should be
+ * present/enabled/functioning.
+ */
+ if (!(current_status &
+ (ACPI_STA_DEVICE_PRESENT | ACPI_STA_DEVICE_ENABLED |
+ ACPI_STA_DEVICE_FUNCTIONING)))
+ goto install;
+
+ if (acpi_memory_get_device(handle, &mem_device))
+ goto install;
+
+ /* We have bound this device while we register the driver */
+ if (mem_device->state == MEMORY_POWER_ON_STATE)
+ goto install;
+
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+ "\nauto probe memory device\n"));
+
+ if (acpi_memory_enable_device(mem_device))
+ pr_err(PREFIX "Cannot enable memory device\n");
+ }
+
+install:
status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
acpi_memory_device_notify, NULL);
/* continue */
--
1.8.0
If acpi_memory_enable_device() fails, acpi_memory_enable_device() will
return a non-zero value, which means we fail to bind the memory device to
this driver. So we should free memory device before
acpi_memory_device_add() returns.
CC: David Rientjes <[email protected]>
CC: Jiang Liu <[email protected]>
CC: Len Brown <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Christoph Lameter <[email protected]>
Cc: Minchan Kim <[email protected]>
CC: Andrew Morton <[email protected]>
CC: KOSAKI Motohiro <[email protected]>
CC: Yasuaki Ishimatsu <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Wen Congyang <[email protected]>
---
drivers/acpi/acpi_memhotplug.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 5e5ac80..8914399 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -506,9 +506,11 @@ static int acpi_memory_device_add(struct acpi_device *device)
if (!acpi_memory_check_device(mem_device)) {
/* call add_memory func */
result = acpi_memory_enable_device(mem_device);
- if (result)
+ if (result) {
printk(KERN_ERR PREFIX
"Error in acpi_memory_enable_device\n");
+ acpi_memory_device_free(mem_device);
+ }
}
return result;
}
--
1.8.0
We had introduced acpi_hotmem_initialized to avoid strange add_memory fail
message. But the memory device may not be used by the kernel, and the
device should be bound when the driver is being loaded. Remove
acpi_hotmem_initialized to allow that the device can be bound when the
driver is being loaded.
CC: David Rientjes <[email protected]>
CC: Jiang Liu <[email protected]>
CC: Len Brown <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Christoph Lameter <[email protected]>
Cc: Minchan Kim <[email protected]>
CC: Andrew Morton <[email protected]>
CC: KOSAKI Motohiro <[email protected]>
CC: Yasuaki Ishimatsu <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Wen Congyang <[email protected]>
---
drivers/acpi/acpi_memhotplug.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 1fb1342..8a8716f 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -88,8 +88,6 @@ struct acpi_memory_device {
struct list_head res_list; /* protected by list_lock */
};
-static int acpi_hotmem_initialized;
-
static acpi_status
acpi_memory_get_resource(struct acpi_resource *resource, void *context)
{
@@ -520,15 +518,6 @@ static int acpi_memory_device_add(struct acpi_device *device)
printk(KERN_DEBUG "%s \n", acpi_device_name(device));
- /*
- * Early boot code has recognized memory area by EFI/E820.
- * If DSDT shows these memory devices on boot, hotplug is not necessary
- * for them. So, it just returns until completion of this driver's
- * start up.
- */
- if (!acpi_hotmem_initialized)
- return 0;
-
if (!acpi_memory_check_device(mem_device)) {
/* call add_memory func */
result = acpi_memory_enable_device(mem_device);
@@ -644,7 +633,6 @@ static int __init acpi_memory_device_init(void)
return -ENODEV;
}
- acpi_hotmem_initialized = 1;
return 0;
}
--
1.8.0
We allocate memory to store acpi_memory_info, so we should free it before
freeing mem_device.
CC: David Rientjes <[email protected]>
CC: Jiang Liu <[email protected]>
CC: Len Brown <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Christoph Lameter <[email protected]>
Cc: Minchan Kim <[email protected]>
CC: Andrew Morton <[email protected]>
CC: KOSAKI Motohiro <[email protected]>
CC: Yasuaki Ishimatsu <[email protected]>
CC: Rafael J. Wysocki <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
Signed-off-by: Wen Congyang <[email protected]>
---
drivers/acpi/acpi_memhotplug.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index e573e87..5e5ac80 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -131,12 +131,22 @@ acpi_memory_get_resource(struct acpi_resource *resource, void *context)
return AE_OK;
}
+static void
+acpi_memory_free_device_resources(struct acpi_memory_device *mem_device)
+{
+ struct acpi_memory_info *info, *n;
+
+ mutex_lock(&mem_device->list_lock);
+ list_for_each_entry_safe(info, n, &mem_device->res_list, list)
+ kfree(info);
+ INIT_LIST_HEAD(&mem_device->res_list);
+ mutex_unlock(&mem_device->list_lock);
+}
+
static int
acpi_memory_get_device_resources(struct acpi_memory_device *mem_device)
{
acpi_status status;
- struct acpi_memory_info *info, *n;
-
if (!list_empty(&mem_device->res_list))
return 0;
@@ -144,11 +154,7 @@ acpi_memory_get_device_resources(struct acpi_memory_device *mem_device)
status = acpi_walk_resources(mem_device->device->handle, METHOD_NAME__CRS,
acpi_memory_get_resource, mem_device);
if (ACPI_FAILURE(status)) {
- mutex_lock(&mem_device->list_lock);
- list_for_each_entry_safe(info, n, &mem_device->res_list, list)
- kfree(info);
- INIT_LIST_HEAD(&mem_device->res_list);
- mutex_unlock(&mem_device->list_lock);
+ acpi_memory_free_device_resources(mem_device);
return -EINVAL;
}
@@ -447,6 +453,15 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
return;
}
+static void acpi_memory_device_free(struct acpi_memory_device *mem_device)
+{
+ if (!mem_device)
+ return;
+
+ acpi_memory_free_device_resources(mem_device);
+ kfree(mem_device);
+}
+
static int acpi_memory_device_add(struct acpi_device *device)
{
int result;
@@ -512,7 +527,7 @@ static int acpi_memory_device_remove(struct acpi_device *device, int type)
if (result)
return result;
- kfree(mem_device);
+ acpi_memory_device_free(mem_device);
return 0;
}
--
1.8.0
On Thu, 2012-11-08 at 19:04 +0800, Wen Congyang wrote:
> The memory device can be removed by 2 ways:
> 1. send eject request by SCI
> 2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
>
> This 2 events may happen at the same time, so we may touch
> acpi_memory_device.res_list at the same time. This patch
> introduce a lock to protect this list.
Hi Wen,
This race condition is not unique in memory hot-remove as the sysfs
eject interface is created for all objects with _EJ0. For CPU
hot-remove, I addressed this race condition by making the notify handler
to run the hot-remove operation on kacpi_hotplug_wq by calling
acpi_os_hotplug_execute(). This serializes the hot-remove operations
among the two events since the sysfs eject also runs on
kacpi_hotplug_wq. This way is much simpler and is easy to maintain,
although it does not allow both operations to run simultaneously (which
I do not think we need). Can it be used for memory hot-remove as well?
Thanks,
-Toshi
At 11/13/2012 05:00 AM, Toshi Kani Wrote:
> On Thu, 2012-11-08 at 19:04 +0800, Wen Congyang wrote:
>> The memory device can be removed by 2 ways:
>> 1. send eject request by SCI
>> 2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
>>
>> This 2 events may happen at the same time, so we may touch
>> acpi_memory_device.res_list at the same time. This patch
>> introduce a lock to protect this list.
>
> Hi Wen,
>
> This race condition is not unique in memory hot-remove as the sysfs
> eject interface is created for all objects with _EJ0. For CPU
> hot-remove, I addressed this race condition by making the notify handler
> to run the hot-remove operation on kacpi_hotplug_wq by calling
> acpi_os_hotplug_execute(). This serializes the hot-remove operations
> among the two events since the sysfs eject also runs on
> kacpi_hotplug_wq. This way is much simpler and is easy to maintain,
> although it does not allow both operations to run simultaneously (which
> I do not think we need). Can it be used for memory hot-remove as well?
Good idea. I will update it.
Thanks
Wen Congyang
>
> Thanks,
> -Toshi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Tuesday, November 13, 2012 10:04:53 AM Wen Congyang wrote:
> At 11/13/2012 05:00 AM, Toshi Kani Wrote:
> > On Thu, 2012-11-08 at 19:04 +0800, Wen Congyang wrote:
> >> The memory device can be removed by 2 ways:
> >> 1. send eject request by SCI
> >> 2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> >>
> >> This 2 events may happen at the same time, so we may touch
> >> acpi_memory_device.res_list at the same time. This patch
> >> introduce a lock to protect this list.
> >
> > Hi Wen,
> >
> > This race condition is not unique in memory hot-remove as the sysfs
> > eject interface is created for all objects with _EJ0. For CPU
> > hot-remove, I addressed this race condition by making the notify handler
> > to run the hot-remove operation on kacpi_hotplug_wq by calling
> > acpi_os_hotplug_execute(). This serializes the hot-remove operations
> > among the two events since the sysfs eject also runs on
> > kacpi_hotplug_wq. This way is much simpler and is easy to maintain,
> > although it does not allow both operations to run simultaneously (which
> > I do not think we need). Can it be used for memory hot-remove as well?
>
> Good idea. I will update it.
Still waiting. :-)
But if you want that in v3.8, please repost ASAP.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
At 11/15/2012 07:34 AM, Rafael J. Wysocki Wrote:
> On Tuesday, November 13, 2012 10:04:53 AM Wen Congyang wrote:
>> At 11/13/2012 05:00 AM, Toshi Kani Wrote:
>>> On Thu, 2012-11-08 at 19:04 +0800, Wen Congyang wrote:
>>>> The memory device can be removed by 2 ways:
>>>> 1. send eject request by SCI
>>>> 2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
>>>>
>>>> This 2 events may happen at the same time, so we may touch
>>>> acpi_memory_device.res_list at the same time. This patch
>>>> introduce a lock to protect this list.
>>>
>>> Hi Wen,
>>>
>>> This race condition is not unique in memory hot-remove as the sysfs
>>> eject interface is created for all objects with _EJ0. For CPU
>>> hot-remove, I addressed this race condition by making the notify handler
>>> to run the hot-remove operation on kacpi_hotplug_wq by calling
>>> acpi_os_hotplug_execute(). This serializes the hot-remove operations
>>> among the two events since the sysfs eject also runs on
>>> kacpi_hotplug_wq. This way is much simpler and is easy to maintain,
>>> although it does not allow both operations to run simultaneously (which
>>> I do not think we need). Can it be used for memory hot-remove as well?
>>
>> Good idea. I will update it.
>
> Still waiting. :-)
>
> But if you want that in v3.8, please repost ASAP.
I think I will send it today. It is in test now.
Thanks
Wen Congyang
>
> Thanks,
> Rafael
>
>