[patch 02/18] acpi memory hotplug cannot manage _CRS with plural resoureces

[patch 02/18] acpi memory hotplug cannot manage _CRS with plural resoureces

[akpm]

From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Current acpi memory hotplug just looks into the first entry of resources in
_CRS.  But, _CRS can contain plural resources.  So, if _CRS contains plural
resoureces, acpi memory hot add cannot add all memory.

With this patch, acpi memory hotplug can deal with Memory Device, whose
_CRS contains plural resources.

Tested on ia64 memory hotplug test envrionment (not emulation, uses alpha
version firmware which supports dynamic reconfiguration of NUMA.)

Note: Microsoft's Windows Server 2003 requires big (>4G)resoureces to be
      divided into small (<4G) resources. looks crazy, but not invalid.
      (See http://www.microsoft.com/whdc/system/pnppwr/hotadd/hotaddmem.mspx)
      For this reason, a firmware vendor who supports Windows writes plural
      resources in a _CRS even if they are contiguous.

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: "Brown, Len" <len.brown@intel.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 drivers/acpi/acpi_memhotplug.c |  112 +++++++++++++++++++++----------
 1 file changed, 77 insertions(+), 35 deletions(-)

diff -puN drivers/acpi/acpi_memhotplug.c~acpi-memory-hotplug-cannot-manage-_crs-with-plural-resoureces drivers/acpi/acpi_memhotplug.c
--- a/drivers/acpi/acpi_memhotplug.c~acpi-memory-hotplug-cannot-manage-_crs-with-plural-resoureces
+++ a/drivers/acpi/acpi_memhotplug.c
@@ -68,45 +68,75 @@ static struct acpi_driver acpi_memory_de
 		},
 };
 
+struct acpi_memory_info {
+	struct list_head list;
+	u64 start_addr;		/* Memory Range start physical addr */
+	u64 length;		/* Memory Range length */
+	unsigned short caching;	/* memory cache attribute */
+	unsigned short write_protect;	/* memory read/write attribute */
+	unsigned int enabled:1;
+};
+
 struct acpi_memory_device {
 	acpi_handle handle;
 	unsigned int state;	/* State of the memory device */
-	unsigned short caching;	/* memory cache attribute */
-	unsigned short write_protect;	/* memory read/write attribute */
-	u64 start_addr;		/* Memory Range start physical addr */
-	u64 length;		/* Memory Range length */
+	struct list_head res_list;
 };
 
+static acpi_status
+acpi_memory_get_resource(struct acpi_resource *resource, void *context)
+{
+	struct acpi_memory_device *mem_device = context;
+	struct acpi_resource_address64 address64;
+	struct acpi_memory_info *info, *new;
+	acpi_status status;
+
+	status = acpi_resource_to_address64(resource, &address64);
+	if (ACPI_FAILURE(status) ||
+	    (address64.resource_type != ACPI_MEMORY_RANGE))
+		return AE_OK;
+
+	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;
+			return AE_OK;
+		}
+	}
+
+	new = kzalloc(sizeof(struct acpi_memory_info), GFP_KERNEL);
+	if (!new)
+		return AE_ERROR;
+
+	INIT_LIST_HEAD(&new->list);
+	new->caching = address64.info.mem.caching;
+	new->write_protect = address64.info.mem.write_protect;
+	new->start_addr = address64.minimum;
+	new->length = address64.address_length;
+	list_add_tail(&new->list, &mem_device->res_list);
+
+	return AE_OK;
+}
+
 static int
 acpi_memory_get_device_resources(struct acpi_memory_device *mem_device)
 {
 	acpi_status status;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	struct acpi_resource *resource = NULL;
-	struct acpi_resource_address64 address64;
+	struct acpi_memory_info *info, *n;
 
 	ACPI_FUNCTION_TRACE("acpi_memory_get_device_resources");
 
-	/* Get the range from the _CRS */
-	status = acpi_get_current_resources(mem_device->handle, &buffer);
-	if (ACPI_FAILURE(status))
-		return_VALUE(-EINVAL);
-
-	resource = (struct acpi_resource *)buffer.pointer;
-	status = acpi_resource_to_address64(resource, &address64);
-	if (ACPI_SUCCESS(status)) {
-		if (address64.resource_type == ACPI_MEMORY_RANGE) {
-			/* Populate the structure */
-			mem_device->caching = address64.info.mem.caching;
-			mem_device->write_protect =
-			    address64.info.mem.write_protect;
-			mem_device->start_addr = address64.minimum;
-			mem_device->length = address64.address_length;
-		}
+	status = acpi_walk_resources(mem_device->handle, METHOD_NAME__CRS,
+				     acpi_memory_get_resource, mem_device);
+	if (ACPI_FAILURE(status)) {
+		list_for_each_entry_safe(info, n, &mem_device->res_list, list)
+			kfree(info);
+		return -EINVAL;
 	}
 
-	acpi_os_free(buffer.pointer);
-	return_VALUE(0);
+	return 0;
 }
 
 static int
@@ -181,7 +211,8 @@ static int acpi_memory_check_device(stru
 
 static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
 {
-	int result;
+	int result, num_enabled = 0;
+	struct acpi_memory_info *info;
 
 	ACPI_FUNCTION_TRACE("acpi_memory_enable_device");
 
@@ -197,12 +228,20 @@ static int acpi_memory_enable_device(str
 	/*
 	 * 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)
 	 */
-	result = add_memory(mem_device->start_addr, mem_device->length);
-	if (result) {
+	list_for_each_entry(info, &mem_device->res_list, list) {
+		result = add_memory(info->start_addr, info->length);
+		if (result)
+			continue;
+		info->enabled = 1;
+		num_enabled++;
+	}
+	if (!num_enabled) {
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "\nadd_memory failed\n"));
 		mem_device->state = MEMORY_INVALID_STATE;
-		return result;
+		return -EINVAL;
 	}
 
 	return result;
@@ -246,8 +285,7 @@ static int acpi_memory_powerdown_device(
 static int acpi_memory_disable_device(struct acpi_memory_device *mem_device)
 {
 	int result;
-	u64 start = mem_device->start_addr;
-	u64 len = mem_device->length;
+	struct acpi_memory_info *info, *n;
 
 	ACPI_FUNCTION_TRACE("acpi_memory_disable_device");
 
@@ -255,10 +293,13 @@ static int acpi_memory_disable_device(st
 	 * Ask the VM to offline this memory range.
 	 * Note: Assume that this function returns zero on success
 	 */
-	result = remove_memory(start, len);
-	if (result) {
-		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Hot-Remove failed.\n"));
-		return_VALUE(result);
+	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)
+				return result;
+		}
+		kfree(info);
 	}
 
 	/* Power-off and eject the device */
@@ -356,6 +397,7 @@ static int acpi_memory_device_add(struct
 		return_VALUE(-ENOMEM);
 	memset(mem_device, 0, sizeof(struct acpi_memory_device));
 
+	INIT_LIST_HEAD(&mem_device->res_list);
 	mem_device->handle = device->handle;
 	sprintf(acpi_device_name(device), "%s", ACPI_MEMORY_DEVICE_NAME);
 	sprintf(acpi_device_class(device), "%s", ACPI_MEMORY_DEVICE_CLASS);
_

Re: [patch 02/18] acpi memory hotplug cannot manage _CRS with plural resoureces

[Andrew Morton]

btw, this is, by my count, the seventh time I've sent this patch.  The first
was on March 21.

We have a whole stream of node hot-add patches banked up behind these three
patches:


[patch 02/18] acpi memory hotplug cannot manage _CRS with plural resoureces
[patch 03/18] Catch notification of memory add event of ACPI via container driver. (register start func for memory device)
[patch 04/18] Catch notification of memory add event of ACPI via container driver. (avoid redundant call add_memory)

and we want to get those blocked patches into 2.6.18.

So please review at least these three patches asap and if they're OK please
ack them for inclusion and I'll include them along with the node hotadd
patch series.

Thanks.

RE: [patch 02/18] acpi memory hotplug cannot manage _CRS with plural resoureces

[Brown, Len]

Andrew,
Thanks for forwarding this series to Linus.

>btw, this is, by my count, the seventh time I've sent this patch.

what is your point?

> The first was on March 21.

March 21st was the 1st day of the 2.6.17 integration window,
so unless this series deserved very little testing, it
missed 2.6.17 by definition.

Also, I couldn't apply this patch series because it depended
on other changes in -mm.  Yes, I should have Acked it for you.
However, when it didn't apply I stopped looking at it.

June 17th, 2.6.18 opened

You send this series to Linus today.
So given the current process and the 8-week 2.6.17 cycle,
this patch made it into Linus' tree within days of the speed of light.

I'm happy to work with you on improving the process.
I think I could have acked this one for you, but that
is about it.

thanks,
-Len

Re: [patch 02/18] acpi memory hotplug cannot manage _CRS with plural resoureces

[Andrew Morton]

On Wed, 28 Jun 2006 02:13:26 -0400
"Brown, Len" <len.brown@intel.com> wrote:

> Andrew,
> Thanks for forwarding this series to Linus.
> 
> >btw, this is, by my count, the seventh time I've sent this patch.
> 
> what is your point?

That when someone sends someone else a patch, it's a request that it be
merged.  Or commented on.  Or nacked-with-reason.  Or acked.  Or something.
 The sender doesn't expect or want zero response from seven attempts across
three months!

> > The first was on March 21.
> 
> March 21st was the 1st day of the 2.6.17 integration window,
> so unless this series deserved very little testing, it
> missed 2.6.17 by definition.
> 
> Also, I couldn't apply this patch series because it depended
> on other changes in -mm.  Yes, I should have Acked it for you.
> However, when it didn't apply I stopped looking at it.

The patch was against the current acpi devel tree.  I always send patches
against the recipient's tree.  Perhaps there were earlier sent-to-you-also
patches in that series upon which it had dependencies.

> June 17th, 2.6.18 opened
> 
> You send this series to Linus today.
> So given the current process and the 8-week 2.6.17 cycle,
> this patch made it into Linus' tree within days of the speed of light.
> 
> I'm happy to work with you on improving the process.
> I think I could have acked this one for you, but that
> is about it.

When I send you a patch, unless I specifically mark it as for-review or
something then please treat is as a request to merge.  Unless it's urgent
(like a bugfix) then some action within a couple of weeks is fine.  Action
being merge, nack-with-reason, request-for-modification, please-merge-it-yourself
etc.  Something dispositive.

Thanks.