[PATCH akpm/next 0/4] Fix various issues in new hmm driver

[PATCH akpm/next 0/4] Fix various issues in new hmm driver

[Paul Gortmaker]

My build coverage for non-modules adding dead modular code
tripped on this recently new addition.  After removing that and
rebuilding, I saw the driver still had warnings that were independent
of the "demodular" change, which the 2nd commit fixes.

Then there was "select" usage that was bound to cause troubles because
it was selecting an option with explicit dependencies.  We decouple by
adding an ARCH_HAS so the selects are valid while keeping the depends
listed in a single place.  We do this so we can update the parent depends
properly (and only once) in 4/4.

Build tested for x86-64, arm64 and s390.  Other 64 bit arch don't build
and hence appear to be not applicable, as per details in patch 4/4.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

---

Paul Gortmaker (4):
  mm/hmm: make it explicitly non-modular
  mm/hmm: fix warnings and SECTION_SIZE definition overlap
  mm/hmm: fix Kconfig to have valid usage of "select"
  mm/hmm: exclude 64 bit arch that explicitly fail to work.

 mm/Kconfig | 23 +++++++++--------------
 mm/hmm.c   | 37 +++++++++++++++----------------------
 2 files changed, 24 insertions(+), 36 deletions(-)

-- 
2.11.0

[PATCH akpm/next 0/4] Fix various issues in new hmm driver

[Paul Gortmaker]

My build coverage for non-modules adding dead modular code
tripped on this recently new addition.  After removing that and
rebuilding, I saw the driver still had warnings that were independent
of the "demodular" change, which the 2nd commit fixes.

Then there was "select" usage that was bound to cause troubles because
it was selecting an option with explicit dependencies.  We decouple by
adding an ARCH_HAS so the selects are valid while keeping the depends
listed in a single place.  We do this so we can update the parent depends
properly (and only once) in 4/4.

Build tested for x86-64, arm64 and s390.  Other 64 bit arch don't build
and hence appear to be not applicable, as per details in patch 4/4.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

---

Paul Gortmaker (4):
  mm/hmm: make it explicitly non-modular
  mm/hmm: fix warnings and SECTION_SIZE definition overlap
  mm/hmm: fix Kconfig to have valid usage of "select"
  mm/hmm: exclude 64 bit arch that explicitly fail to work.

 mm/Kconfig | 23 +++++++++--------------
 mm/hmm.c   | 37 +++++++++++++++----------------------
 2 files changed, 24 insertions(+), 36 deletions(-)

-- 
2.11.0

[PATCH 1/4] mm/hmm: make it explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

mm/Kconfig:config HMM
mm/Kconfig:     bool

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/hmm.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index f567a8b3b079..8200cf275fb9 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -19,12 +19,12 @@
  */
 #include <linux/mm.h>
 #include <linux/hmm.h>
+#include <linux/init.h>
 #include <linux/rmap.h>
 #include <linux/swap.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/mmzone.h>
-#include <linux/module.h>
 #include <linux/pagemap.h>
 #include <linux/swapops.h>
 #include <linux/hugetlb.h>
@@ -1192,14 +1192,5 @@ static int __init hmm_init(void)
 	}
 	return 0;
 }
-
-static void __exit hmm_exit(void)
-{
-	unregister_chrdev_region(hmm_device_devt, HMM_DEVICE_MAX);
-	class_destroy(hmm_device_class);
-}
-
-module_init(hmm_init);
-module_exit(hmm_exit);
-MODULE_LICENSE("GPL");
+device_initcall(hmm_init);
 #endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
-- 
2.11.0

[PATCH 1/4] mm/hmm: make it explicitly non-modular

[Paul Gortmaker]

The Kconfig currently controlling compilation of this code is:

mm/Kconfig:config HMM
mm/Kconfig:     bool

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/hmm.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index f567a8b3b079..8200cf275fb9 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -19,12 +19,12 @@
  */
 #include <linux/mm.h>
 #include <linux/hmm.h>
+#include <linux/init.h>
 #include <linux/rmap.h>
 #include <linux/swap.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/mmzone.h>
-#include <linux/module.h>
 #include <linux/pagemap.h>
 #include <linux/swapops.h>
 #include <linux/hugetlb.h>
@@ -1192,14 +1192,5 @@ static int __init hmm_init(void)
 	}
 	return 0;
 }
-
-static void __exit hmm_exit(void)
-{
-	unregister_chrdev_region(hmm_device_devt, HMM_DEVICE_MAX);
-	class_destroy(hmm_device_class);
-}
-
-module_init(hmm_init);
-module_exit(hmm_exit);
-MODULE_LICENSE("GPL");
+device_initcall(hmm_init);
 #endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
-- 
2.11.0

[PATCH 2/4] mm/hmm: fix warnings and SECTION_SIZE definition overlap

[Paul Gortmaker]

SECTION_SIZE is defined twice in the same file; also it is already
used by arch specific headers, which leads to:

mm/hmm.c:34:0: warning: "SECTION_SIZE" redefined [enabled by default]
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 ^
arch/arm64/include/asm/pgtable-hwdef.h:87:0: note: this is the location of the previous definition
 #define SECTION_SIZE  (_AC(1, UL) << SECTION_SHIFT)

Replace instances of SECTION_SIZE with PA_SECTION_SIZE to avoid all
conflict and delete the 2nd instance of the definition in the file.

Also we see this warning:

mm/hmm.c: In function ‘hmm_devmem_release’:
mm/hmm.c:816:2: error: implicit declaration of function ‘arch_remove_memory’ [-Werror=implicit-function-declaration]
  arch_remove_memory(align_start, align_size, devmem->pagemap.type);

... which has two factors: (1) there is an implicit expectation of the
memory hotplug header being present, and (2) the arch_remove_memory()
support depends on CONFIG_MEMORY_HOTREMOVE.  So we fix that as well.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/hmm.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index 8200cf275fb9..080d71e403e7 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -30,8 +30,9 @@
 #include <linux/hugetlb.h>
 #include <linux/memremap.h>
 #include <linux/mmu_notifier.h>
+#include <linux/memory_hotplug.h>
 
-#define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
+#define PA_SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 
 static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
 
@@ -781,18 +782,17 @@ static void hmm_devmem_free(struct page *page, void *data)
 
 static DEFINE_MUTEX(hmm_devmem_lock);
 static RADIX_TREE(hmm_devmem_radix, GFP_KERNEL);
-#define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 
 static void hmm_devmem_radix_release(struct resource *resource)
 {
 	resource_size_t key, align_start, align_size, align_end;
 
-	align_start = resource->start & ~(SECTION_SIZE - 1);
-	align_size = ALIGN(resource_size(resource), SECTION_SIZE);
+	align_start = resource->start & ~(PA_SECTION_SIZE - 1);
+	align_size = ALIGN(resource_size(resource), PA_SECTION_SIZE);
 	align_end = align_start + align_size - 1;
 
 	mutex_lock(&hmm_devmem_lock);
-	for (key = resource->start; key <= resource->end; key += SECTION_SIZE)
+	for (key = resource->start; key <= resource->end; key += PA_SECTION_SIZE)
 		radix_tree_delete(&hmm_devmem_radix, key >> PA_SECTION_SHIFT);
 	mutex_unlock(&hmm_devmem_lock);
 }
@@ -809,12 +809,14 @@ static void hmm_devmem_release(struct device *dev, void *data)
 	}
 
 	/* pages are dead and unused, undo the arch mapping */
-	align_start = resource->start & ~(SECTION_SIZE - 1);
-	align_size = ALIGN(resource_size(resource), SECTION_SIZE);
+	align_start = resource->start & ~(PA_SECTION_SIZE - 1);
+	align_size = ALIGN(resource_size(resource), PA_SECTION_SIZE);
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
 	mem_hotplug_begin();
 	arch_remove_memory(align_start, align_size, devmem->pagemap.type);
 	mem_hotplug_done();
+#endif
 
 	untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
 	hmm_devmem_radix_release(resource);
@@ -835,10 +837,10 @@ static int hmm_devmem_pages_create(struct hmm_devmem *devmem)
 	int ret, nid, is_ram;
 	unsigned long pfn;
 
-	align_start = devmem->resource->start & ~(SECTION_SIZE - 1);
+	align_start = devmem->resource->start & ~(PA_SECTION_SIZE - 1);
 	align_size = ALIGN(devmem->resource->start +
 			   resource_size(devmem->resource),
-			   SECTION_SIZE) - align_start;
+			   PA_SECTION_SIZE) - align_start;
 
 	is_ram = region_intersects(align_start, align_size,
 				   IORESOURCE_SYSTEM_RAM,
@@ -861,7 +863,7 @@ static int hmm_devmem_pages_create(struct hmm_devmem *devmem)
 
 	mutex_lock(&hmm_devmem_lock);
 	align_end = align_start + align_size - 1;
-	for (key = align_start; key <= align_end; key += SECTION_SIZE) {
+	for (key = align_start; key <= align_end; key += PA_SECTION_SIZE) {
 		struct hmm_devmem *dup;
 
 		rcu_read_lock();
@@ -979,7 +981,7 @@ struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
 	if (ret)
 		goto error_devm_add_action;
 
-	size = ALIGN(size, SECTION_SIZE);
+	size = ALIGN(size, PA_SECTION_SIZE);
 	addr = (iomem_resource.end + 1ULL) - size;
 
 	/*
-- 
2.11.0

[PATCH 2/4] mm/hmm: fix warnings and SECTION_SIZE definition overlap

[Paul Gortmaker]

SECTION_SIZE is defined twice in the same file; also it is already
used by arch specific headers, which leads to:

mm/hmm.c:34:0: warning: "SECTION_SIZE" redefined [enabled by default]
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 ^
arch/arm64/include/asm/pgtable-hwdef.h:87:0: note: this is the location of the previous definition
 #define SECTION_SIZE  (_AC(1, UL) << SECTION_SHIFT)

Replace instances of SECTION_SIZE with PA_SECTION_SIZE to avoid all
conflict and delete the 2nd instance of the definition in the file.

Also we see this warning:

mm/hmm.c: In function ‘hmm_devmem_release’:
mm/hmm.c:816:2: error: implicit declaration of function ‘arch_remove_memory’ [-Werror=implicit-function-declaration]
  arch_remove_memory(align_start, align_size, devmem->pagemap.type);

... which has two factors: (1) there is an implicit expectation of the
memory hotplug header being present, and (2) the arch_remove_memory()
support depends on CONFIG_MEMORY_HOTREMOVE.  So we fix that as well.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/hmm.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index 8200cf275fb9..080d71e403e7 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -30,8 +30,9 @@
 #include <linux/hugetlb.h>
 #include <linux/memremap.h>
 #include <linux/mmu_notifier.h>
+#include <linux/memory_hotplug.h>
 
-#define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
+#define PA_SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 
 static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
 
@@ -781,18 +782,17 @@ static void hmm_devmem_free(struct page *page, void *data)
 
 static DEFINE_MUTEX(hmm_devmem_lock);
 static RADIX_TREE(hmm_devmem_radix, GFP_KERNEL);
-#define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
 
 static void hmm_devmem_radix_release(struct resource *resource)
 {
 	resource_size_t key, align_start, align_size, align_end;
 
-	align_start = resource->start & ~(SECTION_SIZE - 1);
-	align_size = ALIGN(resource_size(resource), SECTION_SIZE);
+	align_start = resource->start & ~(PA_SECTION_SIZE - 1);
+	align_size = ALIGN(resource_size(resource), PA_SECTION_SIZE);
 	align_end = align_start + align_size - 1;
 
 	mutex_lock(&hmm_devmem_lock);
-	for (key = resource->start; key <= resource->end; key += SECTION_SIZE)
+	for (key = resource->start; key <= resource->end; key += PA_SECTION_SIZE)
 		radix_tree_delete(&hmm_devmem_radix, key >> PA_SECTION_SHIFT);
 	mutex_unlock(&hmm_devmem_lock);
 }
@@ -809,12 +809,14 @@ static void hmm_devmem_release(struct device *dev, void *data)
 	}
 
 	/* pages are dead and unused, undo the arch mapping */
-	align_start = resource->start & ~(SECTION_SIZE - 1);
-	align_size = ALIGN(resource_size(resource), SECTION_SIZE);
+	align_start = resource->start & ~(PA_SECTION_SIZE - 1);
+	align_size = ALIGN(resource_size(resource), PA_SECTION_SIZE);
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
 	mem_hotplug_begin();
 	arch_remove_memory(align_start, align_size, devmem->pagemap.type);
 	mem_hotplug_done();
+#endif
 
 	untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
 	hmm_devmem_radix_release(resource);
@@ -835,10 +837,10 @@ static int hmm_devmem_pages_create(struct hmm_devmem *devmem)
 	int ret, nid, is_ram;
 	unsigned long pfn;
 
-	align_start = devmem->resource->start & ~(SECTION_SIZE - 1);
+	align_start = devmem->resource->start & ~(PA_SECTION_SIZE - 1);
 	align_size = ALIGN(devmem->resource->start +
 			   resource_size(devmem->resource),
-			   SECTION_SIZE) - align_start;
+			   PA_SECTION_SIZE) - align_start;
 
 	is_ram = region_intersects(align_start, align_size,
 				   IORESOURCE_SYSTEM_RAM,
@@ -861,7 +863,7 @@ static int hmm_devmem_pages_create(struct hmm_devmem *devmem)
 
 	mutex_lock(&hmm_devmem_lock);
 	align_end = align_start + align_size - 1;
-	for (key = align_start; key <= align_end; key += SECTION_SIZE) {
+	for (key = align_start; key <= align_end; key += PA_SECTION_SIZE) {
 		struct hmm_devmem *dup;
 
 		rcu_read_lock();
@@ -979,7 +981,7 @@ struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
 	if (ret)
 		goto error_devm_add_action;
 
-	size = ALIGN(size, SECTION_SIZE);
+	size = ALIGN(size, PA_SECTION_SIZE);
 	addr = (iomem_resource.end + 1ULL) - size;
 
 	/*
-- 
2.11.0

[PATCH 3/4] mm/hmm: fix Kconfig to have valid usage of "select"

[Paul Gortmaker]

The use of "select" like this will most likely cause new
randconfig warnings and failures because the child categories
get chosen, and force select the parent, even though the parent
dependencies are not met.  For further details, see "select"
in Documentation/kbuild/kconfig-language.txt file.

But we also don't want to be duplicating the main feature dependencies
across all the child options, which is annoying when the dependencies
change.  So we use the typical ARCH_HAS as a dependency collector,
and the existing HMM option is now dependency free, and hence a
valid option for use with "select".  It retains its original purpose
which is to be set if and only if any of the child options are set.

Finally, the existing help text is currently orphaned, since the
HMM bool doesn't have a following short description, and hence it
will never be offered to the end user.  So delete it, but ensure
the two child options point to the Documentation/vm/hmm.txt file.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/Kconfig | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 43d000e44424..3e9c31bf9aaa 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -289,23 +289,17 @@ config MIGRATION
 config ARCH_ENABLE_HUGEPAGE_MIGRATION
 	bool
 
-config HMM
+config ARCH_HAS_HMM
 	bool
+	default y
 	depends on MMU && 64BIT
-	help
-	  HMM provides a set of helpers to share a virtual address
-	  space between CPU and a device, so that the device can access any valid
-	  address of the process (while still obeying memory protection). HMM also
-	  provides helpers to migrate process memory to device memory, and back.
-	  Each set of functionality (address space mirroring, and migration to and
-	  from device memory) can be used independently of the other.
 
-	  This is primarily useful for devices like GPU, for GPGPU compute workload,
-	  with APIs such as OpenCL or CUDA. See Documentation/vm/hmm.txt.
+config HMM
+	bool
 
 config HMM_MIRROR
 	bool "HMM mirror CPU page table into a device page table"
-	depends on MMU && 64BIT
+	depends on ARCH_HAS_HMM
 	select HMM
 	select MMU_NOTIFIER
 	help
@@ -313,16 +307,16 @@ config HMM_MIRROR
 	  process into a device page table. Here, mirror means "keep synchronized".
 	  Prerequisites: the device must provide the ability to write-protect its
 	  page tables (at PAGE_SIZE granularity), and must be able to recover from
-	  the resulting potential page faults.
+	  the resulting potential page faults.  See Documentation/vm/hmm.txt.
 
 config HMM_DEVMEM
 	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
-	depends on MMU && 64BIT
+	depends on ARCH_HAS_HMM
 	select HMM
 	help
 	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
 	  feature. This is just to avoid having device drivers to replicating a lot
-	  of boiler plate code.
+	  of boiler plate code.  See Documentation/vm/hmm.txt.
 
 config PHYS_ADDR_T_64BIT
 	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
-- 
2.11.0

[PATCH 3/4] mm/hmm: fix Kconfig to have valid usage of "select"

[Paul Gortmaker]

The use of "select" like this will most likely cause new
randconfig warnings and failures because the child categories
get chosen, and force select the parent, even though the parent
dependencies are not met.  For further details, see "select"
in Documentation/kbuild/kconfig-language.txt file.

But we also don't want to be duplicating the main feature dependencies
across all the child options, which is annoying when the dependencies
change.  So we use the typical ARCH_HAS as a dependency collector,
and the existing HMM option is now dependency free, and hence a
valid option for use with "select".  It retains its original purpose
which is to be set if and only if any of the child options are set.

Finally, the existing help text is currently orphaned, since the
HMM bool doesn't have a following short description, and hence it
will never be offered to the end user.  So delete it, but ensure
the two child options point to the Documentation/vm/hmm.txt file.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/Kconfig | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 43d000e44424..3e9c31bf9aaa 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -289,23 +289,17 @@ config MIGRATION
 config ARCH_ENABLE_HUGEPAGE_MIGRATION
 	bool
 
-config HMM
+config ARCH_HAS_HMM
 	bool
+	default y
 	depends on MMU && 64BIT
-	help
-	  HMM provides a set of helpers to share a virtual address
-	  space between CPU and a device, so that the device can access any valid
-	  address of the process (while still obeying memory protection). HMM also
-	  provides helpers to migrate process memory to device memory, and back.
-	  Each set of functionality (address space mirroring, and migration to and
-	  from device memory) can be used independently of the other.
 
-	  This is primarily useful for devices like GPU, for GPGPU compute workload,
-	  with APIs such as OpenCL or CUDA. See Documentation/vm/hmm.txt.
+config HMM
+	bool
 
 config HMM_MIRROR
 	bool "HMM mirror CPU page table into a device page table"
-	depends on MMU && 64BIT
+	depends on ARCH_HAS_HMM
 	select HMM
 	select MMU_NOTIFIER
 	help
@@ -313,16 +307,16 @@ config HMM_MIRROR
 	  process into a device page table. Here, mirror means "keep synchronized".
 	  Prerequisites: the device must provide the ability to write-protect its
 	  page tables (at PAGE_SIZE granularity), and must be able to recover from
-	  the resulting potential page faults.
+	  the resulting potential page faults.  See Documentation/vm/hmm.txt.
 
 config HMM_DEVMEM
 	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
-	depends on MMU && 64BIT
+	depends on ARCH_HAS_HMM
 	select HMM
 	help
 	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
 	  feature. This is just to avoid having device drivers to replicating a lot
-	  of boiler plate code.
+	  of boiler plate code.  See Documentation/vm/hmm.txt.
 
 config PHYS_ADDR_T_64BIT
 	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
-- 
2.11.0

[PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.

[Paul Gortmaker]

On sparc64, we see:

  mm/hmm.c: In function 'hmm_vma_walk_pmd':
  mm/hmm.c:371:53: error: macro "pte_index" requires 2 arguments, but only 1 given
  mm/hmm.c:371:39: error: 'pte_index' undeclared (first use in this function)

...and on MIPS 64, we see:

  mm/hmm.c:57:22: error: field 'mmu_notifier' has incomplete type
  mm/hmm.c: In function 'hmm_register':
  mm/hmm.c:98:2: error: implicit declaration of function '__mmu_notifier_register' [-Werror=implicit-function-declaration]
  mm/hmm.c:111:3: error: implicit declaration of function 'mmu_notifier_unregister' [-Werror=implicit-function-declaration]

...and on Alpha, we see:

  mm/hmm.c: In function 'hmm_vma_walk_pmd':
  mm/hmm.c:371:4: error: implicit declaration of function 'pmd_pfn'
  mm/hmm.c:371:4: error: implicit declaration of function 'pte_index'

...and on PaRISC 64 we see:

  include/linux/hmm.h:405:7: warning: 'struct migrate_vma_ops' declared inside parameter list
  include/linux/hmm.h:405:7: warning: its scope is only this definition or declaration, which is probably not what you want
  mm/hmm.c: In function 'hmm_vma_walk_pmd':
  mm/hmm.c:371:4: error: implicit declaration of function 'pmd_pfn'
  [...]

Set the dependency to the three arch that currently seem to build
without issue -- ARM_64, X86_64 and S390.

Since ia64 and ppc64 don't set CONFIG_64BIT, they were already
excluded by the original dependency.

The failing arch can be re-added as demand dictates, if/when they
have been validated to build and function.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 3e9c31bf9aaa..6c27d2003a63 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -293,6 +293,7 @@ config ARCH_HAS_HMM
 	bool
 	default y
 	depends on MMU && 64BIT
+	depends on ARM64 || X86_64 || S390
 
 config HMM
 	bool
-- 
2.11.0

[PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.

[Paul Gortmaker]

On sparc64, we see:

  mm/hmm.c: In function 'hmm_vma_walk_pmd':
  mm/hmm.c:371:53: error: macro "pte_index" requires 2 arguments, but only 1 given
  mm/hmm.c:371:39: error: 'pte_index' undeclared (first use in this function)

...and on MIPS 64, we see:

  mm/hmm.c:57:22: error: field 'mmu_notifier' has incomplete type
  mm/hmm.c: In function 'hmm_register':
  mm/hmm.c:98:2: error: implicit declaration of function '__mmu_notifier_register' [-Werror=implicit-function-declaration]
  mm/hmm.c:111:3: error: implicit declaration of function 'mmu_notifier_unregister' [-Werror=implicit-function-declaration]

...and on Alpha, we see:

  mm/hmm.c: In function 'hmm_vma_walk_pmd':
  mm/hmm.c:371:4: error: implicit declaration of function 'pmd_pfn'
  mm/hmm.c:371:4: error: implicit declaration of function 'pte_index'

...and on PaRISC 64 we see:

  include/linux/hmm.h:405:7: warning: 'struct migrate_vma_ops' declared inside parameter list
  include/linux/hmm.h:405:7: warning: its scope is only this definition or declaration, which is probably not what you want
  mm/hmm.c: In function 'hmm_vma_walk_pmd':
  mm/hmm.c:371:4: error: implicit declaration of function 'pmd_pfn'
  [...]

Set the dependency to the three arch that currently seem to build
without issue -- ARM_64, X86_64 and S390.

Since ia64 and ppc64 don't set CONFIG_64BIT, they were already
excluded by the original dependency.

The failing arch can be re-added as demand dictates, if/when they
have been validated to build and function.

Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Evgeny Baskakov <ebaskakov@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Mark Hairgrove <mhairgrove@nvidia.com>
Cc: Sherry Cheung <SCheung@nvidia.com>
Cc: Subhash Gutti <sgutti@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 mm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 3e9c31bf9aaa..6c27d2003a63 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -293,6 +293,7 @@ config ARCH_HAS_HMM
 	bool
 	default y
 	depends on MMU && 64BIT
+	depends on ARM64 || X86_64 || S390
 
 config HMM
 	bool
-- 
2.11.0

Re: [PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.

[Stephen Rothwell]

Hi Paul,

On Wed, 12 Apr 2017 20:30:14 -0400 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>
> Since ia64 and ppc64 don't set CONFIG_64BIT, they were already
> excluded by the original dependency.

My powerpc ppc64_defconfig builds have CONFIG_64BIT set ...

$ grep CONFIG_64BIT ~/next/powerpc_ppc64_defconfig/.config
CONFIG_64BIT=y

-- 
Cheers,
Stephen Rothwell

Re: [PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.

[Paul Gortmaker]

[Re: [PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.] On 13/04/2017 (Thu 13:27) Stephen Rothwell wrote:

> Hi Paul,
> 
> On Wed, 12 Apr 2017 20:30:14 -0400 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> >
> > Since ia64 and ppc64 don't set CONFIG_64BIT, they were already
> > excluded by the original dependency.
> 
> My powerpc ppc64_defconfig builds have CONFIG_64BIT set ...
> 
> $ grep CONFIG_64BIT ~/next/powerpc_ppc64_defconfig/.config
> CONFIG_64BIT=y

I must have fat fingered the grep; I was using the PA Semi defconfig
since I knew that was 64 bit.  I probably searched for 64_BIT or
something stupid.  The underscore ARM64 vs. X86_64 always gets me.

In the end, it doesn't change the commit itself, since the driver
still only builds on X86_64, S390 and ARM64(but only after my patch).
The ppc64 was also never compile tested it seems.

I can tweak the commit log of this patch in a v2 once there has been
a chance for others to put in their feedback as well.

Here is the spew I got when I tried to compile hmm on next/master ppc64.

Paul.
--

  CC      mm/hmm.o
mm/hmm.c: In function 'hmm_devmem_radix_release':
mm/hmm.c:784:30: error: 'PA_SECTION_SHIFT' undeclared (first use in this function)
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
                              ^
mm/hmm.c:790:36: note: in expansion of macro 'SECTION_SIZE'
  align_start = resource->start & ~(SECTION_SIZE - 1);
                                    ^
mm/hmm.c:784:30: note: each undeclared identifier is reported only once for each function it appears in
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
                              ^
mm/hmm.c:790:36: note: in expansion of macro 'SECTION_SIZE'
  align_start = resource->start & ~(SECTION_SIZE - 1);
                                    ^
mm/hmm.c: In function 'hmm_devmem_release':
mm/hmm.c:784:30: error: 'PA_SECTION_SHIFT' undeclared (first use in this function)
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
                              ^
mm/hmm.c:812:36: note: in expansion of macro 'SECTION_SIZE'
  align_start = resource->start & ~(SECTION_SIZE - 1);
                                    ^
mm/hmm.c:816:2: error: implicit declaration of function 'arch_remove_memory' [-Werror=implicit-function-declaration]
  arch_remove_memory(align_start, align_size, devmem->pagemap.type);
  ^
mm/hmm.c: In function 'hmm_devmem_find':
mm/hmm.c:827:54: error: 'PA_SECTION_SHIFT' undeclared (first use in this function)
  return radix_tree_lookup(&hmm_devmem_radix, phys >> PA_SECTION_SHIFT);
                                                      ^
mm/hmm.c: In function 'hmm_devmem_pages_create':
mm/hmm.c:784:30: error: 'PA_SECTION_SHIFT' undeclared (first use in this function)
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
                              ^
mm/hmm.c:838:44: note: in expansion of macro 'SECTION_SIZE'
  align_start = devmem->resource->start & ~(SECTION_SIZE - 1);
                                            ^
In file included from include/linux/cache.h:4:0,
                 from include/linux/printk.h:8,
                 from include/linux/kernel.h:13,
                 from include/asm-generic/bug.h:15,
                 from arch/powerpc/include/asm/bug.h:127,
                 from include/linux/bug.h:4,
                 from include/linux/mmdebug.h:4,
                 from include/linux/mm.h:8,
                 from mm/hmm.c:20:
mm/hmm.c: In function 'hmm_devmem_add':
mm/hmm.c:784:30: error: 'PA_SECTION_SHIFT' undeclared (first use in this function)
 #define SECTION_SIZE (1UL << PA_SECTION_SHIFT)
                              ^
include/uapi/linux/kernel.h:10:47: note: in definition of macro '__ALIGN_KERNEL_MASK'
 #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
                                               ^
include/linux/kernel.h:49:22: note: in expansion of macro '__ALIGN_KERNEL'
 #define ALIGN(x, a)  __ALIGN_KERNEL((x), (a))
                      ^
mm/hmm.c:982:9: note: in expansion of macro 'ALIGN'
  size = ALIGN(size, SECTION_SIZE);
         ^
mm/hmm.c:982:21: note: in expansion of macro 'SECTION_SIZE'
  size = ALIGN(size, SECTION_SIZE);
                     ^
mm/hmm.c: In function 'hmm_devmem_find':
mm/hmm.c:828:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
cc1: some warnings being treated as errors
scripts/Makefile.build:294: recipe for target 'mm/hmm.o' failed
make[2]: *** [mm/hmm.o] Error 1
make[2]: *** Waiting for unfinished jobs....
Makefile:1663: recipe for target 'mm/' failed
make[1]: *** [mm/] Error 2
make[1]: Leaving directory '/home/paul/git/ppc-build'
Makefile:152: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2

Re: [PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.

[Michael Ellerman]

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Hi Paul,
>
> On Wed, 12 Apr 2017 20:30:14 -0400 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>>
>> Since ia64 and ppc64 don't set CONFIG_64BIT, they were already
>> excluded by the original dependency.
>
> My powerpc ppc64_defconfig builds have CONFIG_64BIT set ...
>
> $ grep CONFIG_64BIT ~/next/powerpc_ppc64_defconfig/.config
> CONFIG_64BIT=y

Yeah, arch/powerpc/Kconfig:

config 64BIT
	bool
	default y if PPC64

cheers

Re: [PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.

[Paul Gortmaker]

[Re: [PATCH 4/4] mm/hmm: exclude 64 bit arch that explicitly fail to work.] On 13/04/2017 (Thu 15:12) Michael Ellerman wrote:

> Stephen Rothwell <sfr@canb.auug.org.au> writes:
> 
> > Hi Paul,
> >
> > On Wed, 12 Apr 2017 20:30:14 -0400 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> >>
> >> Since ia64 and ppc64 don't set CONFIG_64BIT, they were already
> >> excluded by the original dependency.
> >
> > My powerpc ppc64_defconfig builds have CONFIG_64BIT set ...
> >
> > $ grep CONFIG_64BIT ~/next/powerpc_ppc64_defconfig/.config
> > CONFIG_64BIT=y
> 
> Yeah, arch/powerpc/Kconfig:
> 
> config 64BIT
> 	bool
> 	default y if PPC64

Yep, as I said to Stephen earlier in this thread, I think I mistakenly
searched for 64_BIT; unfortunately I can't tell since all history has is:

   16  make  O=../ppc-build/ pasemi_defconfig
   17  vi ../ppc-build/.config

Anyway, that mis-documentation in the commit log aside, it seems like
the driver has been pulled from the linux-next content for today;
presumably to be reworked and resubmitted with better arch coverage
to address all the issues raised here.  That is probably the right
approach - we don't need to pollute git history with all that..

Thanks,
Paul.
--

> 
> cheers

Re: [PATCH 2/4] mm/hmm: fix warnings and SECTION_SIZE definition overlap

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

Hi Paul,

[auto build test ERROR on mmotm/master]
[cannot apply to v4.11-rc6 next-20170413]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Gortmaker/Fix-various-issues-in-new-hmm-driver/20170413-132233
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   mm/built-in.o: In function `hmm_devmem_add':
>> usercopy.c:(.text+0xe5b48): undefined reference to `arch_add_memory'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 55238 bytes --]

Re: [PATCH 2/4] mm/hmm: fix warnings and SECTION_SIZE definition overlap

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

Hi Paul,

[auto build test ERROR on mmotm/master]
[cannot apply to v4.11-rc6 next-20170413]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Gortmaker/Fix-various-issues-in-new-hmm-driver/20170413-132233
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   mm/built-in.o: In function `hmm_devmem_add':
>> usercopy.c:(.text+0xe5b48): undefined reference to `arch_add_memory'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 55238 bytes --]