[PATCH] mm: free reserved area's memmap if possiable

[PATCH] mm: free reserved area's memmap if possiable

[Will Deacon]

Adding linux-arm-kernel and devicetree (look for "raw-pfn"), since this
impacts directly on those.

On Mon, Feb 13, 2017 at 08:02:29PM +0800, zhouxianrong at huawei.com wrote:
> From: zhouxianrong <zhouxianrong@huawei.com>
> 
> just like freeing no-map area's memmap we could free reserved
> area's memmap as well only when user of reserved area indicate
> that we can do this in dts or drivers. that is, user of reserved
> area know how to use the reserved area who could not memblock_free
> or free_reserved_xxx the reserved area and regard the area as raw
> pfn usage. the patch supply a way to users who want to utilize the
> memmap memory corresponding to raw pfn reserved areas as many as
> possible.

I don't really understand this. Can you point me at a specific use-case,
please? Is CMA involved here?

Will

(entire patch follows, for the benefit of the extra CCs)

> Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
> ---
>  arch/arm64/mm/init.c         |   14 +++++++++++++-
>  drivers/of/fdt.c             |   31 +++++++++++++++++++++++--------
>  drivers/of/of_reserved_mem.c |   21 ++++++++++++++-------
>  include/linux/memblock.h     |    3 +++
>  include/linux/of_fdt.h       |    2 +-
>  mm/memblock.c                |   24 ++++++++++++++++++++++++
>  6 files changed, 78 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 380ebe7..7e62ef8 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -358,7 +358,7 @@ static inline void free_memmap(unsigned long start_pfn, unsigned long end_pfn)
>   */
>  static void __init free_unused_memmap(void)
>  {
> -	unsigned long start, prev_end = 0;
> +	unsigned long start, end, prev_end = 0;
>  	struct memblock_region *reg;
>  
>  	for_each_memblock(memory, reg) {
> @@ -391,6 +391,18 @@ static void __init free_unused_memmap(void)
>  	if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
>  		free_memmap(prev_end, ALIGN(prev_end, PAGES_PER_SECTION));
>  #endif
> +
> +	for_each_memblock(reserved, reg) {
> +		if (!(reg->flags & MEMBLOCK_RAW_PFN))
> +			continue;
> +
> +		start = memblock_region_memory_base_pfn(reg);
> +		end = round_down(memblock_region_memory_end_pfn(reg),
> +				 MAX_ORDER_NR_PAGES);
> +
> +		if (start < end)
> +			free_memmap(start, end);
> +	}
>  }
>  #endif	/* !CONFIG_SPARSEMEM_VMEMMAP */
>  
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c9b5cac..39e7474 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -582,7 +582,7 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
>  	phys_addr_t base, size;
>  	int len;
>  	const __be32 *prop;
> -	int nomap, first = 1;
> +	int nomap, raw_pfn, first = 1;
>  
>  	prop = of_get_flat_dt_prop(node, "reg", &len);
>  	if (!prop)
> @@ -595,13 +595,15 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
>  	}
>  
>  	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +	raw_pfn = of_get_flat_dt_prop(node, "raw-pfn", NULL) != NULL;
>  
>  	while (len >= t_len) {
>  		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
>  		size = dt_mem_next_cell(dt_root_size_cells, &prop);
>  
>  		if (size &&
> -		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> +		    early_init_dt_reserve_memory_arch(base, size, nomap,
> +				raw_pfn) == 0)
>  			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %ld MiB\n",
>  				uname, &base, (unsigned long)size / SZ_1M);
>  		else
> @@ -699,7 +701,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>  		fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
>  		if (!size)
>  			break;
> -		early_init_dt_reserve_memory_arch(base, size, 0);
> +		early_init_dt_reserve_memory_arch(base, size, 0, 0);
>  	}
>  
>  	of_scan_flat_dt(__fdt_scan_reserved_mem, NULL);
> @@ -717,6 +719,7 @@ void __init early_init_fdt_reserve_self(void)
>  	/* Reserve the dtb region */
>  	early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
>  					  fdt_totalsize(initial_boot_params),
> +					  0,
>  					  0);
>  }
>  
> @@ -1161,11 +1164,21 @@ int __init __weak early_init_dt_mark_hotplug_memory_arch(u64 base, u64 size)
>  }
>  
>  int __init __weak early_init_dt_reserve_memory_arch(phys_addr_t base,
> -					phys_addr_t size, bool nomap)
> +					phys_addr_t size, bool nomap,
> +					bool raw_pfn)
>  {
> +	int err;
> +
>  	if (nomap)
>  		return memblock_remove(base, size);
> -	return memblock_reserve(base, size);
> +
> +	err = memblock_reserve(base, size);
> +	if (err == 0) {
> +		if (raw_pfn)
> +			memblock_mark_raw_pfn(base, size);
> +	}
> +
> +	return err;
>  }
>  
>  /*
> @@ -1188,10 +1201,12 @@ int __init __weak early_init_dt_mark_hotplug_memory_arch(u64 base, u64 size)
>  }
>  
>  int __init __weak early_init_dt_reserve_memory_arch(phys_addr_t base,
> -					phys_addr_t size, bool nomap)
> +					phys_addr_t size, bool nomap,
> +					bool raw_pfn)
>  {
> -	pr_err("Reserved memory not supported, ignoring range %pa - %pa%s\n",
> -		  &base, &size, nomap ? " (nomap)" : "");
> +	pr_err("Reserved memory not supported, ignoring range %pa - %pa%s - %pa%s\n",
> +		  &base, &size, nomap ? " (nomap)" : "",
> +		  raw_pfn ? " (raw-pfn)" : "");
>  	return -ENOSYS;
>  }
>  
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 366d8c3..d7d9255 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -33,7 +33,7 @@
>  #include <linux/memblock.h>
>  int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  	phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
> -	phys_addr_t *res_base)
> +	bool raw_pfn, phys_addr_t *res_base)
>  {
>  	phys_addr_t base;
>  	/*
> @@ -56,15 +56,19 @@ int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  	*res_base = base;
>  	if (nomap)
>  		return memblock_remove(base, size);
> +
> +	if (raw_pfn)
> +		memblock_mark_raw_pfn(base, size);
> +
>  	return 0;
>  }
>  #else
>  int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  	phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
> -	phys_addr_t *res_base)
> +	bool raw_pfn, phys_addr_t *res_base)
>  {
> -	pr_err("Reserved memory not supported, ignoring region 0x%llx%s\n",
> -		  size, nomap ? " (nomap)" : "");
> +	pr_err("Reserved memory not supported, ignoring region 0x%llx%s 0x%llx%s\n",
> +		  size, nomap ? " (nomap)" : "", raw_pfn ? " (raw-pfn)" : "");
>  	return -ENOSYS;
>  }
>  #endif
> @@ -103,7 +107,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>  	phys_addr_t base = 0, align = 0, size;
>  	int len;
>  	const __be32 *prop;
> -	int nomap;
> +	int nomap, raw_pfn;
>  	int ret;
>  
>  	prop = of_get_flat_dt_prop(node, "size", &len);
> @@ -117,6 +121,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>  	size = dt_mem_next_cell(dt_root_size_cells, &prop);
>  
>  	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +	raw_pfn = of_get_flat_dt_prop(node, "raw-pfn", NULL) != NULL;
>  
>  	prop = of_get_flat_dt_prop(node, "alignment", &len);
>  	if (prop) {
> @@ -156,7 +161,8 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>  						       &prop);
>  
>  			ret = early_init_dt_alloc_reserved_memory_arch(size,
> -					align, start, end, nomap, &base);
> +					align, start, end, nomap,
> +					raw_pfn, &base);
>  			if (ret == 0) {
>  				pr_debug("allocated memory for '%s' node: base %pa, size %ld MiB\n",
>  					uname, &base,
> @@ -168,7 +174,8 @@ static int __init __reserved_mem_alloc_size(unsigned long node,
>  
>  	} else {
>  		ret = early_init_dt_alloc_reserved_memory_arch(size, align,
> -							0, 0, nomap, &base);
> +							0, 0, nomap,
> +							raw_pfn, &base);
>  		if (ret == 0)
>  			pr_debug("allocated memory for '%s' node: base %pa, size %ld MiB\n",
>  				uname, &base, (unsigned long)size / SZ_1M);
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 5b759c9..7266be1 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -26,6 +26,7 @@ enum {
>  	MEMBLOCK_HOTPLUG	= 0x1,	/* hotpluggable region */
>  	MEMBLOCK_MIRROR		= 0x2,	/* mirrored region */
>  	MEMBLOCK_NOMAP		= 0x4,	/* don't add to kernel direct mapping */
> +	MEMBLOCK_RAW_PFN	= 0x8,	/* raw pfn region's memmap never used */
>  };
>  
>  struct memblock_region {
> @@ -92,6 +93,8 @@ bool memblock_overlaps_region(struct memblock_type *type,
>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
> +int memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size);
> +int memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size);
>  ulong choose_memblock_flags(void);
>  
>  /* Low level functions */
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 271b3fd..29284d7 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -73,7 +73,7 @@ extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>  extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>  extern int early_init_dt_mark_hotplug_memory_arch(u64 base, u64 size);
>  extern int early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
> -					     bool no_map);
> +					     bool no_map, bool raw_pfn);
>  extern void * early_init_dt_alloc_memory_arch(u64 size, u64 align);
>  extern u64 dt_mem_next_cell(int s, const __be32 **cellp);
>  
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..c103b94 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -814,6 +814,30 @@ int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
>  }
>  
>  /**
> + * memblock_mark_raw_pfn - Mark raw pfn memory with flag MEMBLOCK_RAW_PFN.
> + * @base: the base phys addr of the region
> + * @size: the size of the region
> + *
> + * Return 0 on succees, -errno on failure.
> + */
> +int __init_memblock memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size)
> +{
> +	return memblock_setclr_flag(base, size, 1, MEMBLOCK_RAW_PFN);
> +}
> +
> +/**
> + * memblock_clear_raw_pfn - Clear flag MEMBLOCK_RAW_PFN for a specified region.
> + * @base: the base phys addr of the region
> + * @size: the size of the region
> + *
> + * Return 0 on succees, -errno on failure.
> + */
> +int __init_memblock memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size)
> +{
> +	return memblock_setclr_flag(base, size, 0, MEMBLOCK_RAW_PFN);
> +}
> +
> +/**
>   * __next_reserved_mem_region - next function for for_each_reserved_region()
>   * @idx: pointer to u64 loop variable
>   * @out_start: ptr to phys_addr_t for start address of the region, can be %NULL
> -- 
> 1.7.9.5
> 

[PATCH] mm: free reserved area's memmap if possiable

[Russell King - ARM Linux]

On Mon, Feb 13, 2017 at 12:17:00PM +0000, Will Deacon wrote:
> Adding linux-arm-kernel and devicetree (look for "raw-pfn"), since this
> impacts directly on those.
> 
> On Mon, Feb 13, 2017 at 08:02:29PM +0800, zhouxianrong at huawei.com wrote:
> > From: zhouxianrong <zhouxianrong@huawei.com>
> > 
> > just like freeing no-map area's memmap we could free reserved
> > area's memmap as well only when user of reserved area indicate
> > that we can do this in dts or drivers. that is, user of reserved
> > area know how to use the reserved area who could not memblock_free
> > or free_reserved_xxx the reserved area and regard the area as raw
> > pfn usage. the patch supply a way to users who want to utilize the
> > memmap memory corresponding to raw pfn reserved areas as many as
> > possible.
> 
> I don't really understand this. Can you point me at a specific use-case,
> please? Is CMA involved here?

You don't need "dt permission" to free the memmap page array for the
regions between memory areas.  In fact, adding a DT property for that
goes against the "DT describes the hardware not the implementation"
requirement, since the memmap page array is a Linux implementation
detail.

32-bit ARM has been freeing the memmap page array between memory areas
for years since pre-DT days, and continues to do so.  See
free_unused_memmap().

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] mm: free reserved area's memmap if possiable

[zhouxianrong at huawei.com]

From: zhouxianrong <zhouxianrong@huawei.com>

just like freeing no-map area's memmap (gaps of memblock.memory)
we could free reserved area's memmap (areas of memblock.reserved)
as well only when user of reserved area indicate that we can do
this in drivers. that is, user of reserved area know how to
use the reserved area who could not memblock_free or free_reserved_xxx
the reserved area and regard the area as raw pfn usage by kernel.
the patch supply a way to users who want to utilize the memmap
memory corresponding to raw pfn reserved areas as many as possible.
users can do this by memblock_mark_raw_pfn interface which mark the
reserved area as raw pfn and tell free_unused_memmap that this area's
memmap could be freeed.

Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
---
 arch/arm64/mm/init.c     |   14 +++++++++++++-
 include/linux/memblock.h |    3 +++
 mm/memblock.c            |   24 ++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 380ebe7..7e62ef8 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -358,7 +358,7 @@ static inline void free_memmap(unsigned long start_pfn, unsigned long end_pfn)
  */
 static void __init free_unused_memmap(void)
 {
-	unsigned long start, prev_end = 0;
+	unsigned long start, end, prev_end = 0;
 	struct memblock_region *reg;
 
 	for_each_memblock(memory, reg) {
@@ -391,6 +391,18 @@ static void __init free_unused_memmap(void)
 	if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
 		free_memmap(prev_end, ALIGN(prev_end, PAGES_PER_SECTION));
 #endif
+
+	for_each_memblock(reserved, reg) {
+		if (!(reg->flags & MEMBLOCK_RAW_PFN))
+			continue;
+
+		start = memblock_region_memory_base_pfn(reg);
+		end = round_down(memblock_region_memory_end_pfn(reg),
+				 MAX_ORDER_NR_PAGES);
+
+		if (start < end)
+			free_memmap(start, end);
+	}
 }
 #endif	/* !CONFIG_SPARSEMEM_VMEMMAP */
 
diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 5b759c9..9f8d277 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -26,6 +26,7 @@ enum {
 	MEMBLOCK_HOTPLUG	= 0x1,	/* hotpluggable region */
 	MEMBLOCK_MIRROR		= 0x2,	/* mirrored region */
 	MEMBLOCK_NOMAP		= 0x4,	/* don't add to kernel direct mapping */
+	MEMBLOCK_RAW_PFN	= 0x8,	/* region whose memmap never be used */
 };
 
 struct memblock_region {
@@ -92,6 +93,8 @@ bool memblock_overlaps_region(struct memblock_type *type,
 int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
 int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
 int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
+int memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size);
+int memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size);
 ulong choose_memblock_flags(void);
 
 /* Low level functions */
diff --git a/mm/memblock.c b/mm/memblock.c
index 7608bc3..c103b94 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -814,6 +814,30 @@ int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
 }
 
 /**
+ * memblock_mark_raw_pfn - Mark raw pfn memory with flag MEMBLOCK_RAW_PFN.
+ * @base: the base phys addr of the region
+ * @size: the size of the region
+ *
+ * Return 0 on succees, -errno on failure.
+ */
+int __init_memblock memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size)
+{
+	return memblock_setclr_flag(base, size, 1, MEMBLOCK_RAW_PFN);
+}
+
+/**
+ * memblock_clear_raw_pfn - Clear flag MEMBLOCK_RAW_PFN for a specified region.
+ * @base: the base phys addr of the region
+ * @size: the size of the region
+ *
+ * Return 0 on succees, -errno on failure.
+ */
+int __init_memblock memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size)
+{
+	return memblock_setclr_flag(base, size, 0, MEMBLOCK_RAW_PFN);
+}
+
+/**
  * __next_reserved_mem_region - next function for for_each_reserved_region()
  * @idx: pointer to u64 loop variable
  * @out_start: ptr to phys_addr_t for start address of the region, can be %NULL
-- 
1.7.9.5

[PATCH] mm: free reserved area's memmap if possiable

[zhouxianrong]

if the reserved area by user were so big which caused the memmap big,
and the reserved area's memamp did not be used by kernel, then user
could free the the reserved area's memamp by memblock_mark_raw_pfn
interface which is added by me.

On 2017/2/14 14:53, zhouxianrong at huawei.com wrote:
> From: zhouxianrong <zhouxianrong@huawei.com>
>
> just like freeing no-map area's memmap (gaps of memblock.memory)
> we could free reserved area's memmap (areas of memblock.reserved)
> as well only when user of reserved area indicate that we can do
> this in drivers. that is, user of reserved area know how to
> use the reserved area who could not memblock_free or free_reserved_xxx
> the reserved area and regard the area as raw pfn usage by kernel.
> the patch supply a way to users who want to utilize the memmap
> memory corresponding to raw pfn reserved areas as many as possible.
> users can do this by memblock_mark_raw_pfn interface which mark the
> reserved area as raw pfn and tell free_unused_memmap that this area's
> memmap could be freeed.
>
> Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
> ---
>  arch/arm64/mm/init.c     |   14 +++++++++++++-
>  include/linux/memblock.h |    3 +++
>  mm/memblock.c            |   24 ++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 380ebe7..7e62ef8 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -358,7 +358,7 @@ static inline void free_memmap(unsigned long start_pfn, unsigned long end_pfn)
>   */
>  static void __init free_unused_memmap(void)
>  {
> -	unsigned long start, prev_end = 0;
> +	unsigned long start, end, prev_end = 0;
>  	struct memblock_region *reg;
>
>  	for_each_memblock(memory, reg) {
> @@ -391,6 +391,18 @@ static void __init free_unused_memmap(void)
>  	if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
>  		free_memmap(prev_end, ALIGN(prev_end, PAGES_PER_SECTION));
>  #endif
> +
> +	for_each_memblock(reserved, reg) {
> +		if (!(reg->flags & MEMBLOCK_RAW_PFN))
> +			continue;
> +
> +		start = memblock_region_memory_base_pfn(reg);
> +		end = round_down(memblock_region_memory_end_pfn(reg),
> +				 MAX_ORDER_NR_PAGES);
> +
> +		if (start < end)
> +			free_memmap(start, end);
> +	}
>  }
>  #endif	/* !CONFIG_SPARSEMEM_VMEMMAP */
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 5b759c9..9f8d277 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -26,6 +26,7 @@ enum {
>  	MEMBLOCK_HOTPLUG	= 0x1,	/* hotpluggable region */
>  	MEMBLOCK_MIRROR		= 0x2,	/* mirrored region */
>  	MEMBLOCK_NOMAP		= 0x4,	/* don't add to kernel direct mapping */
> +	MEMBLOCK_RAW_PFN	= 0x8,	/* region whose memmap never be used */
>  };
>
>  struct memblock_region {
> @@ -92,6 +93,8 @@ bool memblock_overlaps_region(struct memblock_type *type,
>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
> +int memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size);
> +int memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size);
>  ulong choose_memblock_flags(void);
>
>  /* Low level functions */
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..c103b94 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -814,6 +814,30 @@ int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
>  }
>
>  /**
> + * memblock_mark_raw_pfn - Mark raw pfn memory with flag MEMBLOCK_RAW_PFN.
> + * @base: the base phys addr of the region
> + * @size: the size of the region
> + *
> + * Return 0 on succees, -errno on failure.
> + */
> +int __init_memblock memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size)
> +{
> +	return memblock_setclr_flag(base, size, 1, MEMBLOCK_RAW_PFN);
> +}
> +
> +/**
> + * memblock_clear_raw_pfn - Clear flag MEMBLOCK_RAW_PFN for a specified region.
> + * @base: the base phys addr of the region
> + * @size: the size of the region
> + *
> + * Return 0 on succees, -errno on failure.
> + */
> +int __init_memblock memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size)
> +{
> +	return memblock_setclr_flag(base, size, 0, MEMBLOCK_RAW_PFN);
> +}
> +
> +/**
>   * __next_reserved_mem_region - next function for for_each_reserved_region()
>   * @idx: pointer to u64 loop variable
>   * @out_start: ptr to phys_addr_t for start address of the region, can be %NULL
>

[PATCH] mm: free reserved area's memmap if possiable

[Ard Biesheuvel]

On 14 February 2017 at 06:53,  <zhouxianrong@huawei.com> wrote:
> From: zhouxianrong <zhouxianrong@huawei.com>
>
> just like freeing no-map area's memmap (gaps of memblock.memory)
> we could free reserved area's memmap (areas of memblock.reserved)
> as well only when user of reserved area indicate that we can do
> this in drivers. that is, user of reserved area know how to
> use the reserved area who could not memblock_free or free_reserved_xxx
> the reserved area and regard the area as raw pfn usage by kernel.
> the patch supply a way to users who want to utilize the memmap
> memory corresponding to raw pfn reserved areas as many as possible.
> users can do this by memblock_mark_raw_pfn interface which mark the
> reserved area as raw pfn and tell free_unused_memmap that this area's
> memmap could be freeed.
>

Could you give an example how much memory we actually recover by doing
this? I understand it depends on the size of the reserved regions, but
I'm sure you have an actual example that inspired you to write this
patch.

> Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
> ---
>  arch/arm64/mm/init.c     |   14 +++++++++++++-
>  include/linux/memblock.h |    3 +++
>  mm/memblock.c            |   24 ++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 380ebe7..7e62ef8 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -358,7 +358,7 @@ static inline void free_memmap(unsigned long start_pfn, unsigned long end_pfn)
>   */
>  static void __init free_unused_memmap(void)
>  {
> -       unsigned long start, prev_end = 0;
> +       unsigned long start, end, prev_end = 0;
>         struct memblock_region *reg;
>
>         for_each_memblock(memory, reg) {
> @@ -391,6 +391,18 @@ static void __init free_unused_memmap(void)
>         if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
>                 free_memmap(prev_end, ALIGN(prev_end, PAGES_PER_SECTION));
>  #endif
> +
> +       for_each_memblock(reserved, reg) {
> +               if (!(reg->flags & MEMBLOCK_RAW_PFN))
> +                       continue;
> +
> +               start = memblock_region_memory_base_pfn(reg);
> +               end = round_down(memblock_region_memory_end_pfn(reg),
> +                                MAX_ORDER_NR_PAGES);
> +

Why are you rounding down end only? Shouldn't you round up start and
round down end? Or does free_memmap() deal with that already?

In any case, it is good to emphasize that on 4 KB pagesize kernels, we
will only free multiples of 8 MB that are 8 MB aligned, resulting in
128 KB of memmap backing to be released.


> +               if (start < end)
> +                       free_memmap(start, end);
> +       }
>  }
>  #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 5b759c9..9f8d277 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -26,6 +26,7 @@ enum {
>         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
>         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
>         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct mapping */
> +       MEMBLOCK_RAW_PFN        = 0x8,  /* region whose memmap never be used */

I think we should be *very* careful about the combinatorial explosion
that results when combining all these flags, given that this is not a
proper enum but a bit field.

In any case, the generic memblock change should be in a separate patch
from the arm64 change.

>  };
>
>  struct memblock_region {
> @@ -92,6 +93,8 @@ bool memblock_overlaps_region(struct memblock_type *type,
>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
> +int memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size);
> +int memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size);
>  ulong choose_memblock_flags(void);
>
>  /* Low level functions */
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 7608bc3..c103b94 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -814,6 +814,30 @@ int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
>  }
>
>  /**
> + * memblock_mark_raw_pfn - Mark raw pfn memory with flag MEMBLOCK_RAW_PFN.
> + * @base: the base phys addr of the region
> + * @size: the size of the region
> + *
> + * Return 0 on succees, -errno on failure.
> + */
> +int __init_memblock memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size)
> +{
> +       return memblock_setclr_flag(base, size, 1, MEMBLOCK_RAW_PFN);
> +}
> +
> +/**
> + * memblock_clear_raw_pfn - Clear flag MEMBLOCK_RAW_PFN for a specified region.
> + * @base: the base phys addr of the region
> + * @size: the size of the region
> + *
> + * Return 0 on succees, -errno on failure.
> + */
> +int __init_memblock memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size)
> +{
> +       return memblock_setclr_flag(base, size, 0, MEMBLOCK_RAW_PFN);
> +}
> +
> +/**
>   * __next_reserved_mem_region - next function for for_each_reserved_region()
>   * @idx: pointer to u64 loop variable
>   * @out_start: ptr to phys_addr_t for start address of the region, can be %NULL
> --
> 1.7.9.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] mm: free reserved area's memmap if possiable

[zhouxianrong]

On 2017/2/14 17:03, Ard Biesheuvel wrote:
> On 14 February 2017 at 06:53,  <zhouxianrong@huawei.com> wrote:
>> From: zhouxianrong <zhouxianrong@huawei.com>
>>
>> just like freeing no-map area's memmap (gaps of memblock.memory)
>> we could free reserved area's memmap (areas of memblock.reserved)
>> as well only when user of reserved area indicate that we can do
>> this in drivers. that is, user of reserved area know how to
>> use the reserved area who could not memblock_free or free_reserved_xxx
>> the reserved area and regard the area as raw pfn usage by kernel.
>> the patch supply a way to users who want to utilize the memmap
>> memory corresponding to raw pfn reserved areas as many as possible.
>> users can do this by memblock_mark_raw_pfn interface which mark the
>> reserved area as raw pfn and tell free_unused_memmap that this area's
>> memmap could be freeed.
>>
>
> Could you give an example how much memory we actually recover by doing
> this? I understand it depends on the size of the reserved regions, but
> I'm sure you have an actual example that inspired you to write this
> patch.

i did statistics in our platform, the memmap of reserved region that can be freed
is about 6MB. it's fewer.

>
>> Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
>> ---
>>  arch/arm64/mm/init.c     |   14 +++++++++++++-
>>  include/linux/memblock.h |    3 +++
>>  mm/memblock.c            |   24 ++++++++++++++++++++++++
>>  3 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 380ebe7..7e62ef8 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -358,7 +358,7 @@ static inline void free_memmap(unsigned long start_pfn, unsigned long end_pfn)
>>   */
>>  static void __init free_unused_memmap(void)
>>  {
>> -       unsigned long start, prev_end = 0;
>> +       unsigned long start, end, prev_end = 0;
>>         struct memblock_region *reg;
>>
>>         for_each_memblock(memory, reg) {
>> @@ -391,6 +391,18 @@ static void __init free_unused_memmap(void)
>>         if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
>>                 free_memmap(prev_end, ALIGN(prev_end, PAGES_PER_SECTION));
>>  #endif
>> +
>> +       for_each_memblock(reserved, reg) {
>> +               if (!(reg->flags & MEMBLOCK_RAW_PFN))
>> +                       continue;
>> +
>> +               start = memblock_region_memory_base_pfn(reg);
>> +               end = round_down(memblock_region_memory_end_pfn(reg),
>> +                                MAX_ORDER_NR_PAGES);
>> +
>
> Why are you rounding down end only? Shouldn't you round up start and
> round down end? Or does free_memmap() deal with that already?

ok, i could round up start.

>
> In any case, it is good to emphasize that on 4 KB pagesize kernels, we
> will only free multiples of 8 MB that are 8 MB aligned, resulting in
> 128 KB of memmap backing to be released.


>
>
>> +               if (start < end)
>> +                       free_memmap(start, end);
>> +       }
>>  }
>>  #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
>>
>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>> index 5b759c9..9f8d277 100644
>> --- a/include/linux/memblock.h
>> +++ b/include/linux/memblock.h
>> @@ -26,6 +26,7 @@ enum {
>>         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
>>         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
>>         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct mapping */
>> +       MEMBLOCK_RAW_PFN        = 0x8,  /* region whose memmap never be used */
>
> I think we should be *very* careful about the combinatorial explosion
> that results when combining all these flags, given that this is not a
> proper enum but a bit field.
>
> In any case, the generic memblock change should be in a separate patch
> from the arm64 change.

MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP can not be set at the same time

>
>>  };
>>
>>  struct memblock_region {
>> @@ -92,6 +93,8 @@ bool memblock_overlaps_region(struct memblock_type *type,
>>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>> +int memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size);
>> +int memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size);
>>  ulong choose_memblock_flags(void);
>>
>>  /* Low level functions */
>> diff --git a/mm/memblock.c b/mm/memblock.c
>> index 7608bc3..c103b94 100644
>> --- a/mm/memblock.c
>> +++ b/mm/memblock.c
>> @@ -814,6 +814,30 @@ int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
>>  }
>>
>>  /**
>> + * memblock_mark_raw_pfn - Mark raw pfn memory with flag MEMBLOCK_RAW_PFN.
>> + * @base: the base phys addr of the region
>> + * @size: the size of the region
>> + *
>> + * Return 0 on succees, -errno on failure.
>> + */
>> +int __init_memblock memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size)
>> +{
>> +       return memblock_setclr_flag(base, size, 1, MEMBLOCK_RAW_PFN);
>> +}
>> +
>> +/**
>> + * memblock_clear_raw_pfn - Clear flag MEMBLOCK_RAW_PFN for a specified region.
>> + * @base: the base phys addr of the region
>> + * @size: the size of the region
>> + *
>> + * Return 0 on succees, -errno on failure.
>> + */
>> +int __init_memblock memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size)
>> +{
>> +       return memblock_setclr_flag(base, size, 0, MEMBLOCK_RAW_PFN);
>> +}
>> +
>> +/**
>>   * __next_reserved_mem_region - next function for for_each_reserved_region()
>>   * @idx: pointer to u64 loop variable
>>   * @out_start: ptr to phys_addr_t for start address of the region, can be %NULL
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> .
>

[PATCH] mm: free reserved area's memmap if possiable

[Ard Biesheuvel]

On 15 February 2017 at 01:44, zhouxianrong <zhouxianrong@huawei.com> wrote:
>
>
> On 2017/2/14 17:03, Ard Biesheuvel wrote:
>>
>> On 14 February 2017 at 06:53,  <zhouxianrong@huawei.com> wrote:
>>>
>>> From: zhouxianrong <zhouxianrong@huawei.com>
>>>
>>> just like freeing no-map area's memmap (gaps of memblock.memory)
>>> we could free reserved area's memmap (areas of memblock.reserved)
>>> as well only when user of reserved area indicate that we can do
>>> this in drivers. that is, user of reserved area know how to
>>> use the reserved area who could not memblock_free or free_reserved_xxx
>>> the reserved area and regard the area as raw pfn usage by kernel.
>>> the patch supply a way to users who want to utilize the memmap
>>> memory corresponding to raw pfn reserved areas as many as possible.
>>> users can do this by memblock_mark_raw_pfn interface which mark the
>>> reserved area as raw pfn and tell free_unused_memmap that this area's
>>> memmap could be freeed.
>>>
>>
>> Could you give an example how much memory we actually recover by doing
>> this? I understand it depends on the size of the reserved regions, but
>> I'm sure you have an actual example that inspired you to write this
>> patch.
>
>
> i did statistics in our platform, the memmap of reserved region that can be
> freed
> is about 6MB. it's fewer.
>

So if you round up the start and round down the end to 8 MB alignment,
as you do in this patch, how much do you end up freeing?

>
>>
>>> Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
>>> ---
>>>  arch/arm64/mm/init.c     |   14 +++++++++++++-
>>>  include/linux/memblock.h |    3 +++
>>>  mm/memblock.c            |   24 ++++++++++++++++++++++++
>>>  3 files changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>>> index 380ebe7..7e62ef8 100644
>>> --- a/arch/arm64/mm/init.c
>>> +++ b/arch/arm64/mm/init.c
>>> @@ -358,7 +358,7 @@ static inline void free_memmap(unsigned long
>>> start_pfn, unsigned long end_pfn)
>>>   */
>>>  static void __init free_unused_memmap(void)
>>>  {
>>> -       unsigned long start, prev_end = 0;
>>> +       unsigned long start, end, prev_end = 0;
>>>         struct memblock_region *reg;
>>>
>>>         for_each_memblock(memory, reg) {
>>> @@ -391,6 +391,18 @@ static void __init free_unused_memmap(void)
>>>         if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
>>>                 free_memmap(prev_end, ALIGN(prev_end,
>>> PAGES_PER_SECTION));
>>>  #endif
>>> +
>>> +       for_each_memblock(reserved, reg) {
>>> +               if (!(reg->flags & MEMBLOCK_RAW_PFN))
>>> +                       continue;
>>> +
>>> +               start = memblock_region_memory_base_pfn(reg);
>>> +               end = round_down(memblock_region_memory_end_pfn(reg),
>>> +                                MAX_ORDER_NR_PAGES);
>>> +
>>
>>
>> Why are you rounding down end only? Shouldn't you round up start and
>> round down end? Or does free_memmap() deal with that already?
>
>
> ok, i could round up start.
>

Yes, but is that necessary? Could you explain
a) why you round down end, and
b) why the reason under a) does not apply to start?

>>
>> In any case, it is good to emphasize that on 4 KB pagesize kernels, we
>> will only free multiples of 8 MB that are 8 MB aligned, resulting in
>> 128 KB of memmap backing to be released.
>
>
>
>>
>>
>>> +               if (start < end)
>>> +                       free_memmap(start, end);
>>> +       }
>>>  }
>>>  #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
>>>
>>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>>> index 5b759c9..9f8d277 100644
>>> --- a/include/linux/memblock.h
>>> +++ b/include/linux/memblock.h
>>> @@ -26,6 +26,7 @@ enum {
>>>         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
>>>         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
>>>         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct
>>> mapping */
>>> +       MEMBLOCK_RAW_PFN        = 0x8,  /* region whose memmap never be
>>> used */
>>
>>
>> I think we should be *very* careful about the combinatorial explosion
>> that results when combining all these flags, given that this is not a
>> proper enum but a bit field.
>>
>> In any case, the generic memblock change should be in a separate patch
>> from the arm64 change.
>
>
> MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP can not be set at the same time
>

They should not. But if I call  memblock_mark_raw_pfn() on a
MEMBLOCK_NOMAP region, it will have both flags set.

In summary, I don't think we need this patch. And if you can convince
us otherwise, you should really be more methodical and explicit in
implementing this RAW_PFN flag, not add it as a byproduct of the arch
code that uses it. Also, you should explain how RAW_PFN relates to
NOMAP, and ensure that RAW_PFN and NOMAP regions don't intersect if
that is an unsupported combination.

Regards,
Ard.


>>
>>>  };
>>>
>>>  struct memblock_region {
>>> @@ -92,6 +93,8 @@ bool memblock_overlaps_region(struct memblock_type
>>> *type,
>>>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>>>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>>> +int memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size);
>>> +int memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size);
>>>  ulong choose_memblock_flags(void);
>>>
>>>  /* Low level functions */
>>> diff --git a/mm/memblock.c b/mm/memblock.c
>>> index 7608bc3..c103b94 100644
>>> --- a/mm/memblock.c
>>> +++ b/mm/memblock.c
>>> @@ -814,6 +814,30 @@ int __init_memblock memblock_mark_nomap(phys_addr_t
>>> base, phys_addr_t size)
>>>  }
>>>
>>>  /**
>>> + * memblock_mark_raw_pfn - Mark raw pfn memory with flag
>>> MEMBLOCK_RAW_PFN.
>>> + * @base: the base phys addr of the region
>>> + * @size: the size of the region
>>> + *
>>> + * Return 0 on succees, -errno on failure.
>>> + */
>>> +int __init_memblock memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t
>>> size)
>>> +{
>>> +       return memblock_setclr_flag(base, size, 1, MEMBLOCK_RAW_PFN);
>>> +}
>>> +
>>> +/**
>>> + * memblock_clear_raw_pfn - Clear flag MEMBLOCK_RAW_PFN for a specified
>>> region.
>>> + * @base: the base phys addr of the region
>>> + * @size: the size of the region
>>> + *
>>> + * Return 0 on succees, -errno on failure.
>>> + */
>>> +int __init_memblock memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t
>>> size)
>>> +{
>>> +       return memblock_setclr_flag(base, size, 0, MEMBLOCK_RAW_PFN);
>>> +}
>>> +
>>> +/**
>>>   * __next_reserved_mem_region - next function for
>>> for_each_reserved_region()
>>>   * @idx: pointer to u64 loop variable
>>>   * @out_start: ptr to phys_addr_t for start address of the region, can
>>> be %NULL
>>> --
>>> 1.7.9.5
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>> .
>>
>

[PATCH] mm: free reserved area's memmap if possiable

[zhouxianrong]

On 2017/2/15 15:10, Ard Biesheuvel wrote:
> On 15 February 2017 at 01:44, zhouxianrong <zhouxianrong@huawei.com> wrote:
>>
>>
>> On 2017/2/14 17:03, Ard Biesheuvel wrote:
>>>
>>> On 14 February 2017 at 06:53,  <zhouxianrong@huawei.com> wrote:
>>>>
>>>> From: zhouxianrong <zhouxianrong@huawei.com>
>>>>
>>>> just like freeing no-map area's memmap (gaps of memblock.memory)
>>>> we could free reserved area's memmap (areas of memblock.reserved)
>>>> as well only when user of reserved area indicate that we can do
>>>> this in drivers. that is, user of reserved area know how to
>>>> use the reserved area who could not memblock_free or free_reserved_xxx
>>>> the reserved area and regard the area as raw pfn usage by kernel.
>>>> the patch supply a way to users who want to utilize the memmap
>>>> memory corresponding to raw pfn reserved areas as many as possible.
>>>> users can do this by memblock_mark_raw_pfn interface which mark the
>>>> reserved area as raw pfn and tell free_unused_memmap that this area's
>>>> memmap could be freeed.
>>>>
>>>
>>> Could you give an example how much memory we actually recover by doing
>>> this? I understand it depends on the size of the reserved regions, but
>>> I'm sure you have an actual example that inspired you to write this
>>> patch.
>>
>>
>> i did statistics in our platform, the memmap of reserved region that can be
>> freed
>> is about 6MB. it's fewer.
>>
>
> So if you round up the start and round down the end to 8 MB alignment,
> as you do in this patch, how much do you end up freeing?
>

yes, as you say, the size would get fewer if alignment to MAX_ORDER_NR_PAGES
in fact, we could not benifit more from this patch because most reserved regions are
cma (driver user want to multiplexing of memory with buddy) but cma regions could use struct
page so we can not free whose memmap. in our platform only few reserved regions
memmap could be freed. this patch aimed at some special big memory usages for reserved region.

>>
>>>
>>>> Signed-off-by: zhouxianrong <zhouxianrong@huawei.com>
>>>> ---
>>>>  arch/arm64/mm/init.c     |   14 +++++++++++++-
>>>>  include/linux/memblock.h |    3 +++
>>>>  mm/memblock.c            |   24 ++++++++++++++++++++++++
>>>>  3 files changed, 40 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>>>> index 380ebe7..7e62ef8 100644
>>>> --- a/arch/arm64/mm/init.c
>>>> +++ b/arch/arm64/mm/init.c
>>>> @@ -358,7 +358,7 @@ static inline void free_memmap(unsigned long
>>>> start_pfn, unsigned long end_pfn)
>>>>   */
>>>>  static void __init free_unused_memmap(void)
>>>>  {
>>>> -       unsigned long start, prev_end = 0;
>>>> +       unsigned long start, end, prev_end = 0;
>>>>         struct memblock_region *reg;
>>>>
>>>>         for_each_memblock(memory, reg) {
>>>> @@ -391,6 +391,18 @@ static void __init free_unused_memmap(void)
>>>>         if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
>>>>                 free_memmap(prev_end, ALIGN(prev_end,
>>>> PAGES_PER_SECTION));
>>>>  #endif
>>>> +
>>>> +       for_each_memblock(reserved, reg) {
>>>> +               if (!(reg->flags & MEMBLOCK_RAW_PFN))
>>>> +                       continue;
>>>> +
>>>> +               start = memblock_region_memory_base_pfn(reg);
>>>> +               end = round_down(memblock_region_memory_end_pfn(reg),
>>>> +                                MAX_ORDER_NR_PAGES);
>>>> +
>>>
>>>
>>> Why are you rounding down end only? Shouldn't you round up start and
>>> round down end? Or does free_memmap() deal with that already?
>>
>>
>> ok, i could round up start.
>>
>
> Yes, but is that necessary? Could you explain
> a) why you round down end, and
> b) why the reason under a) does not apply to start?

i made a mistake.

for freeing memblock.memory gap, we need alignment by MAX_ORDER_NR_PAGES due to bank ending alignment requirement

for freeing memblock.reserved area, because this is within bank, i think we do not need this alignment requirement.
but if alignment, rounding up start and rounding down end are natural for memblock.reserved.

>
>>>
>>> In any case, it is good to emphasize that on 4 KB pagesize kernels, we
>>> will only free multiples of 8 MB that are 8 MB aligned, resulting in
>>> 128 KB of memmap backing to be released.
>>
>>
>>
>>>
>>>
>>>> +               if (start < end)
>>>> +                       free_memmap(start, end);
>>>> +       }
>>>>  }
>>>>  #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
>>>>
>>>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>>>> index 5b759c9..9f8d277 100644
>>>> --- a/include/linux/memblock.h
>>>> +++ b/include/linux/memblock.h
>>>> @@ -26,6 +26,7 @@ enum {
>>>>         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
>>>>         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
>>>>         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct
>>>> mapping */
>>>> +       MEMBLOCK_RAW_PFN        = 0x8,  /* region whose memmap never be
>>>> used */
>>>
>>>
>>> I think we should be *very* careful about the combinatorial explosion
>>> that results when combining all these flags, given that this is not a
>>> proper enum but a bit field.
>>>
>>> In any case, the generic memblock change should be in a separate patch
>>> from the arm64 change.
>>
>>
>> MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP can not be set at the same time
>>
>
> They should not. But if I call  memblock_mark_raw_pfn() on a
> MEMBLOCK_NOMAP region, it will have both flags set.
>
> In summary, I don't think we need this patch. And if you can convince
> us otherwise, you should really be more methodical and explicit in
> implementing this RAW_PFN flag, not add it as a byproduct of the arch
> code that uses it. Also, you should explain how RAW_PFN relates to
> NOMAP, and ensure that RAW_PFN and NOMAP regions don't intersect if
> that is an unsupported combination.

yes, setting both MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP could meet some problems
when gaps of memblock.memory intersect memblock.reserved. if they do not intersect,
that's ok. so as you said this should be carefully considered.

as you think this patch is not needed because, i have showed my idea, it's enough, thanks!

>
> Regards,
> Ard.
>
>
>>>
>>>>  };
>>>>
>>>>  struct memblock_region {
>>>> @@ -92,6 +93,8 @@ bool memblock_overlaps_region(struct memblock_type
>>>> *type,
>>>>  int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
>>>>  int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
>>>>  int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
>>>> +int memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t size);
>>>> +int memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t size);
>>>>  ulong choose_memblock_flags(void);
>>>>
>>>>  /* Low level functions */
>>>> diff --git a/mm/memblock.c b/mm/memblock.c
>>>> index 7608bc3..c103b94 100644
>>>> --- a/mm/memblock.c
>>>> +++ b/mm/memblock.c
>>>> @@ -814,6 +814,30 @@ int __init_memblock memblock_mark_nomap(phys_addr_t
>>>> base, phys_addr_t size)
>>>>  }
>>>>
>>>>  /**
>>>> + * memblock_mark_raw_pfn - Mark raw pfn memory with flag
>>>> MEMBLOCK_RAW_PFN.
>>>> + * @base: the base phys addr of the region
>>>> + * @size: the size of the region
>>>> + *
>>>> + * Return 0 on succees, -errno on failure.
>>>> + */
>>>> +int __init_memblock memblock_mark_raw_pfn(phys_addr_t base, phys_addr_t
>>>> size)
>>>> +{
>>>> +       return memblock_setclr_flag(base, size, 1, MEMBLOCK_RAW_PFN);
>>>> +}
>>>> +
>>>> +/**
>>>> + * memblock_clear_raw_pfn - Clear flag MEMBLOCK_RAW_PFN for a specified
>>>> region.
>>>> + * @base: the base phys addr of the region
>>>> + * @size: the size of the region
>>>> + *
>>>> + * Return 0 on succees, -errno on failure.
>>>> + */
>>>> +int __init_memblock memblock_clear_raw_pfn(phys_addr_t base, phys_addr_t
>>>> size)
>>>> +{
>>>> +       return memblock_setclr_flag(base, size, 0, MEMBLOCK_RAW_PFN);
>>>> +}
>>>> +
>>>> +/**
>>>>   * __next_reserved_mem_region - next function for
>>>> for_each_reserved_region()
>>>>   * @idx: pointer to u64 loop variable
>>>>   * @out_start: ptr to phys_addr_t for start address of the region, can
>>>> be %NULL
>>>> --
>>>> 1.7.9.5
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>>
>>> .
>>>
>>
>
> .
>

[PATCH] mm: free reserved area's memmap if possiable

[Jisheng Zhang]

Add Chen, Catalin

On Thu, 16 Feb 2017 09:11:29 +0800 zhouxianrong wrote:
> 
> 
> On 2017/2/15 15:10, Ard Biesheuvel wrote:
> > On 15 February 2017 at 01:44, zhouxianrong wrote:  
> >>
> >>
> >> On 2017/2/14 17:03, Ard Biesheuvel wrote:  
> >>>
> >>> On 14 February 2017 at 06:53,  <zhouxianrong@huawei.com> wrote:  
> >>>>
> >>>> From: zhouxianrong <zhouxianrong@huawei.com>
> >>>>
> >>>> just like freeing no-map area's memmap (gaps of memblock.memory)
> >>>> we could free reserved area's memmap (areas of memblock.reserved)
> >>>> as well only when user of reserved area indicate that we can do
> >>>> this in drivers. that is, user of reserved area know how to
> >>>> use the reserved area who could not memblock_free or free_reserved_xxx
> >>>> the reserved area and regard the area as raw pfn usage by kernel.
> >>>> the patch supply a way to users who want to utilize the memmap
> >>>> memory corresponding to raw pfn reserved areas as many as possible.
> >>>> users can do this by memblock_mark_raw_pfn interface which mark the
> >>>> reserved area as raw pfn and tell free_unused_memmap that this area's
> >>>> memmap could be freeed.
> >>>>  
> >>>
> >>> Could you give an example how much memory we actually recover by doing
> >>> this? I understand it depends on the size of the reserved regions, but
> >>> I'm sure you have an actual example that inspired you to write this
> >>> patch.  
> >>
> >>
> >> i did statistics in our platform, the memmap of reserved region that can be
> >> freed
> >> is about 6MB. it's fewer.

<...>

> >>> In any case, it is good to emphasize that on 4 KB pagesize kernels, we
> >>> will only free multiples of 8 MB that are 8 MB aligned, resulting in
> >>> 128 KB of memmap backing to be released.  
> >>
> >>
> >>  
> >>>
> >>>  
> >>>> +               if (start < end)
> >>>> +                       free_memmap(start, end);
> >>>> +       }
> >>>>  }
> >>>>  #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
> >>>>
> >>>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> >>>> index 5b759c9..9f8d277 100644
> >>>> --- a/include/linux/memblock.h
> >>>> +++ b/include/linux/memblock.h
> >>>> @@ -26,6 +26,7 @@ enum {
> >>>>         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
> >>>>         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
> >>>>         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct
> >>>> mapping */
> >>>> +       MEMBLOCK_RAW_PFN        = 0x8,  /* region whose memmap never be
> >>>> used */  
> >>>
> >>>
> >>> I think we should be *very* careful about the combinatorial explosion
> >>> that results when combining all these flags, given that this is not a
> >>> proper enum but a bit field.
> >>>
> >>> In any case, the generic memblock change should be in a separate patch
> >>> from the arm64 change.  
> >>
> >>
> >> MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP can not be set at the same time
> >>  
> >
> > They should not. But if I call  memblock_mark_raw_pfn() on a
> > MEMBLOCK_NOMAP region, it will have both flags set.
> >
> > In summary, I don't think we need this patch. And if you can convince
> > us otherwise, you should really be more methodical and explicit in
> > implementing this RAW_PFN flag, not add it as a byproduct of the arch
> > code that uses it. Also, you should explain how RAW_PFN relates to
> > NOMAP, and ensure that RAW_PFN and NOMAP regions don't intersect if
> > that is an unsupported combination.  
> 
> yes, setting both MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP could meet some problems
> when gaps of memblock.memory intersect memblock.reserved. if they do not intersect,
> that's ok. so as you said this should be carefully considered.
> 
> as you think this patch is not needed because, i have showed my idea, it's enough, thanks!

we are also interested in this area.

Just curious, is this patch to "free the vmemmap holes" mentioned by
by Catalin in [1]?

[1]http://lkml.iu.edu/hypermail/linux/kernel/1604.1/03036.html

[PATCH] mm: free reserved area's memmap if possiable

[zhouxianrong]

On 2017/3/1 18:41, Jisheng Zhang wrote:
> Add Chen, Catalin
>
> On Thu, 16 Feb 2017 09:11:29 +0800 zhouxianrong wrote:
>>
>>
>> On 2017/2/15 15:10, Ard Biesheuvel wrote:
>>> On 15 February 2017 at 01:44, zhouxianrong wrote:
>>>>
>>>>
>>>> On 2017/2/14 17:03, Ard Biesheuvel wrote:
>>>>>
>>>>> On 14 February 2017 at 06:53,  <zhouxianrong@huawei.com> wrote:
>>>>>>
>>>>>> From: zhouxianrong <zhouxianrong@huawei.com>
>>>>>>
>>>>>> just like freeing no-map area's memmap (gaps of memblock.memory)
>>>>>> we could free reserved area's memmap (areas of memblock.reserved)
>>>>>> as well only when user of reserved area indicate that we can do
>>>>>> this in drivers. that is, user of reserved area know how to
>>>>>> use the reserved area who could not memblock_free or free_reserved_xxx
>>>>>> the reserved area and regard the area as raw pfn usage by kernel.
>>>>>> the patch supply a way to users who want to utilize the memmap
>>>>>> memory corresponding to raw pfn reserved areas as many as possible.
>>>>>> users can do this by memblock_mark_raw_pfn interface which mark the
>>>>>> reserved area as raw pfn and tell free_unused_memmap that this area's
>>>>>> memmap could be freeed.
>>>>>>
>>>>>
>>>>> Could you give an example how much memory we actually recover by doing
>>>>> this? I understand it depends on the size of the reserved regions, but
>>>>> I'm sure you have an actual example that inspired you to write this
>>>>> patch.
>>>>
>>>>
>>>> i did statistics in our platform, the memmap of reserved region that can be
>>>> freed
>>>> is about 6MB. it's fewer.
>
> <...>
>
>>>>> In any case, it is good to emphasize that on 4 KB pagesize kernels, we
>>>>> will only free multiples of 8 MB that are 8 MB aligned, resulting in
>>>>> 128 KB of memmap backing to be released.
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>> +               if (start < end)
>>>>>> +                       free_memmap(start, end);
>>>>>> +       }
>>>>>>  }
>>>>>>  #endif /* !CONFIG_SPARSEMEM_VMEMMAP */
>>>>>>
>>>>>> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
>>>>>> index 5b759c9..9f8d277 100644
>>>>>> --- a/include/linux/memblock.h
>>>>>> +++ b/include/linux/memblock.h
>>>>>> @@ -26,6 +26,7 @@ enum {
>>>>>>         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
>>>>>>         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
>>>>>>         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct
>>>>>> mapping */
>>>>>> +       MEMBLOCK_RAW_PFN        = 0x8,  /* region whose memmap never be
>>>>>> used */
>>>>>
>>>>>
>>>>> I think we should be *very* careful about the combinatorial explosion
>>>>> that results when combining all these flags, given that this is not a
>>>>> proper enum but a bit field.
>>>>>
>>>>> In any case, the generic memblock change should be in a separate patch
>>>>> from the arm64 change.
>>>>
>>>>
>>>> MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP can not be set at the same time
>>>>
>>>
>>> They should not. But if I call  memblock_mark_raw_pfn() on a
>>> MEMBLOCK_NOMAP region, it will have both flags set.
>>>
>>> In summary, I don't think we need this patch. And if you can convince
>>> us otherwise, you should really be more methodical and explicit in
>>> implementing this RAW_PFN flag, not add it as a byproduct of the arch
>>> code that uses it. Also, you should explain how RAW_PFN relates to
>>> NOMAP, and ensure that RAW_PFN and NOMAP regions don't intersect if
>>> that is an unsupported combination.
>>
>> yes, setting both MEMBLOCK_RAW_PFN and MEMBLOCK_NOMAP could meet some problems
>> when gaps of memblock.memory intersect memblock.reserved. if they do not intersect,
>> that's ok. so as you said this should be carefully considered.
>>
>> as you think this patch is not needed because, i have showed my idea, it's enough, thanks!
>
> we are also interested in this area.
>
> Just curious, is this patch to "free the vmemmap holes" mentioned by
> by Catalin in [1]?

free the vmemmap of reserved memblock (other than no-map regions) whose driver owner know
it is never be used.

>
> [1]http://lkml.iu.edu/hypermail/linux/kernel/1604.1/03036.html
>
> .
>