[PATCH 0/2] Introduce common code for risc-v sparsemem support

[PATCH 0/2] Introduce common code for risc-v sparsemem support

[Logan Gunthorpe]

These are the first two common patches in my series to introduce
sparsemem support to RISC-V. The full series was posted last cycle
here [1] and the latest version can be found here [2].

As recommended by Palmer, I'd like to get the changes to common code
merged and then I will pursue the cleanups in the individual arches (arm,
arm64, and sh) as well as adding the new feature to riscv.

I would suggest we merge these two patches through Andrew's mm tree.

Thanks,

Logan

[1] https://lore.kernel.org/lkml/20181015175702.9036-1-logang at deltatee.com/T/#u
[2] https://github.com/sbates130272/linux-p2pmem.git riscv-sparsemem-v3

Logan Gunthorpe (2):
  mm: Introduce common STRUCT_PAGE_MAX_SHIFT define
  mm/sparse: add common helper to mark all memblocks present

 arch/arm64/include/asm/memory.h |  9 ---------
 arch/arm64/mm/init.c            |  8 --------
 include/asm-generic/fixmap.h    |  1 +
 include/linux/mm_types.h        |  5 +++++
 include/linux/mmzone.h          |  6 ++++++
 mm/sparse.c                     | 11 +++++++++++
 6 files changed, 23 insertions(+), 17 deletions(-)

--
2.19.0

[PATCH 0/2] Introduce common code for risc-v sparsemem support

[Logan Gunthorpe]

These are the first two common patches in my series to introduce
sparsemem support to RISC-V. The full series was posted last cycle
here [1] and the latest version can be found here [2].

As recommended by Palmer, I'd like to get the changes to common code
merged and then I will pursue the cleanups in the individual arches (arm,
arm64, and sh) as well as adding the new feature to riscv.

I would suggest we merge these two patches through Andrew's mm tree.

Thanks,

Logan

[1] https://lore.kernel.org/lkml/20181015175702.9036-1-logang@deltatee.com/T/#u
[2] https://github.com/sbates130272/linux-p2pmem.git riscv-sparsemem-v3

Logan Gunthorpe (2):
  mm: Introduce common STRUCT_PAGE_MAX_SHIFT define
  mm/sparse: add common helper to mark all memblocks present

 arch/arm64/include/asm/memory.h |  9 ---------
 arch/arm64/mm/init.c            |  8 --------
 include/asm-generic/fixmap.h    |  1 +
 include/linux/mm_types.h        |  5 +++++
 include/linux/mmzone.h          |  6 ++++++
 mm/sparse.c                     | 11 +++++++++++
 6 files changed, 23 insertions(+), 17 deletions(-)

--
2.19.0

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/2] mm: Introduce common STRUCT_PAGE_MAX_SHIFT define

[Logan Gunthorpe]

This define is used by arm64 to calculate the size of the vmemmap
region. It is defined as the log2 of the upper bound on the size
of a struct page.

We move it into mm_types.h so it can be defined properly instead of
set and checked with a build bug. This also allows us to use the same
define for riscv.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/memory.h | 9 ---------
 arch/arm64/mm/init.c            | 8 --------
 include/asm-generic/fixmap.h    | 1 +
 include/linux/mm_types.h        | 5 +++++
 4 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..f0a5c9531e8b 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -34,15 +34,6 @@
  */
 #define PCI_IO_SIZE		SZ_16M
 
-/*
- * Log2 of the upper bound of the size of a struct page. Used for sizing
- * the vmemmap region only, does not affect actual memory footprint.
- * We don't use sizeof(struct page) directly since taking its size here
- * requires its definition to be available at this point in the inclusion
- * chain, and it may not be a power of 2 in the first place.
- */
-#define STRUCT_PAGE_MAX_SHIFT	6
-
 /*
  * VMEMMAP_SIZE - allows the whole linear region to be covered by
  *                a struct page array
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9d9582cac6c4..1a3e411a1d08 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -612,14 +612,6 @@ void __init mem_init(void)
 	BUILD_BUG_ON(TASK_SIZE_32			> TASK_SIZE_64);
 #endif
 
-#ifdef CONFIG_SPARSEMEM_VMEMMAP
-	/*
-	 * Make sure we chose the upper bound of sizeof(struct page)
-	 * correctly when sizing the VMEMMAP array.
-	 */
-	BUILD_BUG_ON(sizeof(struct page) > (1 << STRUCT_PAGE_MAX_SHIFT));
-#endif
-
 	if (PAGE_SIZE >= 16384 && get_num_physpages() <= 128) {
 		extern int sysctl_overcommit_memory;
 		/*
diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
index 827e4d3bbc7a..8cc7b09c1bc7 100644
--- a/include/asm-generic/fixmap.h
+++ b/include/asm-generic/fixmap.h
@@ -16,6 +16,7 @@
 #define __ASM_GENERIC_FIXMAP_H
 
 #include <linux/bug.h>
+#include <linux/mm_types.h>
 
 #define __fix_to_virt(x)	(FIXADDR_TOP - ((x) << PAGE_SHIFT))
 #define __virt_to_fix(x)	((FIXADDR_TOP - ((x)&PAGE_MASK)) >> PAGE_SHIFT)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ed8f6292a53..2c471a2c43fa 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -206,6 +206,11 @@ struct page {
 #endif
 } _struct_page_alignment;
 
+/*
+ * Used for sizing the vmemmap region on some architectures
+ */
+#define STRUCT_PAGE_MAX_SHIFT	(order_base_2(sizeof(struct page)))
+
 #define PAGE_FRAG_CACHE_MAX_SIZE	__ALIGN_MASK(32768, ~PAGE_MASK)
 #define PAGE_FRAG_CACHE_MAX_ORDER	get_order(PAGE_FRAG_CACHE_MAX_SIZE)
 
-- 
2.19.0

[PATCH 1/2] mm: Introduce common STRUCT_PAGE_MAX_SHIFT define

[Logan Gunthorpe]

This define is used by arm64 to calculate the size of the vmemmap
region. It is defined as the log2 of the upper bound on the size
of a struct page.

We move it into mm_types.h so it can be defined properly instead of
set and checked with a build bug. This also allows us to use the same
define for riscv.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/memory.h | 9 ---------
 arch/arm64/mm/init.c            | 8 --------
 include/asm-generic/fixmap.h    | 1 +
 include/linux/mm_types.h        | 5 +++++
 4 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..f0a5c9531e8b 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -34,15 +34,6 @@
  */
 #define PCI_IO_SIZE		SZ_16M
 
-/*
- * Log2 of the upper bound of the size of a struct page. Used for sizing
- * the vmemmap region only, does not affect actual memory footprint.
- * We don't use sizeof(struct page) directly since taking its size here
- * requires its definition to be available at this point in the inclusion
- * chain, and it may not be a power of 2 in the first place.
- */
-#define STRUCT_PAGE_MAX_SHIFT	6
-
 /*
  * VMEMMAP_SIZE - allows the whole linear region to be covered by
  *                a struct page array
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9d9582cac6c4..1a3e411a1d08 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -612,14 +612,6 @@ void __init mem_init(void)
 	BUILD_BUG_ON(TASK_SIZE_32			> TASK_SIZE_64);
 #endif
 
-#ifdef CONFIG_SPARSEMEM_VMEMMAP
-	/*
-	 * Make sure we chose the upper bound of sizeof(struct page)
-	 * correctly when sizing the VMEMMAP array.
-	 */
-	BUILD_BUG_ON(sizeof(struct page) > (1 << STRUCT_PAGE_MAX_SHIFT));
-#endif
-
 	if (PAGE_SIZE >= 16384 && get_num_physpages() <= 128) {
 		extern int sysctl_overcommit_memory;
 		/*
diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
index 827e4d3bbc7a..8cc7b09c1bc7 100644
--- a/include/asm-generic/fixmap.h
+++ b/include/asm-generic/fixmap.h
@@ -16,6 +16,7 @@
 #define __ASM_GENERIC_FIXMAP_H
 
 #include <linux/bug.h>
+#include <linux/mm_types.h>
 
 #define __fix_to_virt(x)	(FIXADDR_TOP - ((x) << PAGE_SHIFT))
 #define __virt_to_fix(x)	((FIXADDR_TOP - ((x)&PAGE_MASK)) >> PAGE_SHIFT)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ed8f6292a53..2c471a2c43fa 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -206,6 +206,11 @@ struct page {
 #endif
 } _struct_page_alignment;
 
+/*
+ * Used for sizing the vmemmap region on some architectures
+ */
+#define STRUCT_PAGE_MAX_SHIFT	(order_base_2(sizeof(struct page)))
+
 #define PAGE_FRAG_CACHE_MAX_SIZE	__ALIGN_MASK(32768, ~PAGE_MASK)
 #define PAGE_FRAG_CACHE_MAX_ORDER	get_order(PAGE_FRAG_CACHE_MAX_SIZE)
 
-- 
2.19.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Logan Gunthorpe]

Presently the arches arm64, arm and sh have a function which loops through
each memblock and calls memory present. riscv will require a similar
function.

Introduce a common memblocks_present() function that can be used by
all the arches. Subsequent patches will cleanup the arches that
make use of this.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Oscar Salvador <osalvador@suse.de>
---
 include/linux/mmzone.h |  6 ++++++
 mm/sparse.c            | 11 +++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 847705a6d0ec..db023a92f3a4 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -783,6 +783,12 @@ void memory_present(int nid, unsigned long start, unsigned long end);
 static inline void memory_present(int nid, unsigned long start, unsigned long end) {}
 #endif
 
+#if defined(CONFIG_SPARSEMEM)
+void memblocks_present(void);
+#else
+static inline void memblocks_present(void) {}
+#endif
+
 #ifdef CONFIG_HAVE_MEMORYLESS_NODES
 int local_memory_node(int node_id);
 #else
diff --git a/mm/sparse.c b/mm/sparse.c
index 33307fc05c4d..5841e4ad6d93 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -239,6 +239,17 @@ void __init memory_present(int nid, unsigned long start, unsigned long end)
 	}
 }
 
+void __init memblocks_present(void)
+{
+	struct memblock_region *reg;
+
+	for_each_memblock(memory, reg) {
+		memory_present(memblock_get_region_node(reg),
+			       memblock_region_memory_base_pfn(reg),
+			       memblock_region_memory_end_pfn(reg));
+	}
+}
+
 /*
  * Subtle, we encode the real pfn into the mem_map such that
  * the identity pfn - section_mem_map will return the actual
-- 
2.19.0

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Logan Gunthorpe]

Presently the arches arm64, arm and sh have a function which loops through
each memblock and calls memory present. riscv will require a similar
function.

Introduce a common memblocks_present() function that can be used by
all the arches. Subsequent patches will cleanup the arches that
make use of this.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Oscar Salvador <osalvador@suse.de>
---
 include/linux/mmzone.h |  6 ++++++
 mm/sparse.c            | 11 +++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 847705a6d0ec..db023a92f3a4 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -783,6 +783,12 @@ void memory_present(int nid, unsigned long start, unsigned long end);
 static inline void memory_present(int nid, unsigned long start, unsigned long end) {}
 #endif
 
+#if defined(CONFIG_SPARSEMEM)
+void memblocks_present(void);
+#else
+static inline void memblocks_present(void) {}
+#endif
+
 #ifdef CONFIG_HAVE_MEMORYLESS_NODES
 int local_memory_node(int node_id);
 #else
diff --git a/mm/sparse.c b/mm/sparse.c
index 33307fc05c4d..5841e4ad6d93 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -239,6 +239,17 @@ void __init memory_present(int nid, unsigned long start, unsigned long end)
 	}
 }
 
+void __init memblocks_present(void)
+{
+	struct memblock_region *reg;
+
+	for_each_memblock(memory, reg) {
+		memory_present(memblock_get_region_node(reg),
+			       memblock_region_memory_base_pfn(reg),
+			       memblock_region_memory_end_pfn(reg));
+	}
+}
+
 /*
  * Subtle, we encode the real pfn into the mem_map such that
  * the identity pfn - section_mem_map will return the actual
-- 
2.19.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/2] mm: Introduce common STRUCT_PAGE_MAX_SHIFT define

[Andrew Morton]

On Wed,  7 Nov 2018 10:38:58 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:

> This define is used by arm64 to calculate the size of the vmemmap
> region. It is defined as the log2 of the upper bound on the size
> of a struct page.
> 
> We move it into mm_types.h so it can be defined properly instead of
> set and checked with a build bug. This also allows us to use the same
> define for riscv.
> 
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -34,15 +34,6 @@
>   */
>  #define PCI_IO_SIZE		SZ_16M
>  
> -/*
> - * Log2 of the upper bound of the size of a struct page. Used for sizing
> - * the vmemmap region only, does not affect actual memory footprint.
> - * We don't use sizeof(struct page) directly since taking its size here
> - * requires its definition to be available at this point in the inclusion
> - * chain, and it may not be a power of 2 in the first place.
> - */
> -#define STRUCT_PAGE_MAX_SHIFT	6

Well that was lame.

> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -206,6 +206,11 @@ struct page {
>  #endif
>  } _struct_page_alignment;
>  
> +/*
> + * Used for sizing the vmemmap region on some architectures
> + */
> +#define STRUCT_PAGE_MAX_SHIFT	(order_base_2(sizeof(struct page)))

Much better.

Acked-by: Andrew Morton <akpm@linux-foundation.org>

Re: [PATCH 1/2] mm: Introduce common STRUCT_PAGE_MAX_SHIFT define

[Andrew Morton]

On Wed,  7 Nov 2018 10:38:58 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:

> This define is used by arm64 to calculate the size of the vmemmap
> region. It is defined as the log2 of the upper bound on the size
> of a struct page.
> 
> We move it into mm_types.h so it can be defined properly instead of
> set and checked with a build bug. This also allows us to use the same
> define for riscv.
> 
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -34,15 +34,6 @@
>   */
>  #define PCI_IO_SIZE		SZ_16M
>  
> -/*
> - * Log2 of the upper bound of the size of a struct page. Used for sizing
> - * the vmemmap region only, does not affect actual memory footprint.
> - * We don't use sizeof(struct page) directly since taking its size here
> - * requires its definition to be available at this point in the inclusion
> - * chain, and it may not be a power of 2 in the first place.
> - */
> -#define STRUCT_PAGE_MAX_SHIFT	6

Well that was lame.

> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -206,6 +206,11 @@ struct page {
>  #endif
>  } _struct_page_alignment;
>  
> +/*
> + * Used for sizing the vmemmap region on some architectures
> + */
> +#define STRUCT_PAGE_MAX_SHIFT	(order_base_2(sizeof(struct page)))

Much better.

Acked-by: Andrew Morton <akpm@linux-foundation.org>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Andrew Morton]

On Wed,  7 Nov 2018 10:38:59 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:

> Presently the arches arm64, arm and sh have a function which loops through
> each memblock and calls memory present. riscv will require a similar
> function.
> 
> Introduce a common memblocks_present() function that can be used by
> all the arches. Subsequent patches will cleanup the arches that
> make use of this.
> 
> ...
>
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -239,6 +239,17 @@ void __init memory_present(int nid, unsigned long start, unsigned long end)
>  	}
>  }
>  
> +void __init memblocks_present(void)
> +{
> +	struct memblock_region *reg;
> +
> +	for_each_memblock(memory, reg) {
> +		memory_present(memblock_get_region_node(reg),
> +			       memblock_region_memory_base_pfn(reg),
> +			       memblock_region_memory_end_pfn(reg));
> +	}
> +}
> +

I don't like the name much.  To me, memblocks_present means "are
memblocks present" whereas this actually means "memblocks are present".
But whatever.  A little covering comment which describes what this
does and why it does it would be nice.

Acked-by: Andrew Morton <akpm@linux-foundation.org>

I can grab both patches and shall sneak them into 4.20-rcX, but feel
free to merge them into some git tree if you'd prefer.  If I see them
turn up in linux-next I shall drop my copy.

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Andrew Morton]

On Wed,  7 Nov 2018 10:38:59 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:

> Presently the arches arm64, arm and sh have a function which loops through
> each memblock and calls memory present. riscv will require a similar
> function.
> 
> Introduce a common memblocks_present() function that can be used by
> all the arches. Subsequent patches will cleanup the arches that
> make use of this.
> 
> ...
>
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -239,6 +239,17 @@ void __init memory_present(int nid, unsigned long start, unsigned long end)
>  	}
>  }
>  
> +void __init memblocks_present(void)
> +{
> +	struct memblock_region *reg;
> +
> +	for_each_memblock(memory, reg) {
> +		memory_present(memblock_get_region_node(reg),
> +			       memblock_region_memory_base_pfn(reg),
> +			       memblock_region_memory_end_pfn(reg));
> +	}
> +}
> +

I don't like the name much.  To me, memblocks_present means "are
memblocks present" whereas this actually means "memblocks are present".
But whatever.  A little covering comment which describes what this
does and why it does it would be nice.

Acked-by: Andrew Morton <akpm@linux-foundation.org>

I can grab both patches and shall sneak them into 4.20-rcX, but feel
free to merge them into some git tree if you'd prefer.  If I see them
turn up in linux-next I shall drop my copy.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Logan Gunthorpe]

On 2018-11-07 1:12 p.m., Andrew Morton wrote:
>> +void __init memblocks_present(void)
>> +{
>> +	struct memblock_region *reg;
>> +
>> +	for_each_memblock(memory, reg) {
>> +		memory_present(memblock_get_region_node(reg),
>> +			       memblock_region_memory_base_pfn(reg),
>> +			       memblock_region_memory_end_pfn(reg));
>> +	}
>> +}
>> +
> 
> I don't like the name much.  To me, memblocks_present means "are
> memblocks present" whereas this actually means "memblocks are present".
> But whatever.  A little covering comment which describes what this
> does and why it does it would be nice.

The same argument can be made about the existing memory_present()
function and I think it's worth keeping the naming consistent. I'll add
a comment and resend shortly.

> Acked-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I can grab both patches and shall sneak them into 4.20-rcX, but feel
> free to merge them into some git tree if you'd prefer.  If I see them
> turn up in linux-next I shall drop my copy.

Sounds good, thanks.

Logan

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Logan Gunthorpe]

On 2018-11-07 1:12 p.m., Andrew Morton wrote:
>> +void __init memblocks_present(void)
>> +{
>> +	struct memblock_region *reg;
>> +
>> +	for_each_memblock(memory, reg) {
>> +		memory_present(memblock_get_region_node(reg),
>> +			       memblock_region_memory_base_pfn(reg),
>> +			       memblock_region_memory_end_pfn(reg));
>> +	}
>> +}
>> +
> 
> I don't like the name much.  To me, memblocks_present means "are
> memblocks present" whereas this actually means "memblocks are present".
> But whatever.  A little covering comment which describes what this
> does and why it does it would be nice.

The same argument can be made about the existing memory_present()
function and I think it's worth keeping the naming consistent. I'll add
a comment and resend shortly.

> Acked-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I can grab both patches and shall sneak them into 4.20-rcX, but feel
> free to merge them into some git tree if you'd prefer.  If I see them
> turn up in linux-next I shall drop my copy.

Sounds good, thanks.

Logan


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Thomas Gleixner]

Logan,

On Wed, 7 Nov 2018, Logan Gunthorpe wrote:
> On 2018-11-07 1:12 p.m., Andrew Morton wrote:
> >> +void __init memblocks_present(void)
> >> +{
> >> +	struct memblock_region *reg;
> >> +
> >> +	for_each_memblock(memory, reg) {
> >> +		memory_present(memblock_get_region_node(reg),
> >> +			       memblock_region_memory_base_pfn(reg),
> >> +			       memblock_region_memory_end_pfn(reg));
> >> +	}
> >> +}
> >> +
> > 
> > I don't like the name much.  To me, memblocks_present means "are
> > memblocks present" whereas this actually means "memblocks are present".
> > But whatever.  A little covering comment which describes what this
> > does and why it does it would be nice.
> 
> The same argument can be made about the existing memory_present()
> function and I think it's worth keeping the naming consistent. I'll add
> a comment and resend shortly.

Actually if both names suck, then there also is the option to rename both
instead of adding a comment to explain the suckage.

Thanks,

	tglx

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Thomas Gleixner]

Logan,

On Wed, 7 Nov 2018, Logan Gunthorpe wrote:
> On 2018-11-07 1:12 p.m., Andrew Morton wrote:
> >> +void __init memblocks_present(void)
> >> +{
> >> +	struct memblock_region *reg;
> >> +
> >> +	for_each_memblock(memory, reg) {
> >> +		memory_present(memblock_get_region_node(reg),
> >> +			       memblock_region_memory_base_pfn(reg),
> >> +			       memblock_region_memory_end_pfn(reg));
> >> +	}
> >> +}
> >> +
> > 
> > I don't like the name much.  To me, memblocks_present means "are
> > memblocks present" whereas this actually means "memblocks are present".
> > But whatever.  A little covering comment which describes what this
> > does and why it does it would be nice.
> 
> The same argument can be made about the existing memory_present()
> function and I think it's worth keeping the naming consistent. I'll add
> a comment and resend shortly.

Actually if both names suck, then there also is the option to rename both
instead of adding a comment to explain the suckage.

Thanks,

	tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Logan Gunthorpe]

On 2018-11-07 1:26 p.m., Thomas Gleixner wrote:
> Logan,
> 
> On Wed, 7 Nov 2018, Logan Gunthorpe wrote:
>> On 2018-11-07 1:12 p.m., Andrew Morton wrote:
>>>> +void __init memblocks_present(void)
>>>> +{
>>>> +	struct memblock_region *reg;
>>>> +
>>>> +	for_each_memblock(memory, reg) {
>>>> +		memory_present(memblock_get_region_node(reg),
>>>> +			       memblock_region_memory_base_pfn(reg),
>>>> +			       memblock_region_memory_end_pfn(reg));
>>>> +	}
>>>> +}
>>>> +
>>>
>>> I don't like the name much.  To me, memblocks_present means "are
>>> memblocks present" whereas this actually means "memblocks are present".
>>> But whatever.  A little covering comment which describes what this
>>> does and why it does it would be nice.
>>
>> The same argument can be made about the existing memory_present()
>> function and I think it's worth keeping the naming consistent. I'll add
>> a comment and resend shortly.
> 
> Actually if both names suck, then there also is the option to rename both
> instead of adding a comment to explain the suckage.

Ok, well, I wasn't expecting to take on a big rename like that as it
would create a patch touching a bunch of arches and mm files... But if
we can come to some agreement on a better name and someone is willing to
take that patch without significant delay then I'd be happy to create
the patch and add it to the start of my series.

Some ideas for new names:

mark_memory_present() / mark_memblocks_present()
set_memory_present() / set_memblocks_present()
memory_register() / memblocks_register()
register_memory() / register_memblocks()

Logan

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Logan Gunthorpe]

On 2018-11-07 1:26 p.m., Thomas Gleixner wrote:
> Logan,
> 
> On Wed, 7 Nov 2018, Logan Gunthorpe wrote:
>> On 2018-11-07 1:12 p.m., Andrew Morton wrote:
>>>> +void __init memblocks_present(void)
>>>> +{
>>>> +	struct memblock_region *reg;
>>>> +
>>>> +	for_each_memblock(memory, reg) {
>>>> +		memory_present(memblock_get_region_node(reg),
>>>> +			       memblock_region_memory_base_pfn(reg),
>>>> +			       memblock_region_memory_end_pfn(reg));
>>>> +	}
>>>> +}
>>>> +
>>>
>>> I don't like the name much.  To me, memblocks_present means "are
>>> memblocks present" whereas this actually means "memblocks are present".
>>> But whatever.  A little covering comment which describes what this
>>> does and why it does it would be nice.
>>
>> The same argument can be made about the existing memory_present()
>> function and I think it's worth keeping the naming consistent. I'll add
>> a comment and resend shortly.
> 
> Actually if both names suck, then there also is the option to rename both
> instead of adding a comment to explain the suckage.

Ok, well, I wasn't expecting to take on a big rename like that as it
would create a patch touching a bunch of arches and mm files... But if
we can come to some agreement on a better name and someone is willing to
take that patch without significant delay then I'd be happy to create
the patch and add it to the start of my series.

Some ideas for new names:

mark_memory_present() / mark_memblocks_present()
set_memory_present() / set_memblocks_present()
memory_register() / memblocks_register()
register_memory() / register_memblocks()

Logan

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Andrew Morton]

On Wed, 7 Nov 2018 13:36:34 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:

> > Actually if both names suck, then there also is the option to rename both
> > instead of adding a comment to explain the suckage.
> 
> Ok, well, I wasn't expecting to take on a big rename like that as it
> would create a patch touching a bunch of arches and mm files... But if
> we can come to some agreement on a better name and someone is willing to
> take that patch without significant delay then I'd be happy to create
> the patch and add it to the start of my series.

Some other time ;)  Let's just get the missing documentation added for now?

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Andrew Morton]

On Wed, 7 Nov 2018 13:36:34 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:

> > Actually if both names suck, then there also is the option to rename both
> > instead of adding a comment to explain the suckage.
> 
> Ok, well, I wasn't expecting to take on a big rename like that as it
> would create a patch touching a bunch of arches and mm files... But if
> we can come to some agreement on a better name and someone is willing to
> take that patch without significant delay then I'd be happy to create
> the patch and add it to the start of my series.

Some other time ;)  Let's just get the missing documentation added for now?


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Thomas Gleixner]

On Wed, 7 Nov 2018, Andrew Morton wrote:
> On Wed, 7 Nov 2018 13:36:34 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:
> 
> > > Actually if both names suck, then there also is the option to rename both
> > > instead of adding a comment to explain the suckage.
> > 
> > Ok, well, I wasn't expecting to take on a big rename like that as it
> > would create a patch touching a bunch of arches and mm files... But if
> > we can come to some agreement on a better name and someone is willing to
> > take that patch without significant delay then I'd be happy to create
> > the patch and add it to the start of my series.
> 
> Some other time ;)

More precise: Manjana. You live way too close to Mexico :)

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Thomas Gleixner]

On Wed, 7 Nov 2018, Andrew Morton wrote:
> On Wed, 7 Nov 2018 13:36:34 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:
> 
> > > Actually if both names suck, then there also is the option to rename both
> > > instead of adding a comment to explain the suckage.
> > 
> > Ok, well, I wasn't expecting to take on a big rename like that as it
> > would create a patch touching a bunch of arches and mm files... But if
> > we can come to some agreement on a better name and someone is willing to
> > take that patch without significant delay then I'd be happy to create
> > the patch and add it to the start of my series.
> 
> Some other time ;)

More precise: Manjana. You live way too close to Mexico :)


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Logan Gunthorpe]

Hey Andrew,

On 2018-11-07 1:12 p.m., Andrew Morton wrote:
> Acked-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I can grab both patches and shall sneak them into 4.20-rcX, but feel
> free to merge them into some git tree if you'd prefer.  If I see them
> turn up in linux-next I shall drop my copy.

Just wanted to check if you are still planning to get these patches into
4.20-rcX. It would really help us if you can do this seeing we then
won't have to delay a cycle and can target the riscv sparsemem code for
4.21.

Thanks,

Logan

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] mm/sparse: add common helper to mark all memblocks present

[Andrew Morton]

On Thu, 6 Dec 2018 10:40:31 -0700 Logan Gunthorpe <logang@deltatee.com> wrote:

> Hey Andrew,
> 
> On 2018-11-07 1:12 p.m., Andrew Morton wrote:
> > Acked-by: Andrew Morton <akpm@linux-foundation.org>
> > 
> > I can grab both patches and shall sneak them into 4.20-rcX, but feel
> > free to merge them into some git tree if you'd prefer.  If I see them
> > turn up in linux-next I shall drop my copy.
> 
> Just wanted to check if you are still planning to get these patches into
> 4.20-rcX. It would really help us if you can do this seeing we then
> won't have to delay a cycle and can target the riscv sparsemem code for
> 4.21.
> 

Ah, OK, I assumed that it would be merged via an arm tree.

I moved 

mm-introduce-common-struct_page_max_shift-define.patch
mm-sparse-add-common-helper-to-mark-all-memblocks-present.patch

to head-of-queue.  Shall send to Linus next week.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv