Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1525554 > unrolled thread

[HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:20 +0100
    Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Balbir Singh <bsingharora@gmail.com> - 2016-11-21 01:50 +0100
      Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Jerome Glisse <jglisse@redhat.com> - 2016-11-21 06:00 +0100
        Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-21 08:00 +0100
          Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Jerome Glisse <jglisse@redhat.com> - 2016-11-21 13:20 +0100
    Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-21 07:50 +0100
      Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Jerome Glisse <jglisse@redhat.com> - 2016-11-21 13:30 +0100
        Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-22 06:40 +0100
          Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool  to set of flags Jerome Glisse <jglisse@redhat.com> - 2016-11-22 15:10 +0100

#1525554 — [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

Only usefull for arch where we support ZONE_DEVICE and where we want to
also support un-addressable device memory. We need struct page for such
un-addressable memory. But we should avoid populating the kernel linear
mapping for the physical address range because there is no real memory
or anything behind those physical address.

Hence we need more flags than just knowing if it is device memory or not.

Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/ia64/mm/init.c            | 19 ++++++++++++++++---
 arch/powerpc/mm/mem.c          | 18 +++++++++++++++---
 arch/s390/mm/init.c            | 10 ++++++++--
 arch/sh/mm/init.c              | 18 +++++++++++++++---
 arch/tile/mm/init.c            | 10 ++++++++--
 arch/x86/mm/init_32.c          | 19 ++++++++++++++++---
 arch/x86/mm/init_64.c          | 19 ++++++++++++++++---
 include/linux/memory_hotplug.h | 17 +++++++++++++++--
 kernel/memremap.c              |  4 ++--
 mm/memory_hotplug.c            |  4 ++--
 10 files changed, 113 insertions(+), 25 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 1841ef6..95a2fa5 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -645,7 +645,7 @@ mem_init (void)
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size, int flags)
 {
 	pg_data_t *pgdat;
 	struct zone *zone;
@@ -653,10 +653,17 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int ret;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	pgdat = NODE_DATA(nid);
 
 	zone = pgdat->node_zones +
-		zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
+		zone_for_memory(nid, start, size, ZONE_NORMAL,
+				flags & MEMORY_DEVICE);
 	ret = __add_pages(nid, zone, start_pfn, nr_pages);
 
 	if (ret)
@@ -667,13 +674,19 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, int flags)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	struct zone *zone;
 	int ret;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	zone = page_zone(pfn_to_page(start_pfn));
 	ret = __remove_pages(zone, start_pfn, nr_pages);
 	if (ret)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 5f84433..e3c0532 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -126,7 +126,7 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
 	return -ENODEV;
 }
 
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size, int flags)
 {
 	struct pglist_data *pgdata;
 	struct zone *zone;
@@ -134,6 +134,12 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int rc;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	pgdata = NODE_DATA(nid);
 
 	start = (unsigned long)__va(start);
@@ -147,18 +153,24 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
 
 	/* this should work for most non-highmem platforms */
 	zone = pgdata->node_zones +
-		zone_for_memory(nid, start, size, 0, for_device);
+		zone_for_memory(nid, start, size, 0, flags & MEMORY_DEVICE);
 
 	return __add_pages(nid, zone, start_pfn, nr_pages);
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, int flags)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	struct zone *zone;
 	int ret;
+	
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
 
 	zone = page_zone(pfn_to_page(start_pfn));
 	ret = __remove_pages(zone, start_pfn, nr_pages);
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index f56a39b..4147b87 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -149,7 +149,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size, int flags)
 {
 	unsigned long normal_end_pfn = PFN_DOWN(memblock_end_of_DRAM());
 	unsigned long dma_end_pfn = PFN_DOWN(MAX_DMA_ADDRESS);
@@ -158,6 +158,12 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
 	unsigned long nr_pages;
 	int rc, zone_enum;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	rc = vmem_add_mapping(start, size);
 	if (rc)
 		return rc;
@@ -197,7 +203,7 @@ unsigned long memory_block_size_bytes(void)
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, int flags)
 {
 	/*
 	 * There is no hardware or firmware interface which could trigger a
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 7549186..f72a402 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -485,19 +485,25 @@ void free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size, int flags)
 {
 	pg_data_t *pgdat;
 	unsigned long start_pfn = PFN_DOWN(start);
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int ret;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	pgdat = NODE_DATA(nid);
 
 	/* We only have ZONE_NORMAL, so this is easy.. */
 	ret = __add_pages(nid, pgdat->node_zones +
 			zone_for_memory(nid, start, size, ZONE_NORMAL,
-			for_device),
+					flags & MEMORY_DEVICE),
 			start_pfn, nr_pages);
 	if (unlikely(ret))
 		printk("%s: Failed, __add_pages() == %d\n", __func__, ret);
@@ -516,13 +522,19 @@ EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
 #endif
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, int flags)
 {
 	unsigned long start_pfn = PFN_DOWN(start);
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	struct zone *zone;
 	int ret;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	zone = page_zone(pfn_to_page(start_pfn));
 	ret = __remove_pages(zone, start_pfn, nr_pages);
 	if (unlikely(ret))
diff --git a/arch/tile/mm/init.c b/arch/tile/mm/init.c
index adce254..5fd972c 100644
--- a/arch/tile/mm/init.c
+++ b/arch/tile/mm/init.c
@@ -863,13 +863,19 @@ void __init mem_init(void)
  * memory to the highmem for now.
  */
 #ifndef CONFIG_NEED_MULTIPLE_NODES
-int arch_add_memory(u64 start, u64 size, bool for_device)
+int arch_add_memory(u64 start, u64 size, int flags)
 {
 	struct pglist_data *pgdata = &contig_page_data;
 	struct zone *zone = pgdata->node_zones + MAX_NR_ZONES-1;
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	return __add_pages(zone, start_pfn, nr_pages);
 }
 
@@ -879,7 +885,7 @@ int remove_memory(u64 start, u64 size)
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, int flags)
 {
 	/* TODO */
 	return -EBUSY;
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index cf80590..16a9095 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -816,24 +816,37 @@ void __init mem_init(void)
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size, int flags)
 {
 	struct pglist_data *pgdata = NODE_DATA(nid);
 	struct zone *zone = pgdata->node_zones +
-		zone_for_memory(nid, start, size, ZONE_HIGHMEM, for_device);
+		zone_for_memory(nid, start, size, ZONE_HIGHMEM,
+				flags & MEMORY_DEVICE);
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	return __add_pages(nid, zone, start_pfn, nr_pages);
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, int flags)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	struct zone *zone;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	zone = page_zone(pfn_to_page(start_pfn));
 	return __remove_pages(zone, start_pfn, nr_pages);
 }
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 14b9dd7..8c4abb0 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -651,15 +651,22 @@ static void  update_end_of_memory_vars(u64 start, u64 size)
  * Memory is added always to NORMAL zone. This means you will never get
  * additional DMA/DMA32 memory.
  */
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size, int flags)
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
 	struct zone *zone = pgdat->node_zones +
-		zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
+		zone_for_memory(nid, start, size, ZONE_NORMAL,
+				flags & MEMORY_DEVICE);
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int ret;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	init_memory_mapping(start, start + size);
 
 	ret = __add_pages(nid, zone, start_pfn, nr_pages);
@@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
 	remove_pagetable(start, end, true);
 }
 
-int __ref arch_remove_memory(u64 start, u64 size)
+int __ref arch_remove_memory(u64 start, u64 size, int flags)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
@@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
 	struct zone *zone;
 	int ret;
 
+	/* Need to add support for device and unaddressable memory if needed */
+	if (flags & MEMORY_UNADDRESSABLE) {
+		BUG();
+		return -EINVAL;
+	}
+
 	/* With altmap the first mapped page is offset from @start */
 	altmap = to_vmem_altmap((unsigned long) page);
 	if (altmap)
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 01033fa..ba9b12e 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -103,7 +103,7 @@ extern bool memhp_auto_online;
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
 extern bool is_pageblock_removable_nolock(struct page *page);
-extern int arch_remove_memory(u64 start, u64 size);
+extern int arch_remove_memory(u64 start, u64 size, int flags);
 extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
 	unsigned long nr_pages);
 #endif /* CONFIG_MEMORY_HOTREMOVE */
@@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
 extern int add_memory_resource(int nid, struct resource *resource, bool online);
 extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
 		bool for_device);
-extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
+
+/*
+ * For device memory we want more informations than just knowing it is device
+ * memory. We want to know if we can migrate it (ie it is not storage memory
+ * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
+ * memory can not be access by CPU but we still want struct page so that we
+ * can use it like regular memory.
+ */
+#define MEMORY_FLAGS_NONE 0
+#define MEMORY_DEVICE (1 << 0)
+#define MEMORY_MOVABLE (1 << 1)
+#define MEMORY_UNADDRESSABLE (1 << 2)
+
+extern int arch_add_memory(int nid, u64 start, u64 size, int flags);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
 extern bool is_memblock_offlined(struct memory_block *mem);
 extern void remove_memory(int nid, u64 start, u64 size);
diff --git a/kernel/memremap.c b/kernel/memremap.c
index b501e39..07665eb 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -246,7 +246,7 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
 	/* pages are dead and unused, undo the arch mapping */
 	align_start = res->start & ~(SECTION_SIZE - 1);
 	align_size = ALIGN(resource_size(res), SECTION_SIZE);
-	arch_remove_memory(align_start, align_size);
+	arch_remove_memory(align_start, align_size, MEMORY_DEVICE);
 	untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
 	pgmap_radix_release(res);
 	dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
@@ -358,7 +358,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
 	if (error)
 		goto err_pfn_remap;
 
-	error = arch_add_memory(nid, align_start, align_size, true);
+	error = arch_add_memory(nid, align_start, align_size, MEMORY_DEVICE);
 	if (error)
 		goto err_add_memory;
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 9629273..b2942d7 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1386,7 +1386,7 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
 	}
 
 	/* call arch's memory hotadd */
-	ret = arch_add_memory(nid, start, size, false);
+	ret = arch_add_memory(nid, start, size, MEMORY_FLAGS_NONE);
 
 	if (ret < 0)
 		goto error;
@@ -2205,7 +2205,7 @@ void __ref remove_memory(int nid, u64 start, u64 size)
 	memblock_free(start, size);
 	memblock_remove(start, size);
 
-	arch_remove_memory(start, size);
+	arch_remove_memory(start, size, MEMORY_FLAGS_NONE);
 
 	try_offline_node(nid);
 
-- 
2.4.3

[toc] | [next] | [standalone]

#1526321 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On 19/11/16 05:18, Jérôme Glisse wrote:
> Only usefull for arch where we support ZONE_DEVICE and where we want to
> also support un-addressable device memory. We need struct page for such
> un-addressable memory. But we should avoid populating the kernel linear
> mapping for the physical address range because there is no real memory
> or anything behind those physical address.
> 
> Hence we need more flags than just knowing if it is device memory or not.
> 


Isn't it better to add a wrapper to arch_add/remove_memory and do those
checks inside and then call arch_add/remove_memory to reduce the churn.
If you need selectively enable MEMORY_UNADDRESSABLE that can be done with
_ARCH_HAS_FEATURE

> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Chris Metcalf <cmetcalf@mellanox.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> ---
>  arch/ia64/mm/init.c            | 19 ++++++++++++++++---
>  arch/powerpc/mm/mem.c          | 18 +++++++++++++++---
>  arch/s390/mm/init.c            | 10 ++++++++--
>  arch/sh/mm/init.c              | 18 +++++++++++++++---
>  arch/tile/mm/init.c            | 10 ++++++++--
>  arch/x86/mm/init_32.c          | 19 ++++++++++++++++---
>  arch/x86/mm/init_64.c          | 19 ++++++++++++++++---
>  include/linux/memory_hotplug.h | 17 +++++++++++++++--
>  kernel/memremap.c              |  4 ++--
>  mm/memory_hotplug.c            |  4 ++--
>  10 files changed, 113 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 1841ef6..95a2fa5 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -645,7 +645,7 @@ mem_init (void)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	pg_data_t *pgdat;
>  	struct zone *zone;
> @@ -653,10 +653,17 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	pgdat = NODE_DATA(nid);
>  
>  	zone = pgdat->node_zones +
> -		zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
> +		zone_for_memory(nid, start, size, ZONE_NORMAL,
> +				flags & MEMORY_DEVICE);
>  	ret = __add_pages(nid, zone, start_pfn, nr_pages);
>  
>  	if (ret)
> @@ -667,13 +674,19 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	ret = __remove_pages(zone, start_pfn, nr_pages);
>  	if (ret)
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 5f84433..e3c0532 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -126,7 +126,7 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
>  	return -ENODEV;
>  }
>  
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdata;
>  	struct zone *zone;
> @@ -134,6 +134,12 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int rc;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	pgdata = NODE_DATA(nid);
>  
>  	start = (unsigned long)__va(start);
> @@ -147,18 +153,24 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  
>  	/* this should work for most non-highmem platforms */
>  	zone = pgdata->node_zones +
> -		zone_for_memory(nid, start, size, 0, for_device);
> +		zone_for_memory(nid, start, size, 0, flags & MEMORY_DEVICE);
>  
>  	return __add_pages(nid, zone, start_pfn, nr_pages);
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  	int ret;
> +	
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
>  
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	ret = __remove_pages(zone, start_pfn, nr_pages);
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index f56a39b..4147b87 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -149,7 +149,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	unsigned long normal_end_pfn = PFN_DOWN(memblock_end_of_DRAM());
>  	unsigned long dma_end_pfn = PFN_DOWN(MAX_DMA_ADDRESS);
> @@ -158,6 +158,12 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  	unsigned long nr_pages;
>  	int rc, zone_enum;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	rc = vmem_add_mapping(start, size);
>  	if (rc)
>  		return rc;
> @@ -197,7 +203,7 @@ unsigned long memory_block_size_bytes(void)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	/*
>  	 * There is no hardware or firmware interface which could trigger a
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index 7549186..f72a402 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -485,19 +485,25 @@ void free_initrd_mem(unsigned long start, unsigned long end)
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	pg_data_t *pgdat;
>  	unsigned long start_pfn = PFN_DOWN(start);
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	pgdat = NODE_DATA(nid);
>  
>  	/* We only have ZONE_NORMAL, so this is easy.. */
>  	ret = __add_pages(nid, pgdat->node_zones +
>  			zone_for_memory(nid, start, size, ZONE_NORMAL,
> -			for_device),
> +					flags & MEMORY_DEVICE),
>  			start_pfn, nr_pages);
>  	if (unlikely(ret))
>  		printk("%s: Failed, __add_pages() == %d\n", __func__, ret);
> @@ -516,13 +522,19 @@ EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = PFN_DOWN(start);
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	ret = __remove_pages(zone, start_pfn, nr_pages);
>  	if (unlikely(ret))
> diff --git a/arch/tile/mm/init.c b/arch/tile/mm/init.c
> index adce254..5fd972c 100644
> --- a/arch/tile/mm/init.c
> +++ b/arch/tile/mm/init.c
> @@ -863,13 +863,19 @@ void __init mem_init(void)
>   * memory to the highmem for now.
>   */
>  #ifndef CONFIG_NEED_MULTIPLE_NODES
> -int arch_add_memory(u64 start, u64 size, bool for_device)
> +int arch_add_memory(u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdata = &contig_page_data;
>  	struct zone *zone = pgdata->node_zones + MAX_NR_ZONES-1;
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	return __add_pages(zone, start_pfn, nr_pages);
>  }
>  
> @@ -879,7 +885,7 @@ int remove_memory(u64 start, u64 size)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	/* TODO */
>  	return -EBUSY;
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index cf80590..16a9095 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -816,24 +816,37 @@ void __init mem_init(void)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdata = NODE_DATA(nid);
>  	struct zone *zone = pgdata->node_zones +
> -		zone_for_memory(nid, start, size, ZONE_HIGHMEM, for_device);
> +		zone_for_memory(nid, start, size, ZONE_HIGHMEM,
> +				flags & MEMORY_DEVICE);
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	return __add_pages(nid, zone, start_pfn, nr_pages);
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	return __remove_pages(zone, start_pfn, nr_pages);
>  }
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 14b9dd7..8c4abb0 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -651,15 +651,22 @@ static void  update_end_of_memory_vars(u64 start, u64 size)
>   * Memory is added always to NORMAL zone. This means you will never get
>   * additional DMA/DMA32 memory.
>   */
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdat = NODE_DATA(nid);
>  	struct zone *zone = pgdat->node_zones +
> -		zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
> +		zone_for_memory(nid, start, size, ZONE_NORMAL,
> +				flags & MEMORY_DEVICE);
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	init_memory_mapping(start, start + size);
>  
>  	ret = __add_pages(nid, zone, start_pfn, nr_pages);
> @@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
>  	remove_pagetable(start, end, true);
>  }
>  
> -int __ref arch_remove_memory(u64 start, u64 size)
> +int __ref arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
> @@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
>  	struct zone *zone;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	/* With altmap the first mapped page is offset from @start */
>  	altmap = to_vmem_altmap((unsigned long) page);
>  	if (altmap)
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 01033fa..ba9b12e 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -103,7 +103,7 @@ extern bool memhp_auto_online;
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
>  extern bool is_pageblock_removable_nolock(struct page *page);
> -extern int arch_remove_memory(u64 start, u64 size);
> +extern int arch_remove_memory(u64 start, u64 size, int flags);
>  extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
>  	unsigned long nr_pages);
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
> @@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
>  extern int add_memory_resource(int nid, struct resource *resource, bool online);
>  extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
>  		bool for_device);
> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
> +
> +/*
> + * For device memory we want more informations than just knowing it is device
				     information
> + * memory. We want to know if we can migrate it (ie it is not storage memory
> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
> + * memory can not be access by CPU but we still want struct page so that we
			accessed
> + * can use it like regular memory.

Can you please add some details on why -- migration needs them for example?

> + */
> +#define MEMORY_FLAGS_NONE 0
> +#define MEMORY_DEVICE (1 << 0)
> +#define MEMORY_MOVABLE (1 << 1)
> +#define MEMORY_UNADDRESSABLE (1 << 2)
> +
> +extern int arch_add_memory(int nid, u64 start, u64 size, int flags);
>  extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
>  extern bool is_memblock_offlined(struct memory_block *mem);
>  extern void remove_memory(int nid, u64 start, u64 size);
> diff --git a/kernel/memremap.c b/kernel/memremap.c
> index b501e39..07665eb 100644
> --- a/kernel/memremap.c
> +++ b/kernel/memremap.c
> @@ -246,7 +246,7 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
>  	/* pages are dead and unused, undo the arch mapping */
>  	align_start = res->start & ~(SECTION_SIZE - 1);
>  	align_size = ALIGN(resource_size(res), SECTION_SIZE);
> -	arch_remove_memory(align_start, align_size);
> +	arch_remove_memory(align_start, align_size, MEMORY_DEVICE);
>  	untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
>  	pgmap_radix_release(res);
>  	dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
> @@ -358,7 +358,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
>  	if (error)
>  		goto err_pfn_remap;
>  
> -	error = arch_add_memory(nid, align_start, align_size, true);
> +	error = arch_add_memory(nid, align_start, align_size, MEMORY_DEVICE);
>  	if (error)
>  		goto err_add_memory;
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 9629273..b2942d7 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1386,7 +1386,7 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
>  	}
>  
>  	/* call arch's memory hotadd */
> -	ret = arch_add_memory(nid, start, size, false);
> +	ret = arch_add_memory(nid, start, size, MEMORY_FLAGS_NONE);
>  
>  	if (ret < 0)
>  		goto error;
> @@ -2205,7 +2205,7 @@ void __ref remove_memory(int nid, u64 start, u64 size)
>  	memblock_free(start, size);
>  	memblock_remove(start, size);
>  
> -	arch_remove_memory(start, size);
> +	arch_remove_memory(start, size, MEMORY_FLAGS_NONE);
>  
>  	try_offline_node(nid);
>  
> 

Balbir Singh.

[toc] | [prev] | [next] | [standalone]

#1526360 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On Mon, Nov 21, 2016 at 11:44:36AM +1100, Balbir Singh wrote:
> 
> 
> On 19/11/16 05:18, Jérôme Glisse wrote:
> > Only usefull for arch where we support ZONE_DEVICE and where we want to
> > also support un-addressable device memory. We need struct page for such
> > un-addressable memory. But we should avoid populating the kernel linear
> > mapping for the physical address range because there is no real memory
> > or anything behind those physical address.
> > 
> > Hence we need more flags than just knowing if it is device memory or not.
> > 
> 
> 
> Isn't it better to add a wrapper to arch_add/remove_memory and do those
> checks inside and then call arch_add/remove_memory to reduce the churn.
> If you need selectively enable MEMORY_UNADDRESSABLE that can be done with
> _ARCH_HAS_FEATURE

The flag parameter can be use by other new features and thus i thought the
churn was fine. But i do not mind either way, whatever people like best.

[...]

> > -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
> > +
> > +/*
> > + * For device memory we want more informations than just knowing it is device
> 				     information
> > + * memory. We want to know if we can migrate it (ie it is not storage memory
> > + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
> > + * memory can not be access by CPU but we still want struct page so that we
> 			accessed
> > + * can use it like regular memory.
> 
> Can you please add some details on why -- migration needs them for example?

I am not sure what you mean ? DAX ie persistent memory device is intended to be
use for filesystem or persistent storage. Hence memory migration does not apply
to it (it would go against its purpose).

So i want to extend ZONE_DEVICE to be more then just DAX/persistent memory. For
that i need to differentatiate between device memory that can be migrated and
should be more or less treated like regular memory (with struct page). This is
what the MEMORY_MOVABLE flag is for.

Finaly in my case the device memory is not accessible by the CPU so i need yet
another flag. In the end i am extending ZONE_DEVICE to be use for 3 differents
type of memory.

Is this the kind of explanation you are looking for ?

> > + */
> > +#define MEMORY_FLAGS_NONE 0
> > +#define MEMORY_DEVICE (1 << 0)
> > +#define MEMORY_MOVABLE (1 << 1)
> > +#define MEMORY_UNADDRESSABLE (1 << 2)

Cheers,
Jérôme

[toc] | [prev] | [next] | [standalone]

#1526411 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On 11/21/2016 10:23 AM, Jerome Glisse wrote:
> On Mon, Nov 21, 2016 at 11:44:36AM +1100, Balbir Singh wrote:
>>
>>
>> On 19/11/16 05:18, Jérôme Glisse wrote:
>>> Only usefull for arch where we support ZONE_DEVICE and where we want to
>>> also support un-addressable device memory. We need struct page for such
>>> un-addressable memory. But we should avoid populating the kernel linear
>>> mapping for the physical address range because there is no real memory
>>> or anything behind those physical address.
>>>
>>> Hence we need more flags than just knowing if it is device memory or not.
>>>
>>
>>
>> Isn't it better to add a wrapper to arch_add/remove_memory and do those
>> checks inside and then call arch_add/remove_memory to reduce the churn.
>> If you need selectively enable MEMORY_UNADDRESSABLE that can be done with
>> _ARCH_HAS_FEATURE
> 
> The flag parameter can be use by other new features and thus i thought the
> churn was fine. But i do not mind either way, whatever people like best.

Right, once we get the device memory classification right, these flags
can be used in more places.

> 
> [...]
> 
>>> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
>>> +
>>> +/*
>>> + * For device memory we want more informations than just knowing it is device
>> 				     information
>>> + * memory. We want to know if we can migrate it (ie it is not storage memory
>>> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
>>> + * memory can not be access by CPU but we still want struct page so that we
>> 			accessed
>>> + * can use it like regular memory.
>>
>> Can you please add some details on why -- migration needs them for example?
> 
> I am not sure what you mean ? DAX ie persistent memory device is intended to be
> use for filesystem or persistent storage. Hence memory migration does not apply
> to it (it would go against its purpose).

Why ? It can still be used for compaction, HW errors etc where we need to
move between persistent storage areas. The source and destination can be
persistent storage memory.

> 
> So i want to extend ZONE_DEVICE to be more then just DAX/persistent memory. For
> that i need to differentatiate between device memory that can be migrated and
> should be more or less treated like regular memory (with struct page). This is
> what the MEMORY_MOVABLE flag is for.

ZONE_DEVICE right now also supports struct page for the addressable memory,
(whether inside it's own range or in system RAM) with this we are extending
it to cover un-addressable memory with struct pages. Yes the differentiation
is required.

> 
> Finaly in my case the device memory is not accessible by the CPU so i need yet
> another flag. In the end i am extending ZONE_DEVICE to be use for 3 differents
> type of memory.
> 
> Is this the kind of explanation you are looking for ?

[toc] | [prev] | [next] | [standalone]

#1526622 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On Mon, Nov 21, 2016 at 12:27:15PM +0530, Anshuman Khandual wrote:
> On 11/21/2016 10:23 AM, Jerome Glisse wrote:
> > On Mon, Nov 21, 2016 at 11:44:36AM +1100, Balbir Singh wrote:
> >>
> >>
> >> On 19/11/16 05:18, Jérôme Glisse wrote:
> >>> Only usefull for arch where we support ZONE_DEVICE and where we want to
> >>> also support un-addressable device memory. We need struct page for such
> >>> un-addressable memory. But we should avoid populating the kernel linear
> >>> mapping for the physical address range because there is no real memory
> >>> or anything behind those physical address.
> >>>
> >>> Hence we need more flags than just knowing if it is device memory or not.
> >>>
> >>
> >>
> >> Isn't it better to add a wrapper to arch_add/remove_memory and do those
> >> checks inside and then call arch_add/remove_memory to reduce the churn.
> >> If you need selectively enable MEMORY_UNADDRESSABLE that can be done with
> >> _ARCH_HAS_FEATURE
> > 
> > The flag parameter can be use by other new features and thus i thought the
> > churn was fine. But i do not mind either way, whatever people like best.
> 
> Right, once we get the device memory classification right, these flags
> can be used in more places.
> 
> > 
> > [...]
> > 
> >>> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
> >>> +
> >>> +/*
> >>> + * For device memory we want more informations than just knowing it is device
> >> 				     information
> >>> + * memory. We want to know if we can migrate it (ie it is not storage memory
> >>> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
> >>> + * memory can not be access by CPU but we still want struct page so that we
> >> 			accessed
> >>> + * can use it like regular memory.
> >>
> >> Can you please add some details on why -- migration needs them for example?
> > 
> > I am not sure what you mean ? DAX ie persistent memory device is intended to be
> > use for filesystem or persistent storage. Hence memory migration does not apply
> > to it (it would go against its purpose).
> 
> Why ? It can still be used for compaction, HW errors etc where we need to
> move between persistent storage areas. The source and destination can be
> persistent storage memory.

Well i don't think they intend to do migration for that, HW errors are hidden by
hardware itself so far. But yes if they care then they could use page migration.
So far it does not seems something any of the folks working on persistent memory
think useful.

Cheers,
Jérôme

[toc] | [prev] | [next] | [standalone]

#1526408 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On 11/18/2016 11:48 PM, Jérôme Glisse wrote:
> Only usefull for arch where we support ZONE_DEVICE and where we want to

A small nit s/usefull/useful/

> also support un-addressable device memory. We need struct page for such
> un-addressable memory. But we should avoid populating the kernel linear
> mapping for the physical address range because there is no real memory
> or anything behind those physical address.
> 
> Hence we need more flags than just knowing if it is device memory or not.
> 
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Chris Metcalf <cmetcalf@mellanox.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> ---
>  arch/ia64/mm/init.c            | 19 ++++++++++++++++---
>  arch/powerpc/mm/mem.c          | 18 +++++++++++++++---
>  arch/s390/mm/init.c            | 10 ++++++++--
>  arch/sh/mm/init.c              | 18 +++++++++++++++---
>  arch/tile/mm/init.c            | 10 ++++++++--
>  arch/x86/mm/init_32.c          | 19 ++++++++++++++++---
>  arch/x86/mm/init_64.c          | 19 ++++++++++++++++---
>  include/linux/memory_hotplug.h | 17 +++++++++++++++--
>  kernel/memremap.c              |  4 ++--
>  mm/memory_hotplug.c            |  4 ++--
>  10 files changed, 113 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 1841ef6..95a2fa5 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -645,7 +645,7 @@ mem_init (void)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	pg_data_t *pgdat;
>  	struct zone *zone;
> @@ -653,10 +653,17 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	pgdat = NODE_DATA(nid);
>  
>  	zone = pgdat->node_zones +
> -		zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
> +		zone_for_memory(nid, start, size, ZONE_NORMAL,
> +				flags & MEMORY_DEVICE);
>  	ret = __add_pages(nid, zone, start_pfn, nr_pages);
>  
>  	if (ret)
> @@ -667,13 +674,19 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	ret = __remove_pages(zone, start_pfn, nr_pages);
>  	if (ret)
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 5f84433..e3c0532 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -126,7 +126,7 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
>  	return -ENODEV;
>  }
>  
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdata;
>  	struct zone *zone;
> @@ -134,6 +134,12 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int rc;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	pgdata = NODE_DATA(nid);
>  
>  	start = (unsigned long)__va(start);
> @@ -147,18 +153,24 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  
>  	/* this should work for most non-highmem platforms */
>  	zone = pgdata->node_zones +
> -		zone_for_memory(nid, start, size, 0, for_device);
> +		zone_for_memory(nid, start, size, 0, flags & MEMORY_DEVICE);
>  
>  	return __add_pages(nid, zone, start_pfn, nr_pages);
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  	int ret;
> +	
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
>  
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	ret = __remove_pages(zone, start_pfn, nr_pages);
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index f56a39b..4147b87 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -149,7 +149,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	unsigned long normal_end_pfn = PFN_DOWN(memblock_end_of_DRAM());
>  	unsigned long dma_end_pfn = PFN_DOWN(MAX_DMA_ADDRESS);
> @@ -158,6 +158,12 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>  	unsigned long nr_pages;
>  	int rc, zone_enum;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	rc = vmem_add_mapping(start, size);
>  	if (rc)
>  		return rc;
> @@ -197,7 +203,7 @@ unsigned long memory_block_size_bytes(void)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	/*
>  	 * There is no hardware or firmware interface which could trigger a
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index 7549186..f72a402 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -485,19 +485,25 @@ void free_initrd_mem(unsigned long start, unsigned long end)
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	pg_data_t *pgdat;
>  	unsigned long start_pfn = PFN_DOWN(start);
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	pgdat = NODE_DATA(nid);
>  
>  	/* We only have ZONE_NORMAL, so this is easy.. */
>  	ret = __add_pages(nid, pgdat->node_zones +
>  			zone_for_memory(nid, start, size, ZONE_NORMAL,
> -			for_device),
> +					flags & MEMORY_DEVICE),
>  			start_pfn, nr_pages);
>  	if (unlikely(ret))
>  		printk("%s: Failed, __add_pages() == %d\n", __func__, ret);
> @@ -516,13 +522,19 @@ EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
>  #endif
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = PFN_DOWN(start);
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	ret = __remove_pages(zone, start_pfn, nr_pages);
>  	if (unlikely(ret))
> diff --git a/arch/tile/mm/init.c b/arch/tile/mm/init.c
> index adce254..5fd972c 100644
> --- a/arch/tile/mm/init.c
> +++ b/arch/tile/mm/init.c
> @@ -863,13 +863,19 @@ void __init mem_init(void)
>   * memory to the highmem for now.
>   */
>  #ifndef CONFIG_NEED_MULTIPLE_NODES
> -int arch_add_memory(u64 start, u64 size, bool for_device)
> +int arch_add_memory(u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdata = &contig_page_data;
>  	struct zone *zone = pgdata->node_zones + MAX_NR_ZONES-1;
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	return __add_pages(zone, start_pfn, nr_pages);
>  }
>  
> @@ -879,7 +885,7 @@ int remove_memory(u64 start, u64 size)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	/* TODO */
>  	return -EBUSY;
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index cf80590..16a9095 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -816,24 +816,37 @@ void __init mem_init(void)
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdata = NODE_DATA(nid);
>  	struct zone *zone = pgdata->node_zones +
> -		zone_for_memory(nid, start, size, ZONE_HIGHMEM, for_device);
> +		zone_for_memory(nid, start, size, ZONE_HIGHMEM,
> +				flags & MEMORY_DEVICE);
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	return __add_pages(nid, zone, start_pfn, nr_pages);
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
> -int arch_remove_memory(u64 start, u64 size)
> +int arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	struct zone *zone;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	zone = page_zone(pfn_to_page(start_pfn));
>  	return __remove_pages(zone, start_pfn, nr_pages);
>  }
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 14b9dd7..8c4abb0 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -651,15 +651,22 @@ static void  update_end_of_memory_vars(u64 start, u64 size)
>   * Memory is added always to NORMAL zone. This means you will never get
>   * additional DMA/DMA32 memory.
>   */
> -int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> +int arch_add_memory(int nid, u64 start, u64 size, int flags)
>  {
>  	struct pglist_data *pgdat = NODE_DATA(nid);
>  	struct zone *zone = pgdat->node_zones +
> -		zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
> +		zone_for_memory(nid, start, size, ZONE_NORMAL,
> +				flags & MEMORY_DEVICE);
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	init_memory_mapping(start, start + size);
>  
>  	ret = __add_pages(nid, zone, start_pfn, nr_pages);
> @@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
>  	remove_pagetable(start, end, true);
>  }
>  
> -int __ref arch_remove_memory(u64 start, u64 size)
> +int __ref arch_remove_memory(u64 start, u64 size, int flags)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
> @@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
>  	struct zone *zone;
>  	int ret;
>  
> +	/* Need to add support for device and unaddressable memory if needed */
> +	if (flags & MEMORY_UNADDRESSABLE) {
> +		BUG();
> +		return -EINVAL;
> +	}
> +
>  	/* With altmap the first mapped page is offset from @start */
>  	altmap = to_vmem_altmap((unsigned long) page);
>  	if (altmap)

So with this patch none of the architectures support un-addressable
memory but then support will be added through later patches ?
zone_for_memory function's flag now takes MEMORY_DEVICE parameter.
Then we need to change all the previous ZONE_DEVICE changes which
ever took "for_device" to accommodate this new flag ? just curious.

> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 01033fa..ba9b12e 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -103,7 +103,7 @@ extern bool memhp_auto_online;
>  
>  #ifdef CONFIG_MEMORY_HOTREMOVE
>  extern bool is_pageblock_removable_nolock(struct page *page);
> -extern int arch_remove_memory(u64 start, u64 size);
> +extern int arch_remove_memory(u64 start, u64 size, int flags);
>  extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
>  	unsigned long nr_pages);
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
> @@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
>  extern int add_memory_resource(int nid, struct resource *resource, bool online);
>  extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
>  		bool for_device);
> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
> +
> +/*
> + * For device memory we want more informations than just knowing it is device
> + * memory. We want to know if we can migrate it (ie it is not storage memory
> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
> + * memory can not be access by CPU but we still want struct page so that we
> + * can use it like regular memory.

Some typos here. Needs to be cleaned up as well. But please have a
look at comment below over the classification itself.

> + */
> +#define MEMORY_FLAGS_NONE 0
> +#define MEMORY_DEVICE (1 << 0)
> +#define MEMORY_MOVABLE (1 << 1)
> +#define MEMORY_UNADDRESSABLE (1 << 2)

It should be DEVICE_MEMORY_* instead of MEMORY_* as we are trying to
classify device memory (though they are represented with struct page)
not regular system ram memory. This should attempt to classify device
memory which is backed by struct pages. arch_add_memory/arch_remove
_memory does not come into play if it's traditional device memory
which is just PFN and does not have struct page associated with it.

Broadly they are either CPU accessible or in-accessible. Storage
memory like persistent memory represented though ZONE_DEVICE fall
under the accessible (coherent) category. IIUC right now they are
not movable because page->pgmap replaces page->lru in struct page
hence its inability to be on standard LRU lists as one of the
reasons. As there was a need to have struct page to exploit more
core VM features on these memory going forward it will have to be
migratable one way or the other to accommodate features like
compaction, HW poison etc in these storage memory. Hence my point
here is lets not classify any of these memories as non-movable.
Just addressable or not should be the only classification.

> +
> +extern int arch_add_memory(int nid, u64 start, u64 size, int flags);
>  extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
>  extern bool is_memblock_offlined(struct memory_block *mem);
>  extern void remove_memory(int nid, u64 start, u64 size);
> diff --git a/kernel/memremap.c b/kernel/memremap.c
> index b501e39..07665eb 100644
> --- a/kernel/memremap.c
> +++ b/kernel/memremap.c
> @@ -246,7 +246,7 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
>  	/* pages are dead and unused, undo the arch mapping */
>  	align_start = res->start & ~(SECTION_SIZE - 1);
>  	align_size = ALIGN(resource_size(res), SECTION_SIZE);
> -	arch_remove_memory(align_start, align_size);
> +	arch_remove_memory(align_start, align_size, MEMORY_DEVICE);
>  	untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
>  	pgmap_radix_release(res);
>  	dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
> @@ -358,7 +358,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
>  	if (error)
>  		goto err_pfn_remap;
>  
> -	error = arch_add_memory(nid, align_start, align_size, true);
> +	error = arch_add_memory(nid, align_start, align_size, MEMORY_DEVICE);
>  	if (error)
>  		goto err_add_memory;
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 9629273..b2942d7 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1386,7 +1386,7 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
>  	}
>  
>  	/* call arch's memory hotadd */
> -	ret = arch_add_memory(nid, start, size, false);
> +	ret = arch_add_memory(nid, start, size, MEMORY_FLAGS_NONE);
>  
>  	if (ret < 0)
>  		goto error;
> @@ -2205,7 +2205,7 @@ void __ref remove_memory(int nid, u64 start, u64 size)
>  	memblock_free(start, size);
>  	memblock_remove(start, size);
>  
> -	arch_remove_memory(start, size);
> +	arch_remove_memory(start, size, MEMORY_FLAGS_NONE);

Right, these are system RAM hotplug paths and device memory should
never get plugged in/out from this path.

[toc] | [prev] | [next] | [standalone]

#1526624 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On Mon, Nov 21, 2016 at 12:11:50PM +0530, Anshuman Khandual wrote:
> On 11/18/2016 11:48 PM, Jérôme Glisse wrote:

[...]

> > @@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
> >  	remove_pagetable(start, end, true);
> >  }
> >  
> > -int __ref arch_remove_memory(u64 start, u64 size)
> > +int __ref arch_remove_memory(u64 start, u64 size, int flags)
> >  {
> >  	unsigned long start_pfn = start >> PAGE_SHIFT;
> >  	unsigned long nr_pages = size >> PAGE_SHIFT;
> > @@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
> >  	struct zone *zone;
> >  	int ret;
> >  
> > +	/* Need to add support for device and unaddressable memory if needed */
> > +	if (flags & MEMORY_UNADDRESSABLE) {
> > +		BUG();
> > +		return -EINVAL;
> > +	}
> > +
> >  	/* With altmap the first mapped page is offset from @start */
> >  	altmap = to_vmem_altmap((unsigned long) page);
> >  	if (altmap)
> 
> So with this patch none of the architectures support un-addressable
> memory but then support will be added through later patches ?
> zone_for_memory function's flag now takes MEMORY_DEVICE parameter.
> Then we need to change all the previous ZONE_DEVICE changes which
> ever took "for_device" to accommodate this new flag ? just curious.

Yes correct.


> > diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> > index 01033fa..ba9b12e 100644
> > --- a/include/linux/memory_hotplug.h
> > +++ b/include/linux/memory_hotplug.h
> > @@ -103,7 +103,7 @@ extern bool memhp_auto_online;
> >  
> >  #ifdef CONFIG_MEMORY_HOTREMOVE
> >  extern bool is_pageblock_removable_nolock(struct page *page);
> > -extern int arch_remove_memory(u64 start, u64 size);
> > +extern int arch_remove_memory(u64 start, u64 size, int flags);
> >  extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
> >  	unsigned long nr_pages);
> >  #endif /* CONFIG_MEMORY_HOTREMOVE */
> > @@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
> >  extern int add_memory_resource(int nid, struct resource *resource, bool online);
> >  extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
> >  		bool for_device);
> > -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
> > +
> > +/*
> > + * For device memory we want more informations than just knowing it is device
> > + * memory. We want to know if we can migrate it (ie it is not storage memory
> > + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
> > + * memory can not be access by CPU but we still want struct page so that we
> > + * can use it like regular memory.
> 
> Some typos here. Needs to be cleaned up as well. But please have a
> look at comment below over the classification itself.
> 
> > + */
> > +#define MEMORY_FLAGS_NONE 0
> > +#define MEMORY_DEVICE (1 << 0)
> > +#define MEMORY_MOVABLE (1 << 1)
> > +#define MEMORY_UNADDRESSABLE (1 << 2)
> 
> It should be DEVICE_MEMORY_* instead of MEMORY_* as we are trying to
> classify device memory (though they are represented with struct page)
> not regular system ram memory. This should attempt to classify device
> memory which is backed by struct pages. arch_add_memory/arch_remove
> _memory does not come into play if it's traditional device memory
> which is just PFN and does not have struct page associated with it.

Good idea i will change that.


> Broadly they are either CPU accessible or in-accessible. Storage
> memory like persistent memory represented though ZONE_DEVICE fall
> under the accessible (coherent) category. IIUC right now they are
> not movable because page->pgmap replaces page->lru in struct page
> hence its inability to be on standard LRU lists as one of the
> reasons. As there was a need to have struct page to exploit more
> core VM features on these memory going forward it will have to be
> migratable one way or the other to accommodate features like
> compaction, HW poison etc in these storage memory. Hence my point
> here is lets not classify any of these memories as non-movable.
> Just addressable or not should be the only classification.

Being on the lru or not is not and issue in respect to migration. Being
on the lru was use as an indication that the page is manage through the
standard mm code and thus that many assumptions hold which in turn do
allow migration. But if one use device memory following all rules of
regular memory then migration can be done to no matter if page is on
lru or not.

I still think that the MOVABLE is an important distinction as i am pretty
sure that the persistent folks do not want to see their page migrated in
anyway. I might rename it to DEVICE_MEMORY_ALLOW_MIGRATION.

Cheers,
Jérôme 

[toc] | [prev] | [next] | [standalone]

#1527239 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On 11/21/2016 05:57 PM, Jerome Glisse wrote:
> On Mon, Nov 21, 2016 at 12:11:50PM +0530, Anshuman Khandual wrote:
>> On 11/18/2016 11:48 PM, Jérôme Glisse wrote:
> 
> [...]
> 
>>> @@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
>>>  	remove_pagetable(start, end, true);
>>>  }
>>>  
>>> -int __ref arch_remove_memory(u64 start, u64 size)
>>> +int __ref arch_remove_memory(u64 start, u64 size, int flags)
>>>  {
>>>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>>>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>>> @@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
>>>  	struct zone *zone;
>>>  	int ret;
>>>  
>>> +	/* Need to add support for device and unaddressable memory if needed */
>>> +	if (flags & MEMORY_UNADDRESSABLE) {
>>> +		BUG();
>>> +		return -EINVAL;
>>> +	}
>>> +
>>>  	/* With altmap the first mapped page is offset from @start */
>>>  	altmap = to_vmem_altmap((unsigned long) page);
>>>  	if (altmap)
>>
>> So with this patch none of the architectures support un-addressable
>> memory but then support will be added through later patches ?
>> zone_for_memory function's flag now takes MEMORY_DEVICE parameter.
>> Then we need to change all the previous ZONE_DEVICE changes which
>> ever took "for_device" to accommodate this new flag ? just curious.
> 
> Yes correct.
> 
> 
>>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>>> index 01033fa..ba9b12e 100644
>>> --- a/include/linux/memory_hotplug.h
>>> +++ b/include/linux/memory_hotplug.h
>>> @@ -103,7 +103,7 @@ extern bool memhp_auto_online;
>>>  
>>>  #ifdef CONFIG_MEMORY_HOTREMOVE
>>>  extern bool is_pageblock_removable_nolock(struct page *page);
>>> -extern int arch_remove_memory(u64 start, u64 size);
>>> +extern int arch_remove_memory(u64 start, u64 size, int flags);
>>>  extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
>>>  	unsigned long nr_pages);
>>>  #endif /* CONFIG_MEMORY_HOTREMOVE */
>>> @@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
>>>  extern int add_memory_resource(int nid, struct resource *resource, bool online);
>>>  extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
>>>  		bool for_device);
>>> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
>>> +
>>> +/*
>>> + * For device memory we want more informations than just knowing it is device
>>> + * memory. We want to know if we can migrate it (ie it is not storage memory
>>> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
>>> + * memory can not be access by CPU but we still want struct page so that we
>>> + * can use it like regular memory.
>>
>> Some typos here. Needs to be cleaned up as well. But please have a
>> look at comment below over the classification itself.
>>
>>> + */
>>> +#define MEMORY_FLAGS_NONE 0
>>> +#define MEMORY_DEVICE (1 << 0)
>>> +#define MEMORY_MOVABLE (1 << 1)
>>> +#define MEMORY_UNADDRESSABLE (1 << 2)
>>
>> It should be DEVICE_MEMORY_* instead of MEMORY_* as we are trying to
>> classify device memory (though they are represented with struct page)
>> not regular system ram memory. This should attempt to classify device
>> memory which is backed by struct pages. arch_add_memory/arch_remove
>> _memory does not come into play if it's traditional device memory
>> which is just PFN and does not have struct page associated with it.
> 
> Good idea i will change that.
> 
> 
>> Broadly they are either CPU accessible or in-accessible. Storage
>> memory like persistent memory represented though ZONE_DEVICE fall
>> under the accessible (coherent) category. IIUC right now they are
>> not movable because page->pgmap replaces page->lru in struct page
>> hence its inability to be on standard LRU lists as one of the
>> reasons. As there was a need to have struct page to exploit more
>> core VM features on these memory going forward it will have to be
>> migratable one way or the other to accommodate features like
>> compaction, HW poison etc in these storage memory. Hence my point
>> here is lets not classify any of these memories as non-movable.
>> Just addressable or not should be the only classification.
> 
> Being on the lru or not is not and issue in respect to migration. Being

Right, provided we we create separate migration interfaces for these non
LRU pages (preferably through HMM migration API layer). But where it
stands today, for NUMA migrate_pages() interface device non LRU memory
is a problem and we cannot use it for migration. Hence I brought up the
non LRU issue here.

> on the lru was use as an indication that the page is manage through the
> standard mm code and thus that many assumptions hold which in turn do
> allow migration. But if one use device memory following all rules of
> regular memory then migration can be done to no matter if page is on
> lru or not.

Right.

> 
> I still think that the MOVABLE is an important distinction as i am pretty
> sure that the persistent folks do not want to see their page migrated in
> anyway. I might rename it to DEVICE_MEMORY_ALLOW_MIGRATION.

We should not classify memory based on whether there is a *requirement*
for migration or not at this point of time, the classification should
be done if its inherently migratable or not. I dont see any reason why
persistent memory cannot be migrated. I am not very familiar with DAX
file system and its use of persistent memory but I would guess that
their requirement for compaction and error handling happens way above
in file system layers, hence they never needed these support at struct
page level. I am just guessing.

Added Dan J Williams in this thread list, he might be able to give us
some more details regarding persistent memory migration requirements
and it's current state.

[toc] | [prev] | [next] | [standalone]

#1527530 — Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

On Tue, Nov 22, 2016 at 11:05:30AM +0530, Anshuman Khandual wrote:
> On 11/21/2016 05:57 PM, Jerome Glisse wrote:
> > On Mon, Nov 21, 2016 at 12:11:50PM +0530, Anshuman Khandual wrote:
> >> On 11/18/2016 11:48 PM, Jérôme Glisse wrote:
> > 
> > [...]
> > 
> >>> @@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
> >>>  	remove_pagetable(start, end, true);
> >>>  }
> >>>  
> >>> -int __ref arch_remove_memory(u64 start, u64 size)
> >>> +int __ref arch_remove_memory(u64 start, u64 size, int flags)
> >>>  {
> >>>  	unsigned long start_pfn = start >> PAGE_SHIFT;
> >>>  	unsigned long nr_pages = size >> PAGE_SHIFT;
> >>> @@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
> >>>  	struct zone *zone;
> >>>  	int ret;
> >>>  
> >>> +	/* Need to add support for device and unaddressable memory if needed */
> >>> +	if (flags & MEMORY_UNADDRESSABLE) {
> >>> +		BUG();
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>>  	/* With altmap the first mapped page is offset from @start */
> >>>  	altmap = to_vmem_altmap((unsigned long) page);
> >>>  	if (altmap)
> >>
> >> So with this patch none of the architectures support un-addressable
> >> memory but then support will be added through later patches ?
> >> zone_for_memory function's flag now takes MEMORY_DEVICE parameter.
> >> Then we need to change all the previous ZONE_DEVICE changes which
> >> ever took "for_device" to accommodate this new flag ? just curious.
> > 
> > Yes correct.
> > 
> > 
> >>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> >>> index 01033fa..ba9b12e 100644
> >>> --- a/include/linux/memory_hotplug.h
> >>> +++ b/include/linux/memory_hotplug.h
> >>> @@ -103,7 +103,7 @@ extern bool memhp_auto_online;
> >>>  
> >>>  #ifdef CONFIG_MEMORY_HOTREMOVE
> >>>  extern bool is_pageblock_removable_nolock(struct page *page);
> >>> -extern int arch_remove_memory(u64 start, u64 size);
> >>> +extern int arch_remove_memory(u64 start, u64 size, int flags);
> >>>  extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
> >>>  	unsigned long nr_pages);
> >>>  #endif /* CONFIG_MEMORY_HOTREMOVE */
> >>> @@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
> >>>  extern int add_memory_resource(int nid, struct resource *resource, bool online);
> >>>  extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
> >>>  		bool for_device);
> >>> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
> >>> +
> >>> +/*
> >>> + * For device memory we want more informations than just knowing it is device
> >>> + * memory. We want to know if we can migrate it (ie it is not storage memory
> >>> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
> >>> + * memory can not be access by CPU but we still want struct page so that we
> >>> + * can use it like regular memory.
> >>
> >> Some typos here. Needs to be cleaned up as well. But please have a
> >> look at comment below over the classification itself.
> >>
> >>> + */
> >>> +#define MEMORY_FLAGS_NONE 0
> >>> +#define MEMORY_DEVICE (1 << 0)
> >>> +#define MEMORY_MOVABLE (1 << 1)
> >>> +#define MEMORY_UNADDRESSABLE (1 << 2)
> >>
> >> It should be DEVICE_MEMORY_* instead of MEMORY_* as we are trying to
> >> classify device memory (though they are represented with struct page)
> >> not regular system ram memory. This should attempt to classify device
> >> memory which is backed by struct pages. arch_add_memory/arch_remove
> >> _memory does not come into play if it's traditional device memory
> >> which is just PFN and does not have struct page associated with it.
> > 
> > Good idea i will change that.
> > 
> > 
> >> Broadly they are either CPU accessible or in-accessible. Storage
> >> memory like persistent memory represented though ZONE_DEVICE fall
> >> under the accessible (coherent) category. IIUC right now they are
> >> not movable because page->pgmap replaces page->lru in struct page
> >> hence its inability to be on standard LRU lists as one of the
> >> reasons. As there was a need to have struct page to exploit more
> >> core VM features on these memory going forward it will have to be
> >> migratable one way or the other to accommodate features like
> >> compaction, HW poison etc in these storage memory. Hence my point
> >> here is lets not classify any of these memories as non-movable.
> >> Just addressable or not should be the only classification.
> > 
> > Being on the lru or not is not and issue in respect to migration. Being
> 
> Right, provided we we create separate migration interfaces for these non
> LRU pages (preferably through HMM migration API layer). But where it
> stands today, for NUMA migrate_pages() interface device non LRU memory
> is a problem and we cannot use it for migration. Hence I brought up the
> non LRU issue here.
> 
> > on the lru was use as an indication that the page is manage through the
> > standard mm code and thus that many assumptions hold which in turn do
> > allow migration. But if one use device memory following all rules of
> > regular memory then migration can be done to no matter if page is on
> > lru or not.
> 
> Right.
> 
> > 
> > I still think that the MOVABLE is an important distinction as i am pretty
> > sure that the persistent folks do not want to see their page migrated in
> > anyway. I might rename it to DEVICE_MEMORY_ALLOW_MIGRATION.
> 
> We should not classify memory based on whether there is a *requirement*
> for migration or not at this point of time, the classification should
> be done if its inherently migratable or not. I dont see any reason why
> persistent memory cannot be migrated. I am not very familiar with DAX
> file system and its use of persistent memory but I would guess that
> their requirement for compaction and error handling happens way above
> in file system layers, hence they never needed these support at struct
> page level. I am just guessing.
> 
> Added Dan J Williams in this thread list, he might be able to give us
> some more details regarding persistent memory migration requirements
> and it's current state.
> 

Well my patches change even the existing migrate code and thus i need to make
sure i do not change behavior that DAX/persistent memory folks rely on. Hence
i need a flag to allow device page migration or not. If that flag prove useless
latter on it can be remove but for now to maintain existing behavior it is
needed.

Cheers,
Jérôme

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web