[PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[AKASHI Takahiro]

Add memblock_cap_memory_range() which will remove all the memblock regions
except the range specified in the arguments.

This function, like memblock_mem_limit_remove_map(), will not remove
memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
later as "device memory."
See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
address the mem limit issue").

This function is used, in a succeeding patch in the series of arm64 kdump
suuport, to limit the range of usable memory, System RAM, on crash dump
kernel.
(Please note that "mem=" parameter is of little use for this purpose.)

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: linux-mm@kvack.org
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/memblock.h |  1 +
 mm/memblock.c            | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 5b759c9..0e770af 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
 phys_addr_t memblock_end_of_DRAM(void);
 void memblock_enforce_memory_limit(phys_addr_t memory_limit);
 void memblock_mem_limit_remove_map(phys_addr_t limit);
+void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
 bool memblock_is_memory(phys_addr_t addr);
 int memblock_is_map_memory(phys_addr_t addr);
 int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
diff --git a/mm/memblock.c b/mm/memblock.c
index 7608bc3..eb53876 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
 			      (phys_addr_t)ULLONG_MAX);
 }
 
+void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
+{
+	int start_rgn, end_rgn;
+	int i, ret;
+
+	if (!size)
+		return;
+
+	ret = memblock_isolate_range(&memblock.memory, base, size,
+						&start_rgn, &end_rgn);
+	if (ret)
+		return;
+
+	/* remove all the MAP regions */
+	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
+		if (!memblock_is_nomap(&memblock.memory.regions[i]))
+			memblock_remove_region(&memblock.memory, i);
+
+	for (i = start_rgn - 1; i >= 0; i--)
+		if (!memblock_is_nomap(&memblock.memory.regions[i]))
+			memblock_remove_region(&memblock.memory, i);
+
+	/* truncate the reserved regions */
+	memblock_remove_range(&memblock.reserved, 0, base);
+	memblock_remove_range(&memblock.reserved,
+			base + size, (phys_addr_t)ULLONG_MAX);
+}
+
 static int __init_memblock memblock_search(struct memblock_type *type, phys_addr_t addr)
 {
 	unsigned int left = 0, right = type->cnt;
-- 
2.10.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[Will Deacon]

On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> Add memblock_cap_memory_range() which will remove all the memblock regions
> except the range specified in the arguments.
> 
> This function, like memblock_mem_limit_remove_map(), will not remove
> memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
> later as "device memory."
> See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
> address the mem limit issue").
> 
> This function is used, in a succeeding patch in the series of arm64 kdump
> suuport, to limit the range of usable memory, System RAM, on crash dump
> kernel.
> (Please note that "mem=" parameter is of little use for this purpose.)
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: linux-mm@kvack.org
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  include/linux/memblock.h |  1 +
>  mm/memblock.c            | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 5b759c9..0e770af 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
>  phys_addr_t memblock_end_of_DRAM(void);
>  void memblock_enforce_memory_limit(phys_addr_t memory_limit);
>  void memblock_mem_limit_remove_map(phys_addr_t limit);
> +void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
>  bool memblock_is_memory(phys_addr_t addr);
>  int memblock_is_map_memory(phys_addr_t addr);
>  int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..eb53876 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
>  			      (phys_addr_t)ULLONG_MAX);
>  }
>  
> +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> +{
> +	int start_rgn, end_rgn;
> +	int i, ret;
> +
> +	if (!size)
> +		return;
> +
> +	ret = memblock_isolate_range(&memblock.memory, base, size,
> +						&start_rgn, &end_rgn);
> +	if (ret)
> +		return;
> +
> +	/* remove all the MAP regions */
> +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> +			memblock_remove_region(&memblock.memory, i);
> +
> +	for (i = start_rgn - 1; i >= 0; i--)
> +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> +			memblock_remove_region(&memblock.memory, i);
> +
> +	/* truncate the reserved regions */
> +	memblock_remove_range(&memblock.reserved, 0, base);
> +	memblock_remove_range(&memblock.reserved,
> +			base + size, (phys_addr_t)ULLONG_MAX);
> +}

This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
you not implement that in terms of your new, more general, function? e.g.
by passing base == 0, and size == limit?

Will

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[AKASHI Takahiro]

Will,
(+ Cc: Dennis)

On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > Add memblock_cap_memory_range() which will remove all the memblock regions
> > except the range specified in the arguments.
> > 
> > This function, like memblock_mem_limit_remove_map(), will not remove
> > memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
> > later as "device memory."
> > See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
> > address the mem limit issue").
> > 
> > This function is used, in a succeeding patch in the series of arm64 kdump
> > suuport, to limit the range of usable memory, System RAM, on crash dump
> > kernel.
> > (Please note that "mem=" parameter is of little use for this purpose.)
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Cc: linux-mm@kvack.org
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >  include/linux/memblock.h |  1 +
> >  mm/memblock.c            | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index 5b759c9..0e770af 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
> >  phys_addr_t memblock_end_of_DRAM(void);
> >  void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> >  void memblock_mem_limit_remove_map(phys_addr_t limit);
> > +void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> >  bool memblock_is_memory(phys_addr_t addr);
> >  int memblock_is_map_memory(phys_addr_t addr);
> >  int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index 7608bc3..eb53876 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
> >  			      (phys_addr_t)ULLONG_MAX);
> >  }
> >  
> > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > +{
> > +	int start_rgn, end_rgn;
> > +	int i, ret;
> > +
> > +	if (!size)
> > +		return;
> > +
> > +	ret = memblock_isolate_range(&memblock.memory, base, size,
> > +						&start_rgn, &end_rgn);
> > +	if (ret)
> > +		return;
> > +
> > +	/* remove all the MAP regions */
> > +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > +			memblock_remove_region(&memblock.memory, i);
> > +
> > +	for (i = start_rgn - 1; i >= 0; i--)
> > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > +			memblock_remove_region(&memblock.memory, i);
> > +
> > +	/* truncate the reserved regions */
> > +	memblock_remove_range(&memblock.reserved, 0, base);
> > +	memblock_remove_range(&memblock.reserved,
> > +			base + size, (phys_addr_t)ULLONG_MAX);
> > +}
> 
> This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> you not implement that in terms of your new, more general, function? e.g.
> by passing base == 0, and size == limit?

Obviously it's possible.
I actually talked to Dennis before about merging them,
but he was against my idea.

Thanks,
-Takahiro AKASHI

> Will

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[Dennis Chen]

On Fri, Nov 11, 2016 at 11:50:50AM +0900, AKASHI Takahiro wrote:
> Will,
> (+ Cc: Dennis)
> 
> On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> > On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > > Add memblock_cap_memory_range() which will remove all the memblock regions
> > > except the range specified in the arguments.
> > > 
> > > This function, like memblock_mem_limit_remove_map(), will not remove
> > > memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
> > > later as "device memory."
> > > See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
> > > address the mem limit issue").
> > > 
> > > This function is used, in a succeeding patch in the series of arm64 kdump
> > > suuport, to limit the range of usable memory, System RAM, on crash dump
> > > kernel.
> > > (Please note that "mem=" parameter is of little use for this purpose.)
> > > 
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Cc: linux-mm@kvack.org
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > ---
> > >  include/linux/memblock.h |  1 +
> > >  mm/memblock.c            | 28 ++++++++++++++++++++++++++++
> > >  2 files changed, 29 insertions(+)
> > > 
> > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > index 5b759c9..0e770af 100644
> > > --- a/include/linux/memblock.h
> > > +++ b/include/linux/memblock.h
> > > @@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
> > >  phys_addr_t memblock_end_of_DRAM(void);
> > >  void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > >  void memblock_mem_limit_remove_map(phys_addr_t limit);
> > > +void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> > >  bool memblock_is_memory(phys_addr_t addr);
> > >  int memblock_is_map_memory(phys_addr_t addr);
> > >  int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > index 7608bc3..eb53876 100644
> > > --- a/mm/memblock.c
> > > +++ b/mm/memblock.c
> > > @@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
> > >  			      (phys_addr_t)ULLONG_MAX);
> > >  }
> > >  
> > > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > > +{
> > > +	int start_rgn, end_rgn;
> > > +	int i, ret;
> > > +
> > > +	if (!size)
> > > +		return;
> > > +
> > > +	ret = memblock_isolate_range(&memblock.memory, base, size,
> > > +						&start_rgn, &end_rgn);
> > > +	if (ret)
> > > +		return;
> > > +
> > > +	/* remove all the MAP regions */
> > > +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > +			memblock_remove_region(&memblock.memory, i);
> > > +
> > > +	for (i = start_rgn - 1; i >= 0; i--)
> > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > +			memblock_remove_region(&memblock.memory, i);
> > > +
> > > +	/* truncate the reserved regions */
> > > +	memblock_remove_range(&memblock.reserved, 0, base);
> > > +	memblock_remove_range(&memblock.reserved,
> > > +			base + size, (phys_addr_t)ULLONG_MAX);
> > > +}
> > 
> > This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> > you not implement that in terms of your new, more general, function? e.g.
> > by passing base == 0, and size == limit?
> 
> Obviously it's possible.
> I actually talked to Dennis before about merging them,
> but he was against my idea.
>
Oops! I thought we have reached agreement in the thread:http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/442817.html
So feel free to do that as Will'll do
> 
> Thanks,
> -Takahiro AKASHI
> 
> > Will

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[AKASHI Takahiro]

On Fri, Nov 11, 2016 at 11:19:04AM +0800, Dennis Chen wrote:
> On Fri, Nov 11, 2016 at 11:50:50AM +0900, AKASHI Takahiro wrote:
> > Will,
> > (+ Cc: Dennis)
> > 
> > On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> > > On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > > > Add memblock_cap_memory_range() which will remove all the memblock regions
> > > > except the range specified in the arguments.
> > > > 
> > > > This function, like memblock_mem_limit_remove_map(), will not remove
> > > > memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
> > > > later as "device memory."
> > > > See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
> > > > address the mem limit issue").
> > > > 
> > > > This function is used, in a succeeding patch in the series of arm64 kdump
> > > > suuport, to limit the range of usable memory, System RAM, on crash dump
> > > > kernel.
> > > > (Please note that "mem=" parameter is of little use for this purpose.)
> > > > 
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > Cc: linux-mm@kvack.org
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > ---
> > > >  include/linux/memblock.h |  1 +
> > > >  mm/memblock.c            | 28 ++++++++++++++++++++++++++++
> > > >  2 files changed, 29 insertions(+)
> > > > 
> > > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > > > index 5b759c9..0e770af 100644
> > > > --- a/include/linux/memblock.h
> > > > +++ b/include/linux/memblock.h
> > > > @@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
> > > >  phys_addr_t memblock_end_of_DRAM(void);
> > > >  void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > > >  void memblock_mem_limit_remove_map(phys_addr_t limit);
> > > > +void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> > > >  bool memblock_is_memory(phys_addr_t addr);
> > > >  int memblock_is_map_memory(phys_addr_t addr);
> > > >  int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> > > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > > index 7608bc3..eb53876 100644
> > > > --- a/mm/memblock.c
> > > > +++ b/mm/memblock.c
> > > > @@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
> > > >  			      (phys_addr_t)ULLONG_MAX);
> > > >  }
> > > >  
> > > > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > > > +{
> > > > +	int start_rgn, end_rgn;
> > > > +	int i, ret;
> > > > +
> > > > +	if (!size)
> > > > +		return;
> > > > +
> > > > +	ret = memblock_isolate_range(&memblock.memory, base, size,
> > > > +						&start_rgn, &end_rgn);
> > > > +	if (ret)
> > > > +		return;
> > > > +
> > > > +	/* remove all the MAP regions */
> > > > +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > > +			memblock_remove_region(&memblock.memory, i);
> > > > +
> > > > +	for (i = start_rgn - 1; i >= 0; i--)
> > > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > > +			memblock_remove_region(&memblock.memory, i);
> > > > +
> > > > +	/* truncate the reserved regions */
> > > > +	memblock_remove_range(&memblock.reserved, 0, base);
> > > > +	memblock_remove_range(&memblock.reserved,
> > > > +			base + size, (phys_addr_t)ULLONG_MAX);
> > > > +}
> > > 
> > > This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> > > you not implement that in terms of your new, more general, function? e.g.
> > > by passing base == 0, and size == limit?
> > 
> > Obviously it's possible.
> > I actually talked to Dennis before about merging them,
> > but he was against my idea.
> >
> Oops! I thought we have reached agreement in the thread:http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/442817.html
> So feel free to do that as Will'll do

OK, but I found that the two functions have a bit different semantics
in clipping memory range, in particular, when the range [base,base+size)
goes across several regions with a gap.
(This does not happen in my arm64 kdump, though.)
That is, 'limit' in memblock_mem_limit_remove_map() means total size of
available memory, while 'size' in memblock_cap_memory_range() indicates
the size of _continuous_ memory range.

So I added an extra argument, exact, to a common function to specify
distinct behaviors. Confusing? Please see the patch below.

If nobody objects to this merge, I will submit a whole patchset of kdump
again.

Thanks,
-Takahiro AKASHI
===8<===
 include/linux/memblock.h |  1 +
 mm/memblock.c            | 91 +++++++++++++++++++++++++++++++++++-------------
 2 files changed, 68 insertions(+), 24 deletions(-)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 5b759c9..0e770af 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
 phys_addr_t memblock_end_of_DRAM(void);
 void memblock_enforce_memory_limit(phys_addr_t memory_limit);
 void memblock_mem_limit_remove_map(phys_addr_t limit);
+void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
 bool memblock_is_memory(phys_addr_t addr);
 int memblock_is_map_memory(phys_addr_t addr);
 int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
diff --git a/mm/memblock.c b/mm/memblock.c
index 7608bc3..5f849a9 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1473,9 +1473,10 @@ phys_addr_t __init_memblock memblock_end_of_DRAM(void)
 	return (memblock.memory.regions[idx].base + memblock.memory.regions[idx].size);
 }
 
-static phys_addr_t __init_memblock __find_max_addr(phys_addr_t limit)
+static phys_addr_t __init_memblock __find_max_addr(phys_addr_t min,
+							phys_addr_t limit)
 {
-	phys_addr_t max_addr = (phys_addr_t)ULLONG_MAX;
+	phys_addr_t max_addr = (phys_addr_t)ULLONG_MAX, base, size;
 	struct memblock_region *r;
 
 	/*
@@ -1484,11 +1485,22 @@ static phys_addr_t __init_memblock __find_max_addr(phys_addr_t limit)
 	 * of those regions, max_addr will keep original value ULLONG_MAX
 	 */
 	for_each_memblock(memory, r) {
-		if (limit <= r->size) {
-			max_addr = r->base + limit;
+		if (min >= r->base + r->size)
+			continue;
+
+		if (r->base <= min) {
+			base = min;
+			size = r->base + r->size - min;
+		} else {
+			base = r->base;
+			size = r->size;
+		}
+
+		if (limit <= size) {
+			max_addr = base + limit;
 			break;
 		}
-		limit -= r->size;
+		limit -= size;
 	}
 
 	return max_addr;
@@ -1501,7 +1513,7 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
 	if (!limit)
 		return;
 
-	max_addr = __find_max_addr(limit);
+	max_addr = __find_max_addr(0, limit);
 
 	/* @limit exceeds the total size of the memory, do nothing */
 	if (max_addr == (phys_addr_t)ULLONG_MAX)
@@ -1514,34 +1526,65 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
 			      (phys_addr_t)ULLONG_MAX);
 }
 
-void __init memblock_mem_limit_remove_map(phys_addr_t limit)
+/*
+ * __memblock_cap_memory_range - cap memblock regions
+ * @base: lowest address to clip
+ * @size: size of memory range
+ * @exact: true or false
+ *
+ * If @exact is true, the exact range [@base, @base+@size) of memory with
+ * kernel direct mapping is clipped from memblock.memory. Otherwise, total
+ * of @size of memory is clipped starting from @base.
+ */
+static void __init __memblock_cap_memory_range(phys_addr_t base,
+					phys_addr_t size, bool exact)
 {
-	struct memblock_type *type = &memblock.memory;
-	phys_addr_t max_addr;
-	int i, ret, start_rgn, end_rgn;
+	int start_rgn, end_rgn;
+	int i, ret;
 
-	if (!limit)
+	if (!size)
 		return;
 
-	max_addr = __find_max_addr(limit);
+	if (!exact) {
+		phys_addr_t max_addr;
 
-	/* @limit exceeds the total size of the memory, do nothing */
-	if (max_addr == (phys_addr_t)ULLONG_MAX)
-		return;
+		max_addr = __find_max_addr(base, size);
+		/* @size exceeds the total size of the memory, do nothing */
+		if (max_addr == (phys_addr_t)ULLONG_MAX)
+			return;
+
+		/* recalc the size to clip the exact range [@base, max_addr) */
+		size = max_addr  - base;
+	}
 
-	ret = memblock_isolate_range(type, max_addr, (phys_addr_t)ULLONG_MAX,
-				&start_rgn, &end_rgn);
+	ret = memblock_isolate_range(&memblock.memory, base, size,
+						&start_rgn, &end_rgn);
 	if (ret)
 		return;
 
-	/* remove all the MAP regions above the limit */
-	for (i = end_rgn - 1; i >= start_rgn; i--) {
-		if (!memblock_is_nomap(&type->regions[i]))
-			memblock_remove_region(type, i);
-	}
+	/* remove all the other MAP regions */
+	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
+		if (!memblock_is_nomap(&memblock.memory.regions[i]))
+			memblock_remove_region(&memblock.memory, i);
+
+	for (i = start_rgn - 1; i >= 0; i--)
+		if (!memblock_is_nomap(&memblock.memory.regions[i]))
+			memblock_remove_region(&memblock.memory, i);
+
 	/* truncate the reserved regions */
-	memblock_remove_range(&memblock.reserved, max_addr,
-			      (phys_addr_t)ULLONG_MAX);
+	memblock_remove_range(&memblock.reserved, 0, base);
+	memblock_remove_range(&memblock.reserved,
+			base + size, (phys_addr_t)ULLONG_MAX);
+}
+
+void __init memblock_mem_limit_remove_map(phys_addr_t limit)
+{
+	__memblock_cap_memory_range(0, limit, false);
+}
+
+void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
+{
+	__memblock_cap_memory_range(base, size, true);
 }
 
 static int __init_memblock memblock_search(struct memblock_type *type, phys_addr_t addr)
-- 
2.10.0

===>8===

> > 
> > Thanks,
> > -Takahiro AKASHI
> > 
> > > Will

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[Will Deacon]

Hi Akashi,

On Mon, Nov 14, 2016 at 02:55:16PM +0900, AKASHI Takahiro wrote:
> On Fri, Nov 11, 2016 at 11:19:04AM +0800, Dennis Chen wrote:
> > On Fri, Nov 11, 2016 at 11:50:50AM +0900, AKASHI Takahiro wrote:
> > > On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> > > > On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > > > > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > > > > +{
> > > > > +	int start_rgn, end_rgn;
> > > > > +	int i, ret;
> > > > > +
> > > > > +	if (!size)
> > > > > +		return;
> > > > > +
> > > > > +	ret = memblock_isolate_range(&memblock.memory, base, size,
> > > > > +						&start_rgn, &end_rgn);
> > > > > +	if (ret)
> > > > > +		return;
> > > > > +
> > > > > +	/* remove all the MAP regions */
> > > > > +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > > > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > > > +			memblock_remove_region(&memblock.memory, i);
> > > > > +
> > > > > +	for (i = start_rgn - 1; i >= 0; i--)
> > > > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > > > +			memblock_remove_region(&memblock.memory, i);
> > > > > +
> > > > > +	/* truncate the reserved regions */
> > > > > +	memblock_remove_range(&memblock.reserved, 0, base);
> > > > > +	memblock_remove_range(&memblock.reserved,
> > > > > +			base + size, (phys_addr_t)ULLONG_MAX);
> > > > > +}
> > > > 
> > > > This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> > > > you not implement that in terms of your new, more general, function? e.g.
> > > > by passing base == 0, and size == limit?
> > > 
> > > Obviously it's possible.
> > > I actually talked to Dennis before about merging them,
> > > but he was against my idea.
> > >
> > Oops! I thought we have reached agreement in the thread:http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/442817.html
> > So feel free to do that as Will'll do
> 
> OK, but I found that the two functions have a bit different semantics
> in clipping memory range, in particular, when the range [base,base+size)
> goes across several regions with a gap.
> (This does not happen in my arm64 kdump, though.)
> That is, 'limit' in memblock_mem_limit_remove_map() means total size of
> available memory, while 'size' in memblock_cap_memory_range() indicates
> the size of _continuous_ memory range.

I thought limit was just a physical address, and then
memblock_mem_limit_remove_map operated on the end of the nearest memblock?
You could leave the __find_max_addr call in memblock_mem_limit_remove_map,
given that I don't think you need/want it for memblock_cap_memory_range.

> So I added an extra argument, exact, to a common function to specify
> distinct behaviors. Confusing? Please see the patch below.

Oh yikes, this certainly wasn't what I had in mind! My observation was
just that memblock_mem_limit_remove_map(limit) does:


  1. memblock_isolate_range(limit - limit+ULLONG_MAX)
  2. memblock_remove_region(all non-nomap regions in the isolated region)
  3. truncate reserved regions to limit

and your memblock_cap_memory_range(base, size) does:

  1. memblock_isolate_range(base - base+size)
  2, memblock_remove_region(all non-nomap regions above and below the
     isolated region)
  3. truncate reserved regions around the isolated region

so, assuming we can invert the isolation in one of the cases, then they
could share the same underlying implementation.

I'm probably just missing something here, because the patch you've ended
up with is far more involved than I anticipated...

Will

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[AKASHI Takahiro]

Will,

On Wed, Nov 16, 2016 at 04:30:15PM +0000, Will Deacon wrote:
> Hi Akashi,
> 
> On Mon, Nov 14, 2016 at 02:55:16PM +0900, AKASHI Takahiro wrote:
> > On Fri, Nov 11, 2016 at 11:19:04AM +0800, Dennis Chen wrote:
> > > On Fri, Nov 11, 2016 at 11:50:50AM +0900, AKASHI Takahiro wrote:
> > > > On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> > > > > On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > > > > > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > > > > > +{
> > > > > > +	int start_rgn, end_rgn;
> > > > > > +	int i, ret;
> > > > > > +
> > > > > > +	if (!size)
> > > > > > +		return;
> > > > > > +
> > > > > > +	ret = memblock_isolate_range(&memblock.memory, base, size,
> > > > > > +						&start_rgn, &end_rgn);
> > > > > > +	if (ret)
> > > > > > +		return;
> > > > > > +
> > > > > > +	/* remove all the MAP regions */
> > > > > > +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > > > > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > > > > +			memblock_remove_region(&memblock.memory, i);
> > > > > > +
> > > > > > +	for (i = start_rgn - 1; i >= 0; i--)
> > > > > > +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > > > > > +			memblock_remove_region(&memblock.memory, i);
> > > > > > +
> > > > > > +	/* truncate the reserved regions */
> > > > > > +	memblock_remove_range(&memblock.reserved, 0, base);
> > > > > > +	memblock_remove_range(&memblock.reserved,
> > > > > > +			base + size, (phys_addr_t)ULLONG_MAX);
> > > > > > +}
> > > > > 
> > > > > This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> > > > > you not implement that in terms of your new, more general, function? e.g.
> > > > > by passing base == 0, and size == limit?
> > > > 
> > > > Obviously it's possible.
> > > > I actually talked to Dennis before about merging them,
> > > > but he was against my idea.
> > > >
> > > Oops! I thought we have reached agreement in the thread:http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/442817.html
> > > So feel free to do that as Will'll do
> > 
> > OK, but I found that the two functions have a bit different semantics
> > in clipping memory range, in particular, when the range [base,base+size)
> > goes across several regions with a gap.
> > (This does not happen in my arm64 kdump, though.)
> > That is, 'limit' in memblock_mem_limit_remove_map() means total size of
> > available memory, while 'size' in memblock_cap_memory_range() indicates
> > the size of _continuous_ memory range.
> 
> I thought limit was just a physical address, and then

No, it's not.

> memblock_mem_limit_remove_map operated on the end of the nearest memblock?

No, but "max_addr" returned by __find_max_addr() is a physical address
and the end address of memory of "limit" size in total.

> You could leave the __find_max_addr call in memblock_mem_limit_remove_map,
> given that I don't think you need/want it for memblock_cap_memory_range.
> 
> > So I added an extra argument, exact, to a common function to specify
> > distinct behaviors. Confusing? Please see the patch below.
> 
> Oh yikes, this certainly wasn't what I had in mind! My observation was
> just that memblock_mem_limit_remove_map(limit) does:
> 
> 
>   1. memblock_isolate_range(limit - limit+ULLONG_MAX)
>   2. memblock_remove_region(all non-nomap regions in the isolated region)
>   3. truncate reserved regions to limit
> 
> and your memblock_cap_memory_range(base, size) does:
> 
>   1. memblock_isolate_range(base - base+size)
>   2, memblock_remove_region(all non-nomap regions above and below the
>      isolated region)
>   3. truncate reserved regions around the isolated region
> 
> so, assuming we can invert the isolation in one of the cases, then they
> could share the same underlying implementation.

Please see my simplified patch below which would explain what I meant.
(Note that the size is calculated by 'max_addr - 0'.)

> I'm probably just missing something here, because the patch you've ended
> up with is far more involved than I anticipated...

I hope that it will meet almost your anticipation.

Thanks,
-Takahiro AKASHI

> 
> Will
===8<===
diff --git a/mm/memblock.c b/mm/memblock.c
index 7608bc3..fea1688 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1514,11 +1514,37 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
 			      (phys_addr_t)ULLONG_MAX);
 }
 
+void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
+{
+	int start_rgn, end_rgn;
+	int i, ret;
+
+	if (!size)
+		return;
+
+	ret = memblock_isolate_range(&memblock.memory, base, size,
+						&start_rgn, &end_rgn);
+	if (ret)
+		return;
+
+	/* remove all the MAP regions */
+	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
+		if (!memblock_is_nomap(&memblock.memory.regions[i]))
+			memblock_remove_region(&memblock.memory, i);
+
+	for (i = start_rgn - 1; i >= 0; i--)
+		if (!memblock_is_nomap(&memblock.memory.regions[i]))
+			memblock_remove_region(&memblock.memory, i);
+
+	/* truncate the reserved regions */
+	memblock_remove_range(&memblock.reserved, 0, base);
+	memblock_remove_range(&memblock.reserved,
+			base + size, (phys_addr_t)ULLONG_MAX);
+}
+
 void __init memblock_mem_limit_remove_map(phys_addr_t limit)
 {
-	struct memblock_type *type = &memblock.memory;
 	phys_addr_t max_addr;
-	int i, ret, start_rgn, end_rgn;
 
 	if (!limit)
 		return;
@@ -1529,19 +1555,7 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
 	if (max_addr == (phys_addr_t)ULLONG_MAX)
 		return;
 
-	ret = memblock_isolate_range(type, max_addr, (phys_addr_t)ULLONG_MAX,
-				&start_rgn, &end_rgn);
-	if (ret)
-		return;
-
-	/* remove all the MAP regions above the limit */
-	for (i = end_rgn - 1; i >= start_rgn; i--) {
-		if (!memblock_is_nomap(&type->regions[i]))
-			memblock_remove_region(type, i);
-	}
-	/* truncate the reserved regions */
-	memblock_remove_range(&memblock.reserved, max_addr,
-			      (phys_addr_t)ULLONG_MAX);
+	memblock_cap_memory_range(0, max_addr);
 }
 
 static int __init_memblock memblock_search(struct memblock_type *type, phys_addr_t addr)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[Will Deacon]

Hi Akashi,

On Thu, Nov 17, 2016 at 02:34:24PM +0900, AKASHI Takahiro wrote:
> On Wed, Nov 16, 2016 at 04:30:15PM +0000, Will Deacon wrote:
> > I thought limit was just a physical address, and then
> 
> No, it's not.

Quite right, it's a size. Sorry about that.

> > memblock_mem_limit_remove_map operated on the end of the nearest memblock?
> 
> No, but "max_addr" returned by __find_max_addr() is a physical address
> and the end address of memory of "limit" size in total.
> 
> > You could leave the __find_max_addr call in memblock_mem_limit_remove_map,
> > given that I don't think you need/want it for memblock_cap_memory_range.
> > 
> > > So I added an extra argument, exact, to a common function to specify
> > > distinct behaviors. Confusing? Please see the patch below.
> > 
> > Oh yikes, this certainly wasn't what I had in mind! My observation was
> > just that memblock_mem_limit_remove_map(limit) does:
> > 
> > 
> >   1. memblock_isolate_range(limit - limit+ULLONG_MAX)
> >   2. memblock_remove_region(all non-nomap regions in the isolated region)
> >   3. truncate reserved regions to limit
> > 
> > and your memblock_cap_memory_range(base, size) does:
> > 
> >   1. memblock_isolate_range(base - base+size)
> >   2, memblock_remove_region(all non-nomap regions above and below the
> >      isolated region)
> >   3. truncate reserved regions around the isolated region
> > 
> > so, assuming we can invert the isolation in one of the cases, then they
> > could share the same underlying implementation.
> 
> Please see my simplified patch below which would explain what I meant.
> (Note that the size is calculated by 'max_addr - 0'.)
> 
> > I'm probably just missing something here, because the patch you've ended
> > up with is far more involved than I anticipated...
> 
> I hope that it will meet almost your anticipation.

It looks much better, thanks! Just one question below.

> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..fea1688 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1514,11 +1514,37 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
>  			      (phys_addr_t)ULLONG_MAX);
>  }
>  
> +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> +{
> +	int start_rgn, end_rgn;
> +	int i, ret;
> +
> +	if (!size)
> +		return;
> +
> +	ret = memblock_isolate_range(&memblock.memory, base, size,
> +						&start_rgn, &end_rgn);
> +	if (ret)
> +		return;
> +
> +	/* remove all the MAP regions */
> +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> +			memblock_remove_region(&memblock.memory, i);

In the case that we have only one, giant memblock that covers base all
of base + size, can't we end up with start_rgn = end_rgn = 0? In which
case, we'd end up accidentally removing the map regions here.

The existing code:

> -	/* remove all the MAP regions above the limit */
> -	for (i = end_rgn - 1; i >= start_rgn; i--) {
> -		if (!memblock_is_nomap(&type->regions[i]))
> -			memblock_remove_region(type, i);
> -	}

seems to handle this.

Will

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[James Morse]

Hi Will, Akashi,

On 17/11/16 11:19, Will Deacon wrote:
> It looks much better, thanks! Just one question below.
> 

> On Thu, Nov 17, 2016 at 02:34:24PM +0900, AKASHI Takahiro wrote:
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index 7608bc3..fea1688 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -1514,11 +1514,37 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
>>  			      (phys_addr_t)ULLONG_MAX);
>>  }
>>  
>> +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
>> +{
>> +	int start_rgn, end_rgn;
>> +	int i, ret;
>> +
>> +	if (!size)
>> +		return;
>> +
>> +	ret = memblock_isolate_range(&memblock.memory, base, size,
>> +						&start_rgn, &end_rgn);
>> +	if (ret)
>> +		return;
>> +
>> +	/* remove all the MAP regions */
>> +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
>> +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
>> +			memblock_remove_region(&memblock.memory, i);
> 
> In the case that we have only one, giant memblock that covers base all
> of base + size, can't we end up with start_rgn = end_rgn = 0? In which

Can this happen? If we only have one memblock that exactly spans
base:(base+size), memblock_isolate_range() will hit the '@rgn is fully
contained, record it' code and set start_rgn=0,end_rgn=1. (rbase==base,
rend==end). We only go round the loop once.

If we only have one memblock that is bigger than base:(base+size) we end up with
three regions, start_rgn=1,end_rgn=2. The trickery here is the '@rgn intersects
from above' code decreases the loop counter so we process the same entry twice,
hitting '@rgn is fully contained, record it' the second time round... so we go
round the loop four times.

I can't see how we hit the:
> 	if (rbase >= end)
> 		break;
> 	if (rend <= base)
> 		continue;

code in either case...



Thanks,

James


> case, we'd end up accidentally removing the map regions here.
> 
> The existing code:
> 
>> -	/* remove all the MAP regions above the limit */
>> -	for (i = end_rgn - 1; i >= start_rgn; i--) {
>> -		if (!memblock_is_nomap(&type->regions[i]))
>> -			memblock_remove_region(type, i);
>> -	}
> 
> seems to handle this.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[AKASHI Takahiro]

James,

On Thu, Nov 17, 2016 at 06:00:58PM +0000, James Morse wrote:
> Hi Will, Akashi,
> 
> On 17/11/16 11:19, Will Deacon wrote:
> > It looks much better, thanks! Just one question below.
> > 
> 
> > On Thu, Nov 17, 2016 at 02:34:24PM +0900, AKASHI Takahiro wrote:
> >> diff --git a/mm/memblock.c b/mm/memblock.c
> >> index 7608bc3..fea1688 100644
> >> --- a/mm/memblock.c
> >> +++ b/mm/memblock.c
> >> @@ -1514,11 +1514,37 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
> >>  			      (phys_addr_t)ULLONG_MAX);
> >>  }
> >>  
> >> +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> >> +{
> >> +	int start_rgn, end_rgn;
> >> +	int i, ret;
> >> +
> >> +	if (!size)
> >> +		return;
> >> +
> >> +	ret = memblock_isolate_range(&memblock.memory, base, size,
> >> +						&start_rgn, &end_rgn);
> >> +	if (ret)
> >> +		return;
> >> +
> >> +	/* remove all the MAP regions */
> >> +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> >> +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> >> +			memblock_remove_region(&memblock.memory, i);
> > 
> > In the case that we have only one, giant memblock that covers base all
> > of base + size, can't we end up with start_rgn = end_rgn = 0? In which
> 
> Can this happen? If we only have one memblock that exactly spans
> base:(base+size), memblock_isolate_range() will hit the '@rgn is fully
> contained, record it' code and set start_rgn=0,end_rgn=1. (rbase==base,
> rend==end). We only go round the loop once.
> 
> If we only have one memblock that is bigger than base:(base+size) we end up with
> three regions, start_rgn=1,end_rgn=2. The trickery here is the '@rgn intersects
> from above' code decreases the loop counter so we process the same entry twice,
> hitting '@rgn is fully contained, record it' the second time round... so we go
> round the loop four times.

Thank you for your observation.

> I can't see how we hit the:
> > 	if (rbase >= end)
> > 		break;
> > 	if (rend <= base)
> > 		continue;
> 
> code in either case...

Right. So 'end_rgn' will never be expected to be 0 as far as some
intersection exists.

-Takahiro AKASHI

> 
> 
> Thanks,
> 
> James
> 
> 
> > case, we'd end up accidentally removing the map regions here.
> > 
> > The existing code:
> > 
> >> -	/* remove all the MAP regions above the limit */
> >> -	for (i = end_rgn - 1; i >= start_rgn; i--) {
> >> -		if (!memblock_is_nomap(&type->regions[i]))
> >> -			memblock_remove_region(type, i);
> >> -	}
> > 
> > seems to handle this.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v27 1/9] memblock: add memblock_cap_memory_range()

[Will Deacon]

On Thu, Nov 17, 2016 at 06:00:58PM +0000, James Morse wrote:
> On 17/11/16 11:19, Will Deacon wrote:
> > It looks much better, thanks! Just one question below.
> > 
> 
> > On Thu, Nov 17, 2016 at 02:34:24PM +0900, AKASHI Takahiro wrote:
> >> diff --git a/mm/memblock.c b/mm/memblock.c
> >> index 7608bc3..fea1688 100644
> >> --- a/mm/memblock.c
> >> +++ b/mm/memblock.c
> >> @@ -1514,11 +1514,37 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
> >>  			      (phys_addr_t)ULLONG_MAX);
> >>  }
> >>  
> >> +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> >> +{
> >> +	int start_rgn, end_rgn;
> >> +	int i, ret;
> >> +
> >> +	if (!size)
> >> +		return;
> >> +
> >> +	ret = memblock_isolate_range(&memblock.memory, base, size,
> >> +						&start_rgn, &end_rgn);
> >> +	if (ret)
> >> +		return;
> >> +
> >> +	/* remove all the MAP regions */
> >> +	for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> >> +		if (!memblock_is_nomap(&memblock.memory.regions[i]))
> >> +			memblock_remove_region(&memblock.memory, i);
> > 
> > In the case that we have only one, giant memblock that covers base all
> > of base + size, can't we end up with start_rgn = end_rgn = 0? In which
> 
> Can this happen? If we only have one memblock that exactly spans
> base:(base+size), memblock_isolate_range() will hit the '@rgn is fully
> contained, record it' code and set start_rgn=0,end_rgn=1. (rbase==base,
> rend==end). We only go round the loop once.
> 
> If we only have one memblock that is bigger than base:(base+size) we end up with
> three regions, start_rgn=1,end_rgn=2. The trickery here is the '@rgn intersects
> from above' code decreases the loop counter so we process the same entry twice,
> hitting '@rgn is fully contained, record it' the second time round... so we go
> round the loop four times.
> 
> I can't see how we hit the:
> > 	if (rbase >= end)
> > 		break;
> > 	if (rend <= base)
> > 		continue;
> 
> code in either case...

I consistently misread that as rend >= end and rbase <= base! In which case,
I agree with your analysis:

Reviewed-by: Will Deacon <will.deacon@arm.com>

The patch could probably still use an ack from an mm person.

Will

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>