On Thursday, December 13, 2012 10:36:38 AM Jiang Liu wrote: > On 2012-12-12 22:37, Liu, Jinsong wrote: > > Wen Congyang wrote: > >> At 12/08/2012 06:19 AM, Rafael J. Wysocki Wrote: > >>> On Tuesday, December 04, 2012 01:39:54 AM Liu, Jinsong wrote: > >>>> Resend it, add Rafael and linux-acpi@xxxxxxxxxxxxxxx > >>> > >>> I wonder what memory hotplug people think about that. > >>> > >>> Thanks, > >>> Rafael > >>> > >>> > >>>> =============== > >>>> From 1d39279e45c54ce531691da5ffe261e7689dd92c Mon Sep 17 00:00:00 > >>>> 2001 > >>>> From: Liu Jinsong <jinsong.liu@xxxxxxxxx> > >>>> Date: Wed, 14 Nov 2012 18:52:06 +0800 > >>>> Subject: [PATCH] X86/acpi: remove redundant logic of acpi memory > >>>> hotadd > >>>> > >>>> When memory hotadd, acpi_memory_enable_device has already been done > >>>> at drv->ops.add (acpi_memory_device_add), no need to do it again > >>>> at notify callback. > >>>> > >>>> At acpi_memory_enable_device, acpi_memory_get_device_resources > >>>> is also a redundant action, since it has been done at drv->ops.add. > >>>> > >>>> Signed-off-by: Liu Jinsong <jinsong.liu@xxxxxxxxx> > >>>> --- > >>>> drivers/acpi/acpi_memhotplug.c | 17 ----------------- > >>>> 1 files changed, 0 insertions(+), 17 deletions(-) > >>>> > >>>> diff --git a/drivers/acpi/acpi_memhotplug.c > >>>> b/drivers/acpi/acpi_memhotplug.c > >>>> index 24c807f..a6489fd 100644 > >>>> --- a/drivers/acpi/acpi_memhotplug.c > >>>> +++ b/drivers/acpi/acpi_memhotplug.c > >>>> @@ -220,15 +220,6 @@ static int acpi_memory_enable_device(struct > >>>> acpi_memory_device *mem_device) struct acpi_memory_info *info; > >>>> int node; > >>>> > >>>> - > >>>> - /* Get the range from the _CRS */ > >>>> - result = acpi_memory_get_device_resources(mem_device); > >>>> - if (result) { > >>>> - printk(KERN_ERR PREFIX "get_device_resources failed\n"); > >>>> - mem_device->state = MEMORY_INVALID_STATE; > >>>> - return result; > >>>> - } > >>>> - > >>>> node = acpi_get_node(mem_device->device->handle); /* > >>>> * Tell the VM there is more memory here... > >>>> @@ -357,14 +348,6 @@ static void > >>>> acpi_memory_device_notify(acpi_handle handle, u32 event, void > >>>> *data) break; } > >>>> > >>>> - if (acpi_memory_check_device(mem_device)) > >>>> - break; > >> > >> Hmm, if acpi_memory_check_device() fails, it means the memory device > >> disappears > >> I don't know if a real hardware uses this way to remove memory device. > >> > >>>> - > >>>> - if (acpi_memory_enable_device(mem_device)) { > >>>> - printk(KERN_ERR PREFIX "Cannot enable memory device\n"); > >>>> - break; > >>>> - } > >> > >> If acpi_memory_get_device() doesn't fail, it means that the device > >> has been managed by this driver, so I think we can do this cleanup. > >> > >> Thanks > >> Wen Congyang > >> > > > > Thanks! any comments from Huawei side, Jiang? > Hi Jinsong, > > We think it's ok. > > acpi_memory_device_notify > acpi_memory_get_device > acpi_memory_device_add > acpi_memory_get_device_resources > acpi_memory_enable_device > acpi_memory_get_device_resources(redundant) > acpi_memory_check_device(redundant) > acpi_memory_enable_device(redundant) OK, thanks. I'll queue it up for submission as a fix later in the cycle. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html