[PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

Resend it, add Rafael and linux-acpi@vger.kernel.org

Thanks,
Jinsong

===============
>From 1d39279e45c54ce531691da5ffe261e7689dd92c Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
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@intel.com>
---
 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;
-
-		if (acpi_memory_enable_device(mem_device)) {
-			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
-			break;
-		}
-
 		ost_code = ACPI_OST_SC_SUCCESS;
 		break;
 
-- 
1.7.1

[-- Attachment #2: 0001-X86-acpi-remove-redundant-logic-of-acpi-memory-hotad.patch --]
[-- Type: application/octet-stream, Size: 1705 bytes --]

From 1d39279e45c54ce531691da5ffe261e7689dd92c Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
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@intel.com>
---
 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;
-
-		if (acpi_memory_enable_device(mem_device)) {
-			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
-			break;
-		}
-
 		ost_code = ACPI_OST_SC_SUCCESS;
 		break;
 
-- 
1.7.1

RE: [PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Liu, Jinsong]

Ping?

Thanks,
Jinsong


Liu, Jinsong wrote:
> Resend it, add Rafael and linux-acpi@vger.kernel.org
> 
> Thanks,
> Jinsong
> 
> ===============
> From 1d39279e45c54ce531691da5ffe261e7689dd92c Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> 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@intel.com>
> ---
>  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;
> -
> -		if (acpi_memory_enable_device(mem_device)) {
> -			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
> -			break;
> -		}
> -
>  		ost_code = ACPI_OST_SC_SUCCESS;
>  		break;

Re: [PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Rafael J. Wysocki]

On Tuesday, December 04, 2012 01:39:54 AM Liu, Jinsong wrote:
> Resend it, add Rafael and linux-acpi@vger.kernel.org

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@intel.com>
> 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@intel.com>
> ---
>  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;
> -
> -		if (acpi_memory_enable_device(mem_device)) {
> -			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
> -			break;
> -		}
> -
>  		ost_code = ACPI_OST_SC_SUCCESS;
>  		break;
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Wen Congyang]

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@vger.kernel.org
> 
> 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@intel.com>
>> 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@intel.com>
>> ---
>>  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

>> -
>>  		ost_code = ACPI_OST_SC_SUCCESS;
>>  		break;
>>  
>>

RE: [PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Liu, Jinsong]

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@vger.kernel.org
>> 
>> 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@intel.com>
>>> 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@intel.com>
>>> ---
>>>  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?

Re: [PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Jiang Liu]

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@vger.kernel.org
>>>
>>> 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@intel.com>
>>>> 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@intel.com>
>>>> ---
>>>>  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）

Re: [PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Rafael J. Wysocki]

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@vger.kernel.org
> >>>
> >>> 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@intel.com>
> >>>> 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@intel.com>
> >>>> ---
> >>>>  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.

RE: [PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Liu, Jinsong]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 3521 bytes --]

Rafael J. Wysocki wrote:
> 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@vger.kernel.org
>>>>> 
>>>>> 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@intel.com>
>>>>>> 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@intel.com>
>>>>>> ---
>>>>>>  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

Thank you all!

Jinsongÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH] X86/acpi: remove redundant logic of acpi memory hotadd

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]

>From 1d39279e45c54ce531691da5ffe261e7689dd92c Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
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@intel.com>
---
 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;
-
-		if (acpi_memory_enable_device(mem_device)) {
-			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
-			break;
-		}
-
 		ost_code = ACPI_OST_SC_SUCCESS;
 		break;
 
-- 
1.7.1

[-- Attachment #2: 0001-X86-acpi-remove-redundant-logic-of-acpi-memory-hotad.patch --]
[-- Type: application/octet-stream, Size: 1705 bytes --]

From 1d39279e45c54ce531691da5ffe261e7689dd92c Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
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@intel.com>
---
 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;
-
-		if (acpi_memory_enable_device(mem_device)) {
-			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
-			break;
-		}
-
 		ost_code = ACPI_OST_SC_SUCCESS;
 		break;
 
-- 
1.7.1