ACPI,acpi_memhotplug: enable acpi_memory_info->enabled of memory device presented at boot-time

At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread, Toshi Kani mentioned as follows:

"I have a question about the change you made in commit 65479472 in acpi_memhotplug.c. This change seems to require that acpi_memory_enable_device() calls add_memory() to add all memory ranges represented by memory device objects at boot-time, and keep the results be used for hot-remove.

If I understand it right, this add_memory() call fails with EEXIST at boot-time since all memory ranges should have been added from EFI memory table (or e820) already. This results all memory ranges be marked as ! enabled & !failed. I think this means that we cannot hot-delete any memory ranges presented at boot-time since acpi_memory_remove_memory() only calls remove_memory() when the enabled flag is set. Is that correct?"

Above mention is correct. Thus even if memory device supports hotplug, memory presented at boot-time cannot be hot removed since the memory device's acpi_memory_info->enabled is always 0.

This patch changes to set 1 to "acpi_memory_info->enabled" of memory device presented at boot-time for hot removing the memory device.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

---
 drivers/acpi/acpi_memhotplug.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/acpi/acpi_memhotplug.c +0 -0

index da1f82b..88fd46a 100644

Quoted 38 lines · Yasuaki Ishimatsu

At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread, Toshi Kani mentioned as follows:

"I have a question about the change you made in commit 65479472 in acpi_memhotplug.c. This change seems to require that acpi_memory_enable_device() calls add_memory() to add all memory ranges represented by memory device objects at boot-time, and keep the results be used for hot-remove.

If I understand it right, this add_memory() call fails with EEXIST at boot-time since all memory ranges should have been added from EFI memory table (or e820) already. This results all memory ranges be marked as ! enabled & !failed. I think this means that we cannot hot-delete any memory ranges presented at boot-time since acpi_memory_remove_memory() only calls remove_memory() when the enabled flag is set. Is that correct?"

Above mention is correct. Thus even if memory device supports hotplug, memory presented at boot-time cannot be hot removed since the memory device's acpi_memory_info->enabled is always 0.

This patch changes to set 1 to "acpi_memory_info->enabled" of memory device presented at boot-time for hot removing the memory device.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

---
  drivers/acpi/acpi_memhotplug.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

drivers/acpi/acpi_memhotplug.c +0 -0

index da1f82b..88fd46a 100644
@@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
  			continue;
  		}
 

drivers/acpi/acpi_memhotplug.c static int acpi_memory_enable_device(struct acpi_memory_device *mem_device) · line 257

Quoted 3 lines

drivers/acpi/acpi_memhotplug.c +1 -2

-		if (!result)
-			info->enabled = 1;
+		info->enabled = 1;

Do we still need to keep the enable bit? I think !failed means enabled with this change.

Hi Toshi,

Sorry for the late reply

Quoted 39 lines · Toshi Kani

Quoted 38 lines · Yasuaki Ishimatsu

At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread, Toshi Kani mentioned as follows:

"I have a question about the change you made in commit 65479472 in acpi_memhotplug.c. This change seems to require that acpi_memory_enable_device() calls add_memory() to add all memory ranges represented by memory device objects at boot-time, and keep the results be used for hot-remove.

If I understand it right, this add_memory() call fails with EEXIST at boot-time since all memory ranges should have been added from EFI memory table (or e820) already. This results all memory ranges be marked as ! enabled & !failed. I think this means that we cannot hot-delete any memory ranges presented at boot-time since acpi_memory_remove_memory() only calls remove_memory() when the enabled flag is set. Is that correct?"

Above mention is correct. Thus even if memory device supports hotplug, memory presented at boot-time cannot be hot removed since the memory device's acpi_memory_info->enabled is always 0.

This patch changes to set 1 to "acpi_memory_info->enabled" of memory device presented at boot-time for hot removing the memory device.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

---
  drivers/acpi/acpi_memhotplug.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

drivers/acpi/acpi_memhotplug.c +0 -0

index da1f82b..88fd46a 100644
@@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
  			continue;
  		}
 

Quoted 6 lines

drivers/acpi/acpi_memhotplug.c static int acpi_memory_enable_device(struct acpi_memory_device *mem_device) · line 257

Quoted 3 lines

drivers/acpi/acpi_memhotplug.c +1 -2

-		if (!result)
-			info->enabled = 1;
+		info->enabled = 1;

Do we still need to keep the enable bit? I think !failed means enabled with this change.

For controlling memory hotplug, we need either failed bit or enabled bit. So I want to remove failed bit as follows:

---

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Quoted 56 lines · Yasuaki Ishimatsu

Hi Toshi,

Sorry for the late reply

Quoted 39 lines · Toshi Kani

Quoted 38 lines · Yasuaki Ishimatsu

At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread, Toshi Kani mentioned as follows:

"I have a question about the change you made in commit 65479472 in acpi_memhotplug.c. This change seems to require that acpi_memory_enable_device() calls add_memory() to add all memory ranges represented by memory device objects at boot-time, and keep the results be used for hot-remove.

If I understand it right, this add_memory() call fails with EEXIST at boot-time since all memory ranges should have been added from EFI memory table (or e820) already. This results all memory ranges be marked as ! enabled & !failed. I think this means that we cannot hot-delete any memory ranges presented at boot-time since acpi_memory_remove_memory() only calls remove_memory() when the enabled flag is set. Is that correct?"

Above mention is correct. Thus even if memory device supports hotplug, memory presented at boot-time cannot be hot removed since the memory device's acpi_memory_info->enabled is always 0.

This patch changes to set 1 to "acpi_memory_info->enabled" of memory device presented at boot-time for hot removing the memory device.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

---
   drivers/acpi/acpi_memhotplug.c |    4 ++--
   1 files changed, 2 insertions(+), 2 deletions(-)

drivers/acpi/acpi_memhotplug.c +0 -0

index da1f82b..88fd46a 100644
@@ -254,8 +254,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
   			continue;
   		}
 

Quoted 6 lines

drivers/acpi/acpi_memhotplug.c static int acpi_memory_enable_device(struct acpi_memory_device *mem_device) · line 257

Quoted 3 lines

drivers/acpi/acpi_memhotplug.c +1 -2

-		if (!result)
-			info->enabled = 1;
+		info->enabled = 1;

Do we still need to keep the enable bit? I think !failed means enabled with this change.

For controlling memory hotplug, we need either failed bit or enabled bit. So I want to remove failed bit as follows:

---

Quoted 4 lines

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Thanks for the update. Which branch does this patch apply to? I tried several, but could not apply.

-Toshi

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

---

v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
  drivers/acpi/acpi_memhotplug.c |   13 +------------
  1 files changed, 1 insertions(+), 12 deletions(-)

drivers/acpi/acpi_memhotplug.c +1 -13

index ea78988..597cd65 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
           * 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;
+        if (result && result != -EEXIST)
              continue;
-        }
 
          info->enabled = 1;
 

@@ -238,15 +235,7 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
      nid = acpi_get_node(mem_device->device->handle);
 
      list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-        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.
-             */
              return -EBUSY;
 
          if (nid < 0)
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

---

v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
  drivers/acpi/acpi_memhotplug.c |   13 +------------
  1 files changed, 1 insertions(+), 12 deletions(-)

drivers/acpi/acpi_memhotplug.c +1 -4

index ea78988..597cd65 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
  		 * 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;
+		if (result && result != -EEXIST)
  			continue;
-		}
 
  		info->enabled = 1;
 

@@ -238,15 +235,7 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
  	nid = acpi_get_node(mem_device->device->handle);
 
  	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {

Quoted 41 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

---

v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
   drivers/acpi/acpi_memhotplug.c |   13 +------------
   1 files changed, 1 insertions(+), 12 deletions(-)

drivers/acpi/acpi_memhotplug.c +1 -4

index ea78988..597cd65 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
   		 * 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;
+		if (result && result != -EEXIST)
   			continue;
-		}
 
   		info->enabled = 1;
 

@@ -238,15 +235,7 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
   	nid = acpi_get_node(mem_device->device->handle);
 

drivers/acpi/acpi_memhotplug.c static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device) · line 239

Quoted 11 lines

drivers/acpi/acpi_memhotplug.c +0 -8

   	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-		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.
-			 */
   			return -EBUSY;

Shouldn't this case (!info->enabled) continue since it is the same as info->failed before? -EBUSY was previously used for the -EEXIST case, which is no longer a failure case with this patchset.

Thanks,
-Toshi

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

---

v3 : Continue to memory hot remove when (!info->enabled) case.
v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
  drivers/acpi/acpi_memhotplug.c |   15 ++-------------
  1 files changed, 2 insertions(+), 13 deletions(-)

drivers/acpi/acpi_memhotplug.c +2 -13

index ea78988..5e6301e 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
  		 * 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;
+		if (result && result != -EEXIST)
  			continue;
-		}
 
  		info->enabled = 1;
 

@@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
  	nid = acpi_get_node(mem_device->device->handle);
 
  	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-		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.
-			 */
-			return -EBUSY;
+			continue;
 
  		if (nid < 0)
  			nid = memory_add_physaddr_to_nid(info->start_addr);

Quoted 6 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

Thanks for the update. It looks good. For the series:

Acked-by Toshi Kani <[email protected]>

-Toshi

Quoted 52 lines

---

v3 : Continue to memory hot remove when (!info->enabled) case.
v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
   drivers/acpi/acpi_memhotplug.c |   15 ++-------------
   1 files changed, 2 insertions(+), 13 deletions(-)

drivers/acpi/acpi_memhotplug.c +2 -13

index ea78988..5e6301e 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
   		 * 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;
+		if (result && result != -EEXIST)
   			continue;
-		}
 
   		info->enabled = 1;
 

@@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
   	nid = acpi_get_node(mem_device->device->handle);
 
   	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-		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.
-			 */
-			return -EBUSY;
+			continue;
 
   		if (nid < 0)
   			nid = memory_add_physaddr_to_nid(info->start_addr);

Quoted 6 lines · Toshi Kani

Quoted 5 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Quoted 5 lines

Quoted 1 line

Signed-off-by yasuaki ishimatsu <[email protected]>

Thanks for the update. It looks good. For the series:

Acked-by Toshi Kani <[email protected]>

Thanks!

Quoted 49 lines

Quoted 49 lines

---

v3 : Continue to memory hot remove when (!info->enabled) case.
v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
   drivers/acpi/acpi_memhotplug.c |   15 ++-------------
   1 files changed, 2 insertions(+), 13 deletions(-)

drivers/acpi/acpi_memhotplug.c +2 -13

index ea78988..5e6301e 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
   		 * 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;
+		if (result && result != -EEXIST)
   			continue;
-		}
 
   		info->enabled = 1;
 

@@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
   	nid = acpi_get_node(mem_device->device->handle);
 
   	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-		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.
-			 */
-			return -EBUSY;
+			continue;
 

Quoted 5 lines

drivers/acpi/acpi_memhotplug.c static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device) · line 242

Quoted 3 lines

drivers/acpi/acpi_memhotplug.c +0 -0

   		if (nid < 0)
   			nid = memory_add_physaddr_to_nid(info->start_addr);

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 9 lines · Yasuaki Ishimatsu

Quoted 8 lines · Toshi Kani

Quoted 6 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

Quoted 5 lines

Quoted 3 lines

Thanks for the update. It looks good. For the series:

Acked-by Toshi Kani <[email protected]>

Thanks!

Both patches applied to linux-pm.git/linux-next.

Thanks,
Rafael

Quoted 60 lines

Quoted 49 lines

Quoted 49 lines

---

v3 : Continue to memory hot remove when (!info->enabled) case.
v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
    drivers/acpi/acpi_memhotplug.c |   15 ++-------------
    1 files changed, 2 insertions(+), 13 deletions(-)

drivers/acpi/acpi_memhotplug.c +2 -13

index ea78988..5e6301e 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
    		 * 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;
+		if (result && result != -EEXIST)
    			continue;
-		}
 
    		info->enabled = 1;
 

@@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
    	nid = acpi_get_node(mem_device->device->handle);
 
    	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-		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.
-			 */
-			return -EBUSY;
+			continue;
 

Quoted 5 lines

drivers/acpi/acpi_memhotplug.c static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device) · line 242

Quoted 3 lines

drivers/acpi/acpi_memhotplug.c +0 -0

    		if (nid < 0)
    			nid = memory_add_physaddr_to_nid(info->start_addr);

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Rafael,

Quoted 14 lines · Rafael J. Wysocki

Quoted 13 lines · Yasuaki Ishimatsu

Quoted 11 lines · Toshi Kani

Quoted 6 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

Thanks for the update. It looks good. For the series:

Acked-by Toshi Kani <[email protected]>

Quoted 3 lines

Quoted 1 line

Thanks!

Both patches applied to linux-pm.git/linux-next.

Thank you for applying both patches.
If you don't mind, I hope these patches are merged into linux-3.9 since this is bug fix.

Quoted 56 lines

Quoted 56 lines

Quoted 54 lines

Quoted 52 lines

---

v3 : Continue to memory hot remove when (!info->enabled) case.
v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
    drivers/acpi/acpi_memhotplug.c |   15 ++-------------
    1 files changed, 2 insertions(+), 13 deletions(-)

drivers/acpi/acpi_memhotplug.c +2 -13

index ea78988..5e6301e 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
    		 * 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;
+		if (result && result != -EEXIST)
    			continue;
-		}
 
    		info->enabled = 1;
 

@@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
    	nid = acpi_get_node(mem_device->device->handle);
 
    	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-		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.
-			 */
-			return -EBUSY;
+			continue;
 
    		if (nid < 0)
    			nid = memory_add_physaddr_to_nid(info->start_addr);

Quoted 4 lines

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 19 lines · Yasuaki Ishimatsu

Hi Rafael,

Quoted 16 lines · Rafael J. Wysocki

Quoted 14 lines · Yasuaki Ishimatsu

Quoted 11 lines · Toshi Kani

Quoted 6 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

Thanks for the update. It looks good. For the series:

Acked-by Toshi Kani <[email protected]>

Thanks!

Quoted 5 lines

Quoted 1 line

Both patches applied to linux-pm.git/linux-next.

Thank you for applying both patches.
If you don't mind, I hope these patches are merged into linux-3.9 since this is bug fix.

I honestly don't think that [2/2] is 3.9 material, but I can push [1/2] for 3.9 provided that you also regard it as material for 3.8-stable.

Thanks,
Rafael

Quoted 66 lines

Quoted 56 lines

Quoted 56 lines

Quoted 54 lines

Quoted 52 lines

---

v3 : Continue to memory hot remove when (!info->enabled) case.
v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
     drivers/acpi/acpi_memhotplug.c |   15 ++-------------
     1 files changed, 2 insertions(+), 13 deletions(-)

drivers/acpi/acpi_memhotplug.c +2 -13

index ea78988..5e6301e 100644
@@ -73,7 +73,6 @@ 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 {

@@ -201,10 +200,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
     		 * 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;
+		if (result && result != -EEXIST)
     			continue;
-		}
 
     		info->enabled = 1;
 

@@ -238,16 +235,8 @@ static int acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
     	nid = acpi_get_node(mem_device->device->handle);
 
     	list_for_each_entry_safe(info, n, &mem_device->res_list, list) {
-		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.
-			 */
-			return -EBUSY;
+			continue;
 
     		if (nid < 0)
     			nid = memory_add_physaddr_to_nid(info->start_addr);

Quoted 4 lines

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Rafael,

Quoted 26 lines · Rafael J. Wysocki

Quoted 24 lines · Yasuaki Ishimatsu

Hi Rafael,

Quoted 10 lines · Rafael J. Wysocki

Quoted 9 lines · Yasuaki Ishimatsu

Quoted 8 lines · Toshi Kani

Quoted 6 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Thanks for the update. It looks good. For the series:

Acked-by Toshi Kani <[email protected]>

Thanks!

Both patches applied to linux-pm.git/linux-next.

Thank you for applying both patches.
If you don't mind, I hope these patches are merged into linux-3.9 since this is bug fix.

Quoted 2 lines

I honestly don't think that [2/2] is 3.9 material, but I can push [1/2] for 3.9 provided that you also regard it as material for 3.8-stable.

linux-3.8 does not support physical memory hot remove. It has been supported since linux-3.9. So I don't need [1/2] patch to linux-3.8 but need it to linux-3.9. So please push [1/2] patch to linux-3.9.

Quoted 29 lines · Yasuaki Ishimatsu

Hi Rafael,

Quoted 26 lines · Rafael J. Wysocki

Quoted 24 lines · Yasuaki Ishimatsu

Hi Rafael,

Quoted 10 lines · Rafael J. Wysocki

Quoted 9 lines · Yasuaki Ishimatsu

Quoted 8 lines · Toshi Kani

Quoted 6 lines · Yasuaki Ishimatsu

acpi_memory_info has enabled bit and failed bit for controlling memory hotplug. But we don't need to keep both bits.

The patch removes acpi_memory_info->failed bit.

Signed-off-by yasuaki ishimatsu <[email protected]>

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Thanks for the update. It looks good. For the series:

Acked-by Toshi Kani <[email protected]>

Thanks!

Both patches applied to linux-pm.git/linux-next.

Thank you for applying both patches.
If you don't mind, I hope these patches are merged into linux-3.9 since this is bug fix.

Quoted 6 lines

Quoted 2 lines

I honestly don't think that [2/2] is 3.9 material, but I can push [1/2] for 3.9 provided that you also regard it as material for 3.8-stable.

linux-3.8 does not support physical memory hot remove. It has been supported since linux-3.9. So I don't need [1/2] patch to linux-3.8 but need it to linux-3.9. So please push [1/2] patch to linux-3.9.

Well, I don't think it would be a good idea to push that patch for 3.9 so late in the cycle. Yes, it would make memory hotplug work in 3.9, but that might surprise some people and cause problems to happen as a result and we wouldn't have the time to address them before the 3.9 release.

If that's not a big problem, I'd prefer to defer that patch until the 3.10 merge window is open (which is just about 2 weeks from now) and if need be we can still push it for 3.9-stable afterward.

Thanks,
Rafael

At http://marc.info/?l=linux-acpi&m=135769405622667&w=2 thread, Toshi Kani mentioned as follows:

"I have a question about the change you made in commit 65479472 in acpi_memhotplug.c. This change seems to require that acpi_memory_enable_device() calls add_memory() to add all memory ranges represented by memory device objects at boot-time, and keep the results be used for hot-remove.

If I understand it right, this add_memory() call fails with EEXIST at boot-time since all memory ranges should have been added from EFI memory table (or e820) already. This results all memory ranges be marked as ! enabled & !failed. I think this means that we cannot hot-delete any memory ranges presented at boot-time since acpi_memory_remove_memory() only calls remove_memory() when the enabled flag is set. Is that correct?"

Above mention is correct. Thus even if memory device supports hotplug, memory presented at boot-time cannot be hot removed since the memory device's acpi_memory_info->enabled is always 0.

This patch changes to set 1 to "acpi_memory_info->enabled" of memory device presented at boot-time for hot removing the memory device.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

---

v2 : Changed a based kernel from linux-3.9-rc2 to linux-pm.git/bleeding-edge.

---
  drivers/acpi/acpi_memhotplug.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

drivers/acpi/acpi_memhotplug.c +2 -3

index d4f2eb8..ea78988 100644
@@ -206,8 +206,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
              continue;
          }
 
-        if (!result)
-            info->enabled = 1;
+        info->enabled = 1;
+
          /*
           * Add num_enable even if add_memory() returns -EEXIST, so the
           * device is bound to this driver.
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html