Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
From: John Hubbard
Date: Wed Jun 14 2017 - 19:10:44 EST
On 06/14/2017 01:11 PM, JÃrÃme Glisse wrote:
This just simplify kconfig and allow HMM and DEVICE_PUBLIC to be
selected for ppc64 once ZONE_DEVICE is allowed on ppc64 (different
patchset).
Signed-off-by: JÃrÃme Glisse <jglisse@xxxxxxxxxx>
Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
Cc: Balbir Singh <balbirs@xxxxxxxxxxx>
Cc: Aneesh Kumar <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
---
include/linux/hmm.h | 4 ++--
mm/Kconfig | 27 ++++++---------------------
mm/hmm.c | 4 ++--
3 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index f6713b2..720d18c 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -327,7 +327,7 @@ int hmm_vma_fault(struct vm_area_struct *vma,
#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
struct hmm_devmem;
struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
@@ -456,7 +456,7 @@ struct hmm_device {
*/
struct hmm_device *hmm_device_new(void *drvdata);
void hmm_device_put(struct hmm_device *hmm_device);
-#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
+#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
/* Below are for HMM internal use only! Not to be used by device driver! */
diff --git a/mm/Kconfig b/mm/Kconfig
index ad082b9..7de939a 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -265,7 +265,7 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
config ARCH_HAS_HMM
bool
default y
- depends on X86_64
+ depends on X86_64 || PPC64
depends on ZONE_DEVICE
depends on MMU && 64BIT
depends on MEMORY_HOTPLUG
@@ -277,7 +277,7 @@ config HMM
config HMM_MIRROR
bool "HMM mirror CPU page table into a device page table"
- depends on ARCH_HAS_HMM
+ depends on ARCH_HAS_HMM && X86_64
select MMU_NOTIFIER
select HMM
help
Hi Jerome,
There are still some problems with using this configuration. First and foremost, it is still
possible (and likely, given the complete dissimilarity in naming, and difference in location on the
screen) to choose HMM_MIRROR, and *not* to choose either DEVICE_PRIVATE or DEVICE_PUBLIC. And then
we end up with a swath of important page fault handling code being ifdef'd out, and one ends up
having to investigate why.
As for solutions, at least for the x86 (DEVICE_PRIVATE)case, we could do this:
diff --git a/mm/Kconfig b/mm/Kconfig
index 7de939a29466..f64182d7b956 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -279,6 +279,7 @@ config HMM_MIRROR
bool "HMM mirror CPU page table into a device page table"
depends on ARCH_HAS_HMM && X86_64
select MMU_NOTIFIER
+ select DEVICE_PRIVATE
select HMM
help
Select HMM_MIRROR if you want to mirror range of the CPU page table of a
...and that is better than the other direction (having HMM_MIRROR depend on DEVICE_PRIVATE), because
in the latter case, HMM_MIRROR will disappear (and it's several lines above) until you select
DEVICE_PRIVATE. That is hard to work with for the user.
The user will tend to select HMM_MIRROR, but it is *not* obvious that he/she should also select
DEVICE_PRIVATE. So Kconfig should do it for them.
In fact, I'm not even sure if the DEVICE_PRIVATE and DEVICE_PUBLIC actually need Kconfig protection,
but if they don't, then life would be easier for whoever is configuring their kernel.
@@ -287,15 +287,6 @@ config HMM_MIRROR
page tables (at PAGE_SIZE granularity), and must be able to recover from
the resulting potential page faults.
-config HMM_DEVMEM
- bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
- 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. See Documentation/vm/hmm.txt.
-
Yes, probably good to remove HMM_DEVMEM as a separate conig choice.
config PHYS_ADDR_T_64BIT
def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
@@ -720,11 +711,8 @@ config ZONE_DEVICE
config DEVICE_PRIVATE
bool "Unaddressable device memory (GPU memory, ...)"
- depends on X86_64
- depends on ZONE_DEVICE
- depends on MEMORY_HOTPLUG
- depends on MEMORY_HOTREMOVE
- depends on SPARSEMEM_VMEMMAP
+ depends on ARCH_HAS_HMM && X86_64
+ select HMM
help
Allows creation of struct pages to represent unaddressable device
@@ -733,11 +721,8 @@ config DEVICE_PRIVATE
config DEVICE_PUBLIC
bool "Unaddressable device memory (GPU memory, ...)"
Typo: this is a copy-and-paste from DEVICE_PRIVATE, but the "Unaddressable" part wasn't changed, so
you'll end up with two identical-looking lines in `make menuconfig`.
Maybe "Directly addressable device memory"? And make the line less identical to DEVICE_PRIVATE?
thanks,
--
John Hubbard
NVIDIA
- depends on X86_64
- depends on ZONE_DEVICE
- depends on MEMORY_HOTPLUG
- depends on MEMORY_HOTREMOVE
- depends on SPARSEMEM_VMEMMAP
+ depends on ARCH_HAS_HMM
+ select HMM
help
Allows creation of struct pages to represent addressable device
diff --git a/mm/hmm.c b/mm/hmm.c
index aed110e..085cc06 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -747,7 +747,7 @@ EXPORT_SYMBOL(hmm_vma_fault);
#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
unsigned long addr)
{
@@ -1306,4 +1306,4 @@ static int __init hmm_init(void)
}
device_initcall(hmm_init);
-#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
+#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
--
2.9.3