All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
@ 2018-06-22 11:52 ` guomin chen
  0 siblings, 0 replies; 9+ messages in thread
From: guomin chen @ 2018-06-22 11:52 UTC (permalink / raw)
  To: Thomas Gleixner, Brijesh Singh, Borislav Petkov, Andrew Morton
  Cc: Takashi Iwai, Lee Chun-Yi, linux-efi, linux-kernel, guomin chen,
	Tom Lendacky, Bjorn Helgaas, Yaowei Bai, Dan Williams,
	linux-kernel

We've got a bug report indicating the hot-remove node resource
release failed,when the memory on this node is divided into
several sections.because the release_mem_region_adjustable()
can only release one resource that must be [start,end].

In my case, the BIOS supports faulty memory isolation. if BIOS
detected bad memory block, the BIOS will isolates this badblock.
And set this badblock memory to EfiUnusableMemory in EFI memory map
base on UEFI 2.7 spec.For example in my system, the memory range on
node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
detected the [8004e000000-8004e0fffff] is a badblock memory.
So the memory on node2 seem like this:
   		80000000000-8004dffffff : System RAM
		8004e000000-8004e0fffff : Unusable memory
		8004e100000-807ffffffff : System RAM

Now, when offline the cpu node2,the kernel will try to release
ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this
time, the kernel will release failed,and output error message:
"Unable to release resource <0x0000080000000000-0x00000807ffffffff>
(-22)".
Because the release_mem_region_adjustable() can only release one
resource that must be [0x0000080000000000 , 0x00000807ffffffff].
but now,the iomem resource on node2 [0x0000080000000000,
0x00000807ffffffff] are divided into three resources [80000000000-
8004dffffff],[8004e000000-8004e0fffff]and[8004e100000-807ffffffff].

This patch help to Release multiple iomem resources at once when
node hot-remove. Such as in above case, when hot-remove the cpu
node2,the kernel will try to release resource [0x0000080000000000-
0x00000807ffffffff].And this patch will release three resources
[80000000000-8004dffffff],[8004e000000-8004e0fffff] and
[8004e100000-807ffffffff].

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org your.patch
Cc: Lee Chun-Yi <JLee@suse.com>
Signed-off-by: guomin chen <guomin.chen@suse.com>
---
 kernel/resource.c | 73 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 30e1bc68503b..959bcce4c405 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1240,6 +1240,7 @@ int release_mem_region_adjustable(struct resource *parent,
 	struct resource *res;
 	struct resource *new_res;
 	resource_size_t end;
+	resource_size_t new_start = start;
 	int ret = -EINVAL;
 
 	end = start + size - 1;
@@ -1257,7 +1258,7 @@ int release_mem_region_adjustable(struct resource *parent,
 			break;
 
 		/* look for the next resource if it does not fit into */
-		if (res->start > start || res->end < end) {
+		if (res->end < new_start) {
 			p = &res->sibling;
 			continue;
 		}
@@ -1271,42 +1272,54 @@ int release_mem_region_adjustable(struct resource *parent,
 		}
 
 		/* found the target resource; let's adjust accordingly */
-		if (res->start == start && res->end == end) {
+		if (res->start == new_start && res->end == end) {
 			/* free the whole entry */
 			*p = res->sibling;
 			free_resource(res);
 			ret = 0;
-		} else if (res->start == start && res->end != end) {
-			/* adjust the start */
-			ret = __adjust_resource(res, end + 1,
-						res->end - end);
-		} else if (res->start != start && res->end == end) {
-			/* adjust the end */
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
+		} else if (res->end > end) {
+			if (res->start >= new_start) {
+				/* adjust the start */
+				ret = __adjust_resource(res, end + 1,
+							res->end - end);
+			} else {
+				/* split into two entries */
+				if (!new_res) {
+					ret = -ENOMEM;
+					break;
+				}
+				new_res->name = res->name;
+				new_res->start = end + 1;
+				new_res->end = res->end;
+				new_res->flags = res->flags;
+				new_res->desc = res->desc;
+				new_res->parent = res->parent;
+				new_res->sibling = res->sibling;
+				new_res->child = NULL;
+
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				if (ret)
+					break;
+				res->sibling = new_res;
+				new_res = NULL;
+			}
 		} else {
-			/* split into two entries */
-			if (!new_res) {
-				ret = -ENOMEM;
-				break;
+			if (res->start < new_start) {
+				/* adjust the end */
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				new_start = res->end+1;
+				p = &res->sibling;
+			} else {
+				new_start = res->end+1;
+				*p = res->sibling;
+				free_resource(res);
+				ret = 0;
 			}
-			new_res->name = res->name;
-			new_res->start = end + 1;
-			new_res->end = res->end;
-			new_res->flags = res->flags;
-			new_res->desc = res->desc;
-			new_res->parent = res->parent;
-			new_res->sibling = res->sibling;
-			new_res->child = NULL;
-
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
-			if (ret)
-				break;
-			res->sibling = new_res;
-			new_res = NULL;
+			if (res->end < end)
+				continue;
 		}
-
 		break;
 	}
 
-- 
2.12.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
@ 2018-06-22 11:52 ` guomin chen
  0 siblings, 0 replies; 9+ messages in thread
From: guomin chen @ 2018-06-22 11:52 UTC (permalink / raw)
  To: Thomas Gleixner, Brijesh Singh, Borislav Petkov, Andrew Morton
  Cc: Takashi Iwai, Lee Chun-Yi, linux-efi, linux-kernel, guomin chen,
	Tom Lendacky, Bjorn Helgaas, Yaowei Bai, Dan Williams,
	linux-kernel

We've got a bug report indicating the hot-remove node resource
release failed,when the memory on this node is divided into
several sections.because the release_mem_region_adjustable()
can only release one resource that must be [start,end].

In my case, the BIOS supports faulty memory isolation. if BIOS
detected bad memory block, the BIOS will isolates this badblock.
And set this badblock memory to EfiUnusableMemory in EFI memory map
base on UEFI 2.7 spec.For example in my system, the memory range on
node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
detected the [8004e000000-8004e0fffff] is a badblock memory.
So the memory on node2 seem like this:
   		80000000000-8004dffffff : System RAM
		8004e000000-8004e0fffff : Unusable memory
		8004e100000-807ffffffff : System RAM

Now, when offline the cpu node2,the kernel will try to release
ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this
time, the kernel will release failed,and output error message:
"Unable to release resource <0x0000080000000000-0x00000807ffffffff>
(-22)".
Because the release_mem_region_adjustable() can only release one
resource that must be [0x0000080000000000 , 0x00000807ffffffff].
but now,the iomem resource on node2 [0x0000080000000000,
0x00000807ffffffff] are divided into three resources [80000000000-
8004dffffff],[8004e000000-8004e0fffff]and[8004e100000-807ffffffff].

This patch help to Release multiple iomem resources at once when
node hot-remove. Such as in above case, when hot-remove the cpu
node2,the kernel will try to release resource [0x0000080000000000-
0x00000807ffffffff].And this patch will release three resources
[80000000000-8004dffffff],[8004e000000-8004e0fffff] and
[8004e100000-807ffffffff].

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org your.patch
Cc: Lee Chun-Yi <JLee@suse.com>
Signed-off-by: guomin chen <guomin.chen@suse.com>
---
 kernel/resource.c | 73 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 30e1bc68503b..959bcce4c405 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1240,6 +1240,7 @@ int release_mem_region_adjustable(struct resource *parent,
 	struct resource *res;
 	struct resource *new_res;
 	resource_size_t end;
+	resource_size_t new_start = start;
 	int ret = -EINVAL;
 
 	end = start + size - 1;
@@ -1257,7 +1258,7 @@ int release_mem_region_adjustable(struct resource *parent,
 			break;
 
 		/* look for the next resource if it does not fit into */
-		if (res->start > start || res->end < end) {
+		if (res->end < new_start) {
 			p = &res->sibling;
 			continue;
 		}
@@ -1271,42 +1272,54 @@ int release_mem_region_adjustable(struct resource *parent,
 		}
 
 		/* found the target resource; let's adjust accordingly */
-		if (res->start == start && res->end == end) {
+		if (res->start == new_start && res->end == end) {
 			/* free the whole entry */
 			*p = res->sibling;
 			free_resource(res);
 			ret = 0;
-		} else if (res->start == start && res->end != end) {
-			/* adjust the start */
-			ret = __adjust_resource(res, end + 1,
-						res->end - end);
-		} else if (res->start != start && res->end == end) {
-			/* adjust the end */
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
+		} else if (res->end > end) {
+			if (res->start >= new_start) {
+				/* adjust the start */
+				ret = __adjust_resource(res, end + 1,
+							res->end - end);
+			} else {
+				/* split into two entries */
+				if (!new_res) {
+					ret = -ENOMEM;
+					break;
+				}
+				new_res->name = res->name;
+				new_res->start = end + 1;
+				new_res->end = res->end;
+				new_res->flags = res->flags;
+				new_res->desc = res->desc;
+				new_res->parent = res->parent;
+				new_res->sibling = res->sibling;
+				new_res->child = NULL;
+
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				if (ret)
+					break;
+				res->sibling = new_res;
+				new_res = NULL;
+			}
 		} else {
-			/* split into two entries */
-			if (!new_res) {
-				ret = -ENOMEM;
-				break;
+			if (res->start < new_start) {
+				/* adjust the end */
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				new_start = res->end+1;
+				p = &res->sibling;
+			} else {
+				new_start = res->end+1;
+				*p = res->sibling;
+				free_resource(res);
+				ret = 0;
 			}
-			new_res->name = res->name;
-			new_res->start = end + 1;
-			new_res->end = res->end;
-			new_res->flags = res->flags;
-			new_res->desc = res->desc;
-			new_res->parent = res->parent;
-			new_res->sibling = res->sibling;
-			new_res->child = NULL;
-
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
-			if (ret)
-				break;
-			res->sibling = new_res;
-			new_res = NULL;
+			if (res->end < end)
+				continue;
 		}
-
 		break;
 	}
 
-- 
2.12.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
  2018-06-25 23:55   ` Kani, Toshi
@ 2018-06-27  6:43     ` Guomin Chen
  0 siblings, 0 replies; 9+ messages in thread
From: Guomin Chen @ 2018-06-27  6:43 UTC (permalink / raw)
  To: Kani, Toshi, helgaas
  Cc: linux-kernel, Joey Lee, bp, tglx, tiwai, akpm, dan.j.williams,
	linux-efi, bhelgaas, thomas.lendacky, brijesh.singh, baiyaowei

Hi Toshi
	Thank you for your reply and suggestions.
	I will continue to follow and investigation this issue. thanks 

Thanks and regards


> -----Original Message-----
> From: Kani, Toshi [mailto:toshi.kani@hpe.com]
> Sent: 2018年6月26日 7:56
> To: Guomin Chen <guomin.chen@suse.com>; helgaas@kernel.org
> Cc: linux-kernel@vger.kernel.org; Joey Lee <JLee@suse.com>; bp@suse.de;
> tglx@linutronix.de; tiwai@suse.de; akpm@linux-foundation.org;
> dan.j.williams@intel.com; linux-efi@vger.kernel.org; bhelgaas@google.com;
> thomas.lendacky@amd.com; brijesh.singh@amd.com;
> baiyaowei@cmss.chinamobile.com
> Subject: Re: [PATCH 1/1] resource: Fixed iomem resource release failed on
> release_mem_region_adjustable() when memory node or cpu node
> hot-remove.
> 
> On Fri, 2018-06-22 at 18:19 -0500, Bjorn Helgaas wrote:
> > [+cc Toshi]
> >
> > On Fri, Jun 22, 2018 at 08:01:38PM +0800, guomin chen wrote:
> > > We've got a bug report indicating the hot-remove node resource
> > > release failed,when the memory on this node is divided into several
> > > sections.because the release_mem_region_adjustable() can only
> > > release one resource that must be [start,end].
> >
> > Can you please include a URL for the bug report?  That's useful for
> > additional details and gives hints about how future changes in this
> > area might be tested.
> >
> > release_mem_region_adjustable() and the only call to it were added by
> > Toshi (cc'd):
> >
> >   825f787bb496 ("resource: add release_mem_region_adjustable()")
> >   fe74ebb106a5 ("mm: change __remove_pages() to call
> > release_mem_region_adjustable()")
> >
> > > In my case, the BIOS supports faulty memory isolation. if BIOS
> > > detected bad memory block, the BIOS will isolates this badblock.
> > > And set this badblock memory to EfiUnusableMemory in EFI memory map
> > > base on UEFI 2.7 spec.For example in my system, the memory range on
> > > node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
> > > detected the [8004e000000-8004e0fffff] is a badblock memory.
> > > So the memory on node2 seem like this:
> > >    		80000000000-8004dffffff : System RAM
> > > 		8004e000000-8004e0fffff : Unusable memory
> > > 		8004e100000-807ffffffff : System RAM
> > >
> > > Now, when offline the cpu node2,the kernel will try to release
> > > ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this
> > > time, the kernel will release failed,and output error message:
> > > "Unable to release resource <0x0000080000000000-0x00000807ffffffff>
> > > (-22)".
> 
> Hmm... this BIO implementation does not sound right to me.  When EFI
> memory map has a hole, ACPI memory object should have the same hole in its
> resource info.  If so, this node2 memory hot-delete would have requested a
> deletion of 2 separate System RAM ranges, not a single range including this
> hole.  This is also important for memory hot-add.  ACPI memory object needs
> to describe this hole in order for the OS to avoid adding this badblock memory.
> 
> Thanks,
> -Toshi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
  2018-06-22 23:19 ` Bjorn Helgaas
  2018-06-23  5:53   ` Guomin Chen
@ 2018-06-25 23:55   ` Kani, Toshi
  2018-06-27  6:43     ` Guomin Chen
  1 sibling, 1 reply; 9+ messages in thread
From: Kani, Toshi @ 2018-06-25 23:55 UTC (permalink / raw)
  To: Chen, Guomin, helgaas
  Cc: linux-kernel, Lee, Joey, bp, tglx, tiwai, akpm, dan.j.williams,
	linux-efi, bhelgaas, thomas.lendacky, brijesh.singh, baiyaowei

On Fri, 2018-06-22 at 18:19 -0500, Bjorn Helgaas wrote:
> [+cc Toshi]
> 
> On Fri, Jun 22, 2018 at 08:01:38PM +0800, guomin chen wrote:
> > We've got a bug report indicating the hot-remove node resource
> > release failed,when the memory on this node is divided into
> > several sections.because the release_mem_region_adjustable()
> > can only release one resource that must be [start,end].
> 
> Can you please include a URL for the bug report?  That's useful for
> additional details and gives hints about how future changes in this
> area might be tested.
> 
> release_mem_region_adjustable() and the only call to it were added
> by Toshi (cc'd):
> 
>   825f787bb496 ("resource: add release_mem_region_adjustable()")
>   fe74ebb106a5 ("mm: change __remove_pages() to call release_mem_region_adjustable()")
> 
> > In my case, the BIOS supports faulty memory isolation. if BIOS
> > detected bad memory block, the BIOS will isolates this badblock.
> > And set this badblock memory to EfiUnusableMemory in EFI memory map
> > base on UEFI 2.7 spec.For example in my system, the memory range on
> > node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
> > detected the [8004e000000-8004e0fffff] is a badblock memory.
> > So the memory on node2 seem like this:
> >    		80000000000-8004dffffff : System RAM
> > 		8004e000000-8004e0fffff : Unusable memory
> > 		8004e100000-807ffffffff : System RAM
> > 
> > Now, when offline the cpu node2,the kernel will try to release
> > ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this
> > time, the kernel will release failed,and output error message:
> > "Unable to release resource <0x0000080000000000-0x00000807ffffffff>
> > (-22)".

Hmm... this BIO implementation does not sound right to me.  When EFI
memory map has a hole, ACPI memory object should have the same hole in
its resource info.  If so, this node2 memory hot-delete would have
requested a deletion of 2 separate System RAM ranges, not a single range
including this hole.  This is also important for memory hot-add.  ACPI
memory object needs to describe this hole in order for the OS to avoid
adding this badblock memory.

Thanks,
-Toshi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
  2018-06-23  5:53   ` Guomin Chen
@ 2018-06-25 17:21     ` Bjorn Helgaas
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2018-06-25 17:21 UTC (permalink / raw)
  To: Guomin Chen
  Cc: Brijesh Singh, Tom Lendacky, Yaowei Bai, Bjorn Helgaas,
	Toshi Kani, Dan Williams, Thomas Gleixner, Andrew Morton,
	Joey Lee, Borislav Petkov, Takashi Iwai, linux-efi, linux-kernel

On Sat, Jun 23, 2018 at 05:53:40AM +0000, Guomin Chen wrote:
> Hi 
> 	The report link for this issue is: https://bugzilla.suse.com/show_bug.cgi?id=1092687

If/when you update this patch, please include the URL in the commit log,
e.g.,

  Link: https://bugzilla.suse.com/show_bug.cgi?id=1092687

It would also be nice if you could make that report public so anybody can
read it.

> > -----Original Message-----
> > From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> > Sent: 2018年6月23日 7:20
> > To: Guomin Chen <guomin.chen@suse.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>; Tom Lendacky
> > <thomas.lendacky@amd.com>; Yaowei Bai
> > <baiyaowei@cmss.chinamobile.com>; Bjorn Helgaas <bhelgaas@google.com>;
> > Toshi Kani <toshi.kani@hpe.com>; Dan Williams <dan.j.williams@intel.com>;
> > Thomas Gleixner <tglx@linutronix.de>; Andrew Morton
> > <akpm@linux-foundation.org>; Joey Lee <JLee@suse.com>; Borislav Petkov
> > <bp@suse.de>; Takashi Iwai <tiwai@suse.de>; linux-efi@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 1/1] resource: Fixed iomem resource release failed on
> > release_mem_region_adjustable() when memory node or cpu node
> > hot-remove.
> > 
> > [+cc Toshi]
> > 
> > On Fri, Jun 22, 2018 at 08:01:38PM +0800, guomin chen wrote:
> > > We've got a bug report indicating the hot-remove node resource release
> > > failed,when the memory on this node is divided into several
> > > sections.because the release_mem_region_adjustable() can only release
> > > one resource that must be [start,end].
> > 
> > Can you please include a URL for the bug report?  That's useful for additional
> > details and gives hints about how future changes in this area might be tested.
> > 
> > release_mem_region_adjustable() and the only call to it were added by Toshi
> > (cc'd):
> > 
> >   825f787bb496 ("resource: add release_mem_region_adjustable()")
> >   fe74ebb106a5 ("mm: change __remove_pages() to call
> > release_mem_region_adjustable()")
> > 
> > > In my case, the BIOS supports faulty memory isolation. if BIOS
> > > detected bad memory block, the BIOS will isolates this badblock.
> > > And set this badblock memory to EfiUnusableMemory in EFI memory map
> > > base on UEFI 2.7 spec.For example in my system, the memory range on
> > > node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
> > > detected the [8004e000000-8004e0fffff] is a badblock memory.
> > > So the memory on node2 seem like this:
> > >    		80000000000-8004dffffff : System RAM
> > > 		8004e000000-8004e0fffff : Unusable memory
> > > 		8004e100000-807ffffffff : System RAM
> > >
> > > Now, when offline the cpu node2,the kernel will try to release
> > > ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this time,
> > > the kernel will release failed,and output error message:
> > > "Unable to release resource <0x0000080000000000-0x00000807ffffffff>
> > > (-22)".
> > > Because the release_mem_region_adjustable() can only release one
> > > resource that must be [0x0000080000000000 , 0x00000807ffffffff].
> > > but now,the iomem resource on node2 [0x0000080000000000,
> > > 0x00000807ffffffff] are divided into three resources [80000000000-
> > > 8004dffffff],[8004e000000-8004e0fffff]and[8004e100000-807ffffffff].
> > >
> > > This patch help to Release multiple iomem resources at once when node
> > > hot-remove. Such as in above case, when hot-remove the cpu node2,the
> > > kernel will try to release resource [0x0000080000000000-
> > > 0x00000807ffffffff].And this patch will release three resources
> > > [80000000000-8004dffffff],[8004e000000-8004e0fffff] and
> > > [8004e100000-807ffffffff].
> > >
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > > Cc: Borislav Petkov <bp@suse.de>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Cc: Dan Williams <dan.j.williams@intel.com>
> > > Cc: linux-efi@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Cc: Lee Chun-Yi <JLee@suse.com>
> > > Signed-off-by: guomin chen <guomin.chen@suse.com>
> > > ---
> > >  kernel/resource.c | 73
> > > ++++++++++++++++++++++++++++++++-----------------------
> > >  1 file changed, 43 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/kernel/resource.c b/kernel/resource.c index
> > > 30e1bc68503b..959bcce4c405 100644
> > > --- a/kernel/resource.c
> > > +++ b/kernel/resource.c
> > > @@ -1240,6 +1240,7 @@ int release_mem_region_adjustable(struct
> > resource *parent,
> > >  	struct resource *res;
> > >  	struct resource *new_res;
> > >  	resource_size_t end;
> > > +	resource_size_t new_start = start;
> > >  	int ret = -EINVAL;
> > >
> > >  	end = start + size - 1;
> > > @@ -1257,7 +1258,7 @@ int release_mem_region_adjustable(struct
> > resource *parent,
> > >  			break;
> > >
> > >  		/* look for the next resource if it does not fit into */
> > > -		if (res->start > start || res->end < end) {
> > > +		if (res->end < new_start) {
> > >  			p = &res->sibling;
> > >  			continue;
> > >  		}
> > > @@ -1271,42 +1272,54 @@ int release_mem_region_adjustable(struct
> > resource *parent,
> > >  		}
> > >
> > >  		/* found the target resource; let's adjust accordingly */
> > > -		if (res->start == start && res->end == end) {
> > > +		if (res->start == new_start && res->end == end) {
> > >  			/* free the whole entry */
> > >  			*p = res->sibling;
> > >  			free_resource(res);
> > >  			ret = 0;
> > > -		} else if (res->start == start && res->end != end) {
> > > -			/* adjust the start */
> > > -			ret = __adjust_resource(res, end + 1,
> > > -						res->end - end);
> > > -		} else if (res->start != start && res->end == end) {
> > > -			/* adjust the end */
> > > -			ret = __adjust_resource(res, res->start,
> > > -						start - res->start);
> > > +		} else if (res->end > end) {
> > > +			if (res->start >= new_start) {
> > > +				/* adjust the start */
> > > +				ret = __adjust_resource(res, end + 1,
> > > +							res->end - end);
> > > +			} else {
> > > +				/* split into two entries */
> > > +				if (!new_res) {
> > > +					ret = -ENOMEM;
> > > +					break;
> > > +				}
> > > +				new_res->name = res->name;
> > > +				new_res->start = end + 1;
> > > +				new_res->end = res->end;
> > > +				new_res->flags = res->flags;
> > > +				new_res->desc = res->desc;
> > > +				new_res->parent = res->parent;
> > > +				new_res->sibling = res->sibling;
> > > +				new_res->child = NULL;
> > > +
> > > +				ret = __adjust_resource(res, res->start,
> > > +							new_start - res->start);
> > > +				if (ret)
> > > +					break;
> > > +				res->sibling = new_res;
> > > +				new_res = NULL;
> > > +			}
> > >  		} else {
> > > -			/* split into two entries */
> > > -			if (!new_res) {
> > > -				ret = -ENOMEM;
> > > -				break;
> > > +			if (res->start < new_start) {
> > > +				/* adjust the end */
> > > +				ret = __adjust_resource(res, res->start,
> > > +							new_start - res->start);
> > > +				new_start = res->end+1;
> > > +				p = &res->sibling;
> > > +			} else {
> > > +				new_start = res->end+1;
> > > +				*p = res->sibling;
> > > +				free_resource(res);
> > > +				ret = 0;
> > >  			}
> > > -			new_res->name = res->name;
> > > -			new_res->start = end + 1;
> > > -			new_res->end = res->end;
> > > -			new_res->flags = res->flags;
> > > -			new_res->desc = res->desc;
> > > -			new_res->parent = res->parent;
> > > -			new_res->sibling = res->sibling;
> > > -			new_res->child = NULL;
> > > -
> > > -			ret = __adjust_resource(res, res->start,
> > > -						start - res->start);
> > > -			if (ret)
> > > -				break;
> > > -			res->sibling = new_res;
> > > -			new_res = NULL;
> > > +			if (res->end < end)
> > > +				continue;
> > >  		}
> > > -
> > >  		break;
> > >  	}
> > >
> > > --
> > > 2.12.3
> > >
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
  2018-06-22 23:19 ` Bjorn Helgaas
@ 2018-06-23  5:53   ` Guomin Chen
  2018-06-25 17:21     ` Bjorn Helgaas
  2018-06-25 23:55   ` Kani, Toshi
  1 sibling, 1 reply; 9+ messages in thread
From: Guomin Chen @ 2018-06-23  5:53 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Brijesh Singh, Tom Lendacky, Yaowei Bai, Bjorn Helgaas,
	Toshi Kani, Dan Williams, Thomas Gleixner, Andrew Morton,
	Joey Lee, Borislav Petkov, Takashi Iwai, linux-efi, linux-kernel

Hi 
	The report link for this issue is: https://bugzilla.suse.com/show_bug.cgi?id=1092687

Thanks and regards

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 2018年6月23日 7:20
> To: Guomin Chen <guomin.chen@suse.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Yaowei Bai
> <baiyaowei@cmss.chinamobile.com>; Bjorn Helgaas <bhelgaas@google.com>;
> Toshi Kani <toshi.kani@hpe.com>; Dan Williams <dan.j.williams@intel.com>;
> Thomas Gleixner <tglx@linutronix.de>; Andrew Morton
> <akpm@linux-foundation.org>; Joey Lee <JLee@suse.com>; Borislav Petkov
> <bp@suse.de>; Takashi Iwai <tiwai@suse.de>; linux-efi@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/1] resource: Fixed iomem resource release failed on
> release_mem_region_adjustable() when memory node or cpu node
> hot-remove.
> 
> [+cc Toshi]
> 
> On Fri, Jun 22, 2018 at 08:01:38PM +0800, guomin chen wrote:
> > We've got a bug report indicating the hot-remove node resource release
> > failed,when the memory on this node is divided into several
> > sections.because the release_mem_region_adjustable() can only release
> > one resource that must be [start,end].
> 
> Can you please include a URL for the bug report?  That's useful for additional
> details and gives hints about how future changes in this area might be tested.
> 
> release_mem_region_adjustable() and the only call to it were added by Toshi
> (cc'd):
> 
>   825f787bb496 ("resource: add release_mem_region_adjustable()")
>   fe74ebb106a5 ("mm: change __remove_pages() to call
> release_mem_region_adjustable()")
> 
> > In my case, the BIOS supports faulty memory isolation. if BIOS
> > detected bad memory block, the BIOS will isolates this badblock.
> > And set this badblock memory to EfiUnusableMemory in EFI memory map
> > base on UEFI 2.7 spec.For example in my system, the memory range on
> > node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
> > detected the [8004e000000-8004e0fffff] is a badblock memory.
> > So the memory on node2 seem like this:
> >    		80000000000-8004dffffff : System RAM
> > 		8004e000000-8004e0fffff : Unusable memory
> > 		8004e100000-807ffffffff : System RAM
> >
> > Now, when offline the cpu node2,the kernel will try to release
> > ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this time,
> > the kernel will release failed,and output error message:
> > "Unable to release resource <0x0000080000000000-0x00000807ffffffff>
> > (-22)".
> > Because the release_mem_region_adjustable() can only release one
> > resource that must be [0x0000080000000000 , 0x00000807ffffffff].
> > but now,the iomem resource on node2 [0x0000080000000000,
> > 0x00000807ffffffff] are divided into three resources [80000000000-
> > 8004dffffff],[8004e000000-8004e0fffff]and[8004e100000-807ffffffff].
> >
> > This patch help to Release multiple iomem resources at once when node
> > hot-remove. Such as in above case, when hot-remove the cpu node2,the
> > kernel will try to release resource [0x0000080000000000-
> > 0x00000807ffffffff].And this patch will release three resources
> > [80000000000-8004dffffff],[8004e000000-8004e0fffff] and
> > [8004e100000-807ffffffff].
> >
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Cc: Borislav Petkov <bp@suse.de>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: linux-efi@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: Lee Chun-Yi <JLee@suse.com>
> > Signed-off-by: guomin chen <guomin.chen@suse.com>
> > ---
> >  kernel/resource.c | 73
> > ++++++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 43 insertions(+), 30 deletions(-)
> >
> > diff --git a/kernel/resource.c b/kernel/resource.c index
> > 30e1bc68503b..959bcce4c405 100644
> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -1240,6 +1240,7 @@ int release_mem_region_adjustable(struct
> resource *parent,
> >  	struct resource *res;
> >  	struct resource *new_res;
> >  	resource_size_t end;
> > +	resource_size_t new_start = start;
> >  	int ret = -EINVAL;
> >
> >  	end = start + size - 1;
> > @@ -1257,7 +1258,7 @@ int release_mem_region_adjustable(struct
> resource *parent,
> >  			break;
> >
> >  		/* look for the next resource if it does not fit into */
> > -		if (res->start > start || res->end < end) {
> > +		if (res->end < new_start) {
> >  			p = &res->sibling;
> >  			continue;
> >  		}
> > @@ -1271,42 +1272,54 @@ int release_mem_region_adjustable(struct
> resource *parent,
> >  		}
> >
> >  		/* found the target resource; let's adjust accordingly */
> > -		if (res->start == start && res->end == end) {
> > +		if (res->start == new_start && res->end == end) {
> >  			/* free the whole entry */
> >  			*p = res->sibling;
> >  			free_resource(res);
> >  			ret = 0;
> > -		} else if (res->start == start && res->end != end) {
> > -			/* adjust the start */
> > -			ret = __adjust_resource(res, end + 1,
> > -						res->end - end);
> > -		} else if (res->start != start && res->end == end) {
> > -			/* adjust the end */
> > -			ret = __adjust_resource(res, res->start,
> > -						start - res->start);
> > +		} else if (res->end > end) {
> > +			if (res->start >= new_start) {
> > +				/* adjust the start */
> > +				ret = __adjust_resource(res, end + 1,
> > +							res->end - end);
> > +			} else {
> > +				/* split into two entries */
> > +				if (!new_res) {
> > +					ret = -ENOMEM;
> > +					break;
> > +				}
> > +				new_res->name = res->name;
> > +				new_res->start = end + 1;
> > +				new_res->end = res->end;
> > +				new_res->flags = res->flags;
> > +				new_res->desc = res->desc;
> > +				new_res->parent = res->parent;
> > +				new_res->sibling = res->sibling;
> > +				new_res->child = NULL;
> > +
> > +				ret = __adjust_resource(res, res->start,
> > +							new_start - res->start);
> > +				if (ret)
> > +					break;
> > +				res->sibling = new_res;
> > +				new_res = NULL;
> > +			}
> >  		} else {
> > -			/* split into two entries */
> > -			if (!new_res) {
> > -				ret = -ENOMEM;
> > -				break;
> > +			if (res->start < new_start) {
> > +				/* adjust the end */
> > +				ret = __adjust_resource(res, res->start,
> > +							new_start - res->start);
> > +				new_start = res->end+1;
> > +				p = &res->sibling;
> > +			} else {
> > +				new_start = res->end+1;
> > +				*p = res->sibling;
> > +				free_resource(res);
> > +				ret = 0;
> >  			}
> > -			new_res->name = res->name;
> > -			new_res->start = end + 1;
> > -			new_res->end = res->end;
> > -			new_res->flags = res->flags;
> > -			new_res->desc = res->desc;
> > -			new_res->parent = res->parent;
> > -			new_res->sibling = res->sibling;
> > -			new_res->child = NULL;
> > -
> > -			ret = __adjust_resource(res, res->start,
> > -						start - res->start);
> > -			if (ret)
> > -				break;
> > -			res->sibling = new_res;
> > -			new_res = NULL;
> > +			if (res->end < end)
> > +				continue;
> >  		}
> > -
> >  		break;
> >  	}
> >
> > --
> > 2.12.3
> >


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
  2018-06-22 12:01 ` guomin chen
  (?)
@ 2018-06-22 23:19 ` Bjorn Helgaas
  2018-06-23  5:53   ` Guomin Chen
  2018-06-25 23:55   ` Kani, Toshi
  -1 siblings, 2 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2018-06-22 23:19 UTC (permalink / raw)
  To: guomin chen
  Cc: Thomas Gleixner, Brijesh Singh, Borislav Petkov, Andrew Morton,
	Takashi Iwai, Lee Chun-Yi, linux-efi, linux-kernel, Tom Lendacky,
	Bjorn Helgaas, Yaowei Bai, Dan Williams, Toshi Kani

[+cc Toshi]

On Fri, Jun 22, 2018 at 08:01:38PM +0800, guomin chen wrote:
> We've got a bug report indicating the hot-remove node resource
> release failed,when the memory on this node is divided into
> several sections.because the release_mem_region_adjustable()
> can only release one resource that must be [start,end].

Can you please include a URL for the bug report?  That's useful for
additional details and gives hints about how future changes in this
area might be tested.

release_mem_region_adjustable() and the only call to it were added
by Toshi (cc'd):

  825f787bb496 ("resource: add release_mem_region_adjustable()")
  fe74ebb106a5 ("mm: change __remove_pages() to call release_mem_region_adjustable()")

> In my case, the BIOS supports faulty memory isolation. if BIOS
> detected bad memory block, the BIOS will isolates this badblock.
> And set this badblock memory to EfiUnusableMemory in EFI memory map
> base on UEFI 2.7 spec.For example in my system, the memory range on
> node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
> detected the [8004e000000-8004e0fffff] is a badblock memory.
> So the memory on node2 seem like this:
>    		80000000000-8004dffffff : System RAM
> 		8004e000000-8004e0fffff : Unusable memory
> 		8004e100000-807ffffffff : System RAM
> 
> Now, when offline the cpu node2,the kernel will try to release
> ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this
> time, the kernel will release failed,and output error message:
> "Unable to release resource <0x0000080000000000-0x00000807ffffffff>
> (-22)".
> Because the release_mem_region_adjustable() can only release one
> resource that must be [0x0000080000000000 , 0x00000807ffffffff].
> but now,the iomem resource on node2 [0x0000080000000000,
> 0x00000807ffffffff] are divided into three resources [80000000000-
> 8004dffffff],[8004e000000-8004e0fffff]and[8004e100000-807ffffffff].
> 
> This patch help to Release multiple iomem resources at once when
> node hot-remove. Such as in above case, when hot-remove the cpu
> node2,the kernel will try to release resource [0x0000080000000000-
> 0x00000807ffffffff].And this patch will release three resources
> [80000000000-8004dffffff],[8004e000000-8004e0fffff] and
> [8004e100000-807ffffffff].
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: linux-efi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Lee Chun-Yi <JLee@suse.com>
> Signed-off-by: guomin chen <guomin.chen@suse.com>
> ---
>  kernel/resource.c | 73 ++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 43 insertions(+), 30 deletions(-)
> 
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 30e1bc68503b..959bcce4c405 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1240,6 +1240,7 @@ int release_mem_region_adjustable(struct resource *parent,
>  	struct resource *res;
>  	struct resource *new_res;
>  	resource_size_t end;
> +	resource_size_t new_start = start;
>  	int ret = -EINVAL;
>  
>  	end = start + size - 1;
> @@ -1257,7 +1258,7 @@ int release_mem_region_adjustable(struct resource *parent,
>  			break;
>  
>  		/* look for the next resource if it does not fit into */
> -		if (res->start > start || res->end < end) {
> +		if (res->end < new_start) {
>  			p = &res->sibling;
>  			continue;
>  		}
> @@ -1271,42 +1272,54 @@ int release_mem_region_adjustable(struct resource *parent,
>  		}
>  
>  		/* found the target resource; let's adjust accordingly */
> -		if (res->start == start && res->end == end) {
> +		if (res->start == new_start && res->end == end) {
>  			/* free the whole entry */
>  			*p = res->sibling;
>  			free_resource(res);
>  			ret = 0;
> -		} else if (res->start == start && res->end != end) {
> -			/* adjust the start */
> -			ret = __adjust_resource(res, end + 1,
> -						res->end - end);
> -		} else if (res->start != start && res->end == end) {
> -			/* adjust the end */
> -			ret = __adjust_resource(res, res->start,
> -						start - res->start);
> +		} else if (res->end > end) {
> +			if (res->start >= new_start) {
> +				/* adjust the start */
> +				ret = __adjust_resource(res, end + 1,
> +							res->end - end);
> +			} else {
> +				/* split into two entries */
> +				if (!new_res) {
> +					ret = -ENOMEM;
> +					break;
> +				}
> +				new_res->name = res->name;
> +				new_res->start = end + 1;
> +				new_res->end = res->end;
> +				new_res->flags = res->flags;
> +				new_res->desc = res->desc;
> +				new_res->parent = res->parent;
> +				new_res->sibling = res->sibling;
> +				new_res->child = NULL;
> +
> +				ret = __adjust_resource(res, res->start,
> +							new_start - res->start);
> +				if (ret)
> +					break;
> +				res->sibling = new_res;
> +				new_res = NULL;
> +			}
>  		} else {
> -			/* split into two entries */
> -			if (!new_res) {
> -				ret = -ENOMEM;
> -				break;
> +			if (res->start < new_start) {
> +				/* adjust the end */
> +				ret = __adjust_resource(res, res->start,
> +							new_start - res->start);
> +				new_start = res->end+1;
> +				p = &res->sibling;
> +			} else {
> +				new_start = res->end+1;
> +				*p = res->sibling;
> +				free_resource(res);
> +				ret = 0;
>  			}
> -			new_res->name = res->name;
> -			new_res->start = end + 1;
> -			new_res->end = res->end;
> -			new_res->flags = res->flags;
> -			new_res->desc = res->desc;
> -			new_res->parent = res->parent;
> -			new_res->sibling = res->sibling;
> -			new_res->child = NULL;
> -
> -			ret = __adjust_resource(res, res->start,
> -						start - res->start);
> -			if (ret)
> -				break;
> -			res->sibling = new_res;
> -			new_res = NULL;
> +			if (res->end < end)
> +				continue;
>  		}
> -
>  		break;
>  	}
>  
> -- 
> 2.12.3
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
@ 2018-06-22 12:01 ` guomin chen
  0 siblings, 0 replies; 9+ messages in thread
From: guomin chen @ 2018-06-22 12:01 UTC (permalink / raw)
  To: Thomas Gleixner, Brijesh Singh, Borislav Petkov, Andrew Morton
  Cc: Takashi Iwai, Lee Chun-Yi, linux-efi, linux-kernel, guomin chen,
	Tom Lendacky, Bjorn Helgaas, Yaowei Bai, Dan Williams

We've got a bug report indicating the hot-remove node resource
release failed,when the memory on this node is divided into
several sections.because the release_mem_region_adjustable()
can only release one resource that must be [start,end].

In my case, the BIOS supports faulty memory isolation. if BIOS
detected bad memory block, the BIOS will isolates this badblock.
And set this badblock memory to EfiUnusableMemory in EFI memory map
base on UEFI 2.7 spec.For example in my system, the memory range on
node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
detected the [8004e000000-8004e0fffff] is a badblock memory.
So the memory on node2 seem like this:
   		80000000000-8004dffffff : System RAM
		8004e000000-8004e0fffff : Unusable memory
		8004e100000-807ffffffff : System RAM

Now, when offline the cpu node2,the kernel will try to release
ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this
time, the kernel will release failed,and output error message:
"Unable to release resource <0x0000080000000000-0x00000807ffffffff>
(-22)".
Because the release_mem_region_adjustable() can only release one
resource that must be [0x0000080000000000 , 0x00000807ffffffff].
but now,the iomem resource on node2 [0x0000080000000000,
0x00000807ffffffff] are divided into three resources [80000000000-
8004dffffff],[8004e000000-8004e0fffff]and[8004e100000-807ffffffff].

This patch help to Release multiple iomem resources at once when
node hot-remove. Such as in above case, when hot-remove the cpu
node2,the kernel will try to release resource [0x0000080000000000-
0x00000807ffffffff].And this patch will release three resources
[80000000000-8004dffffff],[8004e000000-8004e0fffff] and
[8004e100000-807ffffffff].

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Lee Chun-Yi <JLee@suse.com>
Signed-off-by: guomin chen <guomin.chen@suse.com>
---
 kernel/resource.c | 73 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 30e1bc68503b..959bcce4c405 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1240,6 +1240,7 @@ int release_mem_region_adjustable(struct resource *parent,
 	struct resource *res;
 	struct resource *new_res;
 	resource_size_t end;
+	resource_size_t new_start = start;
 	int ret = -EINVAL;
 
 	end = start + size - 1;
@@ -1257,7 +1258,7 @@ int release_mem_region_adjustable(struct resource *parent,
 			break;
 
 		/* look for the next resource if it does not fit into */
-		if (res->start > start || res->end < end) {
+		if (res->end < new_start) {
 			p = &res->sibling;
 			continue;
 		}
@@ -1271,42 +1272,54 @@ int release_mem_region_adjustable(struct resource *parent,
 		}
 
 		/* found the target resource; let's adjust accordingly */
-		if (res->start == start && res->end == end) {
+		if (res->start == new_start && res->end == end) {
 			/* free the whole entry */
 			*p = res->sibling;
 			free_resource(res);
 			ret = 0;
-		} else if (res->start == start && res->end != end) {
-			/* adjust the start */
-			ret = __adjust_resource(res, end + 1,
-						res->end - end);
-		} else if (res->start != start && res->end == end) {
-			/* adjust the end */
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
+		} else if (res->end > end) {
+			if (res->start >= new_start) {
+				/* adjust the start */
+				ret = __adjust_resource(res, end + 1,
+							res->end - end);
+			} else {
+				/* split into two entries */
+				if (!new_res) {
+					ret = -ENOMEM;
+					break;
+				}
+				new_res->name = res->name;
+				new_res->start = end + 1;
+				new_res->end = res->end;
+				new_res->flags = res->flags;
+				new_res->desc = res->desc;
+				new_res->parent = res->parent;
+				new_res->sibling = res->sibling;
+				new_res->child = NULL;
+
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				if (ret)
+					break;
+				res->sibling = new_res;
+				new_res = NULL;
+			}
 		} else {
-			/* split into two entries */
-			if (!new_res) {
-				ret = -ENOMEM;
-				break;
+			if (res->start < new_start) {
+				/* adjust the end */
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				new_start = res->end+1;
+				p = &res->sibling;
+			} else {
+				new_start = res->end+1;
+				*p = res->sibling;
+				free_resource(res);
+				ret = 0;
 			}
-			new_res->name = res->name;
-			new_res->start = end + 1;
-			new_res->end = res->end;
-			new_res->flags = res->flags;
-			new_res->desc = res->desc;
-			new_res->parent = res->parent;
-			new_res->sibling = res->sibling;
-			new_res->child = NULL;
-
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
-			if (ret)
-				break;
-			res->sibling = new_res;
-			new_res = NULL;
+			if (res->end < end)
+				continue;
 		}
-
 		break;
 	}
 
-- 
2.12.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove.
@ 2018-06-22 12:01 ` guomin chen
  0 siblings, 0 replies; 9+ messages in thread
From: guomin chen @ 2018-06-22 12:01 UTC (permalink / raw)
  To: Thomas Gleixner, Brijesh Singh, Borislav Petkov, Andrew Morton
  Cc: Takashi Iwai, Lee Chun-Yi, linux-efi, linux-kernel, guomin chen,
	Tom Lendacky, Bjorn Helgaas, Yaowei Bai, Dan Williams

We've got a bug report indicating the hot-remove node resource
release failed,when the memory on this node is divided into
several sections.because the release_mem_region_adjustable()
can only release one resource that must be [start,end].

In my case, the BIOS supports faulty memory isolation. if BIOS
detected bad memory block, the BIOS will isolates this badblock.
And set this badblock memory to EfiUnusableMemory in EFI memory map
base on UEFI 2.7 spec.For example in my system, the memory range on
node2 is [mem 0x0000080000000000-0x00000807ffffffff].but the BIOS
detected the [8004e000000-8004e0fffff] is a badblock memory.
So the memory on node2 seem like this:
   		80000000000-8004dffffff : System RAM
		8004e000000-8004e0fffff : Unusable memory
		8004e100000-807ffffffff : System RAM

Now, when offline the cpu node2,the kernel will try to release
ioresource [mem 0x0000080000000000-0x00000807ffffffff]. at this
time, the kernel will release failed,and output error message:
"Unable to release resource <0x0000080000000000-0x00000807ffffffff>
(-22)".
Because the release_mem_region_adjustable() can only release one
resource that must be [0x0000080000000000 , 0x00000807ffffffff].
but now,the iomem resource on node2 [0x0000080000000000,
0x00000807ffffffff] are divided into three resources [80000000000-
8004dffffff],[8004e000000-8004e0fffff]and[8004e100000-807ffffffff].

This patch help to Release multiple iomem resources at once when
node hot-remove. Such as in above case, when hot-remove the cpu
node2,the kernel will try to release resource [0x0000080000000000-
0x00000807ffffffff].And this patch will release three resources
[80000000000-8004dffffff],[8004e000000-8004e0fffff] and
[8004e100000-807ffffffff].

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Lee Chun-Yi <JLee@suse.com>
Signed-off-by: guomin chen <guomin.chen@suse.com>
---
 kernel/resource.c | 73 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 30e1bc68503b..959bcce4c405 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1240,6 +1240,7 @@ int release_mem_region_adjustable(struct resource *parent,
 	struct resource *res;
 	struct resource *new_res;
 	resource_size_t end;
+	resource_size_t new_start = start;
 	int ret = -EINVAL;
 
 	end = start + size - 1;
@@ -1257,7 +1258,7 @@ int release_mem_region_adjustable(struct resource *parent,
 			break;
 
 		/* look for the next resource if it does not fit into */
-		if (res->start > start || res->end < end) {
+		if (res->end < new_start) {
 			p = &res->sibling;
 			continue;
 		}
@@ -1271,42 +1272,54 @@ int release_mem_region_adjustable(struct resource *parent,
 		}
 
 		/* found the target resource; let's adjust accordingly */
-		if (res->start == start && res->end == end) {
+		if (res->start == new_start && res->end == end) {
 			/* free the whole entry */
 			*p = res->sibling;
 			free_resource(res);
 			ret = 0;
-		} else if (res->start == start && res->end != end) {
-			/* adjust the start */
-			ret = __adjust_resource(res, end + 1,
-						res->end - end);
-		} else if (res->start != start && res->end == end) {
-			/* adjust the end */
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
+		} else if (res->end > end) {
+			if (res->start >= new_start) {
+				/* adjust the start */
+				ret = __adjust_resource(res, end + 1,
+							res->end - end);
+			} else {
+				/* split into two entries */
+				if (!new_res) {
+					ret = -ENOMEM;
+					break;
+				}
+				new_res->name = res->name;
+				new_res->start = end + 1;
+				new_res->end = res->end;
+				new_res->flags = res->flags;
+				new_res->desc = res->desc;
+				new_res->parent = res->parent;
+				new_res->sibling = res->sibling;
+				new_res->child = NULL;
+
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				if (ret)
+					break;
+				res->sibling = new_res;
+				new_res = NULL;
+			}
 		} else {
-			/* split into two entries */
-			if (!new_res) {
-				ret = -ENOMEM;
-				break;
+			if (res->start < new_start) {
+				/* adjust the end */
+				ret = __adjust_resource(res, res->start,
+							new_start - res->start);
+				new_start = res->end+1;
+				p = &res->sibling;
+			} else {
+				new_start = res->end+1;
+				*p = res->sibling;
+				free_resource(res);
+				ret = 0;
 			}
-			new_res->name = res->name;
-			new_res->start = end + 1;
-			new_res->end = res->end;
-			new_res->flags = res->flags;
-			new_res->desc = res->desc;
-			new_res->parent = res->parent;
-			new_res->sibling = res->sibling;
-			new_res->child = NULL;
-
-			ret = __adjust_resource(res, res->start,
-						start - res->start);
-			if (ret)
-				break;
-			res->sibling = new_res;
-			new_res = NULL;
+			if (res->end < end)
+				continue;
 		}
-
 		break;
 	}
 
-- 
2.12.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-06-27  6:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-22 11:52 [PATCH 1/1] resource: Fixed iomem resource release failed on release_mem_region_adjustable() when memory node or cpu node hot-remove guomin chen
2018-06-22 11:52 ` guomin chen
2018-06-22 12:01 guomin chen
2018-06-22 12:01 ` guomin chen
2018-06-22 23:19 ` Bjorn Helgaas
2018-06-23  5:53   ` Guomin Chen
2018-06-25 17:21     ` Bjorn Helgaas
2018-06-25 23:55   ` Kani, Toshi
2018-06-27  6:43     ` Guomin Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.