HMM tests and minor fixes
Subsystems (4)
cover HMM tests and minor fixes
These changes are based on Jason's latest hmm branch.
Patch 1 was previously posted here [1] but was dropped from the orginal series. Hopefully, the tests will reduce concerns about edge conditions. I'm sure more tests could be usefully added but I thought this was a good starting point.
[1] https://lore.kernel.org/linux-mm/[email protected]/
Ralph Campbell (4):
mm/hmm: make full use of walk_page_range() mm/hmm: allow snapshot of the special zero page mm/hmm: allow hmm_range_fault() of mmap(PROT_NONE) mm/hmm/test: add self tests for HMM
MAINTAINERS | 3 + drivers/char/Kconfig | 11 + drivers/char/Makefile | 1 + drivers/char/hmm_dmirror.c | 1504 ++++++++++++++++++++++++ include/Kbuild | 1 + include/uapi/linux/hmm_dmirror.h | 74 ++ mm/hmm.c | 117 +- tools/testing/selftests/vm/.gitignore | 1 + tools/testing/selftests/vm/Makefile | 3 + tools/testing/selftests/vm/config | 3 + tools/testing/selftests/vm/hmm-tests.c | 1304 ++++++++++++++++++++ tools/testing/selftests/vm/run_vmtests | 16 + tools/testing/selftests/vm/test_hmm.sh | 105 ++ 13 files changed, 3079 insertions(+), 64 deletions(-) create mode 100644 drivers/char/hmm_dmirror.c create mode 100644 include/uapi/linux/hmm_dmirror.h create mode 100644 tools/testing/selftests/vm/hmm-tests.c create mode 100755 tools/testing/selftests/vm/test_hmm.sh
Signature · 1 line
2.20.1
PATCH 1/4 mm/hmm: make full use of walk_page_range()
hmm_range_fault() calls find_vma() and walk_page_range() in a loop. This is unnecessary duplication since walk_page_range() calls find_vma() in a loop already.
Simplify hmm_range_fault() by defining a walk_test() callback function to filter unhandled vmas.
This also fixes a bug where hmm_range_fault() was not checking start >= vma->vm_start before checking vma->vm_flags so hmm_range_fault() could return an error based on the wrong vma for the requested range.
--- mm/hmm.c | 113 ++++++++++++++++++++++++------------------------------- 1 file changed, 50 insertions(+), 63 deletions(-)
mm/hmm.c +50 -63
@@ -252,18 +252,17 @@ static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr,return -EFAULT ; } -static int hmm_pfns_bad (unsigned long addr, -unsigned long end, -struct mm_walk *walk) +static int hmm_pfns_fill (unsigned long addr, +unsigned long end, +struct hmm_range *range, +enum hmm_pfn_value_e value) { -struct hmm_vma_walk *hmm_vma_walk = walk->private; -struct hmm_range *range = hmm_vma_walk->range;uint64_t *pfns = range->pfns;unsigned long i; i = (addr - range->start) >>PAGE_SHIFT ;for (; addr < end; addr +=PAGE_SIZE , i++) - pfns[i] = range->values[HMM_PFN_ERROR ]; + pfns[i] = range->values[value];return 0 ; }
@@ -584,7 +583,7 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp, }return 0 ; }else if (!pmd_present (pmd)) -return hmm_pfns_bad (start, end, walk); +return hmm_pfns_fill (start, end, range,HMM_PFN_ERROR );if (pmd_devmap (pmd) ||pmd_trans_huge (pmd)) {/*
@@ -612,7 +611,7 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,* recover. */ if (pmd_bad (pmd)) -return hmm_pfns_bad (start, end, walk); +return hmm_pfns_fill (start, end, range,HMM_PFN_ERROR ); ptep =pte_offset_map (pmdp, addr); i = (addr - range->start) >>PAGE_SHIFT ;
@@ -770,13 +769,36 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask,#define hmm_vma_walk_hugetlb_entry NULL#endif /* CONFIG_HUGETLB_PAGE */ -static void hmm_pfns_clear (struct hmm_range *range, -uint64_t *pfns, -unsigned long addr, -unsigned long end) +static int hmm_vma_walk_test (unsigned long start, +unsigned long end, +struct mm_walk *walk) { -for (; addr < end; addr +=PAGE_SIZE , pfns++) - *pfns = range->values[HMM_PFN_NONE ]; +struct hmm_vma_walk *hmm_vma_walk = walk->private; +struct hmm_range *range = hmm_vma_walk->range; +struct vm_area_struct *vma = walk->vma; + +/* If range is no longer valid, force retry. */ +if (!range->valid) +return -EBUSY ; + +/* +* Skip vma ranges that don't have struct page backing them or +* map I/O devices directly. +*/ +if (vma->vm_flags & (VM_IO |VM_PFNMAP |VM_MIXEDMAP )) +return -EFAULT ; + +/* +* If the vma does not allow read access, then assume that it does not +* allow write access either. HMM does not support architectures +* that allow write without read. +*/ +if (!(vma->vm_flags &VM_READ )) { + (void )hmm_pfns_fill (start, end, range,HMM_PFN_NONE ); +return -EPERM ; + } + +return 0 ; }/*
@@ -857,6 +879,7 @@ static const struct mm_walk_ops hmm_walk_ops = { .pmd_entry = hmm_vma_walk_pmd, .pte_hole = hmm_vma_walk_hole, .hugetlb_entry = hmm_vma_walk_hugetlb_entry, + .test_walk = hmm_vma_walk_test, };/**
@@ -889,63 +912,27 @@ static const struct mm_walk_ops hmm_walk_ops = {*/ long hmm_range_fault (struct hmm_range *range,unsigned int flags) { -const unsigned long device_vma =VM_IO |VM_PFNMAP |VM_MIXEDMAP ; -unsigned long start = range->start, end; -struct hmm_vma_walk hmm_vma_walk; +unsigned long start = range->start; +struct hmm_vma_walk hmm_vma_walk = { + .range = range, + .last = start, + .flags = flags, + };struct hmm *hmm = range->hmm; -struct vm_area_struct *vma;int ret;lockdep_assert_held (&hmm->mmu_notifier.mm->mmap_sem);do { -/* If range is no longer valid force retry. */ -if (!range->valid) -return -EBUSY ; - - vma =find_vma (hmm->mmu_notifier.mm, start); -if (vma ==NULL || (vma->vm_flags & device_vma)) -return -EFAULT ; - -if (!(vma->vm_flags &VM_READ )) { -/* -* If vma do not allow read access, then assume that it -* does not allow write access, either. HMM does not -* support architecture that allow write without read. -*/ -hmm_pfns_clear (range, range->pfns, - range->start, range->end); -return -EPERM ; - } + ret =walk_page_range (hmm->mmu_notifier.mm, start, range->end, + &hmm_walk_ops, &hmm_vma_walk); + start = hmm_vma_walk.last; - hmm_vma_walk.pgmap =NULL ; - hmm_vma_walk.last = start; - hmm_vma_walk.flags = flags; - hmm_vma_walk.range = range; - end =min (range->end, vma->vm_end); +/* Keep trying while the range is valid. */ + }while (ret == -EBUSY && range->valid); -walk_page_range (vma->vm_mm, start, end, &hmm_walk_ops, - &hmm_vma_walk); - -do { - ret =walk_page_range (vma->vm_mm, start, end, - &hmm_walk_ops, &hmm_vma_walk); - start = hmm_vma_walk.last; - -/* Keep trying while the range is valid. */ - }while (ret == -EBUSY && range->valid); - -if (ret) { -unsigned long i; - - i = (hmm_vma_walk.last - range->start) >>PAGE_SHIFT ; -hmm_pfns_clear (range, &range->pfns[i], - hmm_vma_walk.last, range->end); -return ret; - } - start = end; - - }while (start < range->end); +if (ret) +return ret;return (hmm_vma_walk.last - range->start) >>PAGE_SHIFT ; }
Signature · 1 line
2.20.1
Re: [PATCH 1/4] mm/hmm: make full use of walk_page_range()
mm/hmm.c static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr, · line 258
Quoted 4 lines
mm/hmm.c +4
-0+static int hmm_pfns_fill (unsigned long addr, +unsigned long end, +struct hmm_range *range, +enum hmm_pfn_value_e value)
Nit: can we use the space a little more efficient, e.g.:
static int hmm_pfns_fill(unsigned long addr, unsigned long end,
struct hmm_range *range, enum hmm_pfn_value_e value)
mm/hmm.c static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask, · line 774
Quoted 3 lines
mm/hmm.c +3
-0+static int hmm_vma_walk_test (unsigned long start, +unsigned long end, +struct mm_walk *walk)
Same here.
mm/hmm.c static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask, · line 797
Quoted 2 lines
mm/hmm.c +2
-0+if (!(vma->vm_flags &VM_READ )) { + (void )hmm_pfns_fill (start, end, range,HMM_PFN_NONE );
There should be no need for the void cast here.
Re: [PATCH 1/4] mm/hmm: make full use of walk_page_range()
Quoted 16 lines · Christoph Hellwig
mm/hmm.c static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr, · line 258
Quoted 4 lines
mm/hmm.c +4
-0+static int hmm_pfns_fill (unsigned long addr, +unsigned long end, +struct hmm_range *range, +enum hmm_pfn_value_e value)Nit: can we use the space a little more efficient, e.g.:
static int hmm_pfns_fill(unsigned long addr, unsigned long end,
structhmm_range*range, enumhmm_pfn_value_evalue)mm/hmm.c static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask, · line 774
Quoted 3 lines
mm/hmm.c +3
-0+static int hmm_vma_walk_test (unsigned long start, +unsigned long end, +struct mm_walk *walk)Same here.
Quoted 5 lines
mm/hmm.c static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask, · line 797
Quoted 2 lines
mm/hmm.c +2
-0+if (!(vma->vm_flags &VM_READ )) { + (void )hmm_pfns_fill (start, end, range,HMM_PFN_NONE );There should be no need for the void cast here.
OK. I'll post a v2 with the these changes.
Thanks for the reviews.
PATCH 2/4 mm/hmm: allow snapshot of the special zero page
In reply to Ralph CampbellAllow hmm_range_fault() to return success (0) when the CPU pagetable entry points to the special shared zero page.
The caller can then handle the zero page by possibly clearing device private memory instead of DMAing a zero page.
--- mm/hmm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/hmm.c +1 -1
@@ -532,7 +532,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,return -EBUSY ; }else if (IS_ENABLED (CONFIG_ARCH_HAS_PTE_SPECIAL ) &&pte_special (pte)) { *pfn = range->values[HMM_PFN_SPECIAL ]; -return -EFAULT ; +return is_zero_pfn (pte_pfn (pte)) ?0 : -EFAULT ; } *pfn =hmm_device_entry_from_pfn (range,pte_pfn (pte)) | cpu_flags;
Signature · 1 line
2.20.1
Re: [PATCH 2/4] mm/hmm: allow snapshot of the special zero page
Quoted 18 lines · Ralph Campbell
Allow
hmm_range_fault()to return success (0) when the CPU pagetable entry points to the special shared zero page.
The caller can then handle the zero page by possibly clearing device private memory instead of DMAing a zero page.Signed-off-by Ralph Campbell <[email protected]>Cc "Jérôme Glisse" <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Christoph Hellwig <[email protected]>--- mm/hmm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/hmm.c +0
-0
mm/hmm.c static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr, · line 535
Quoted 5 lines
mm/hmm.c +1
-1@@ -532,7 +532,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,return -EBUSY ; }else if (IS_ENABLED (CONFIG_ARCH_HAS_PTE_SPECIAL ) &&pte_special (pte)) { *pfn = range->values[HMM_PFN_SPECIAL ]; -return -EFAULT ; +return is_zero_pfn (pte_pfn (pte)) ?0 : -EFAULT ;
Any chance to just use a normal if here:
if (!is_zero_pfn(pte_pfn(pte))) return -EFAULT; return 0;
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH 2/4] mm/hmm: allow snapshot of the special zero page
Quoted 27 lines · Christoph Hellwig
Quoted 18 lines · Ralph Campbell
Allow
hmm_range_fault()to return success (0) when the CPU pagetable entry points to the special shared zero page.
The caller can then handle the zero page by possibly clearing device private memory instead of DMAing a zero page.Signed-off-by Ralph Campbell <[email protected]>Cc "Jérôme Glisse" <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc Christoph Hellwig <[email protected]>--- mm/hmm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/hmm.c +0
-0mm/hmm.c static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr, · line 535
Quoted 5 lines
mm/hmm.c +1
-1@@ -532,7 +532,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,return -EBUSY ; }else if (IS_ENABLED (CONFIG_ARCH_HAS_PTE_SPECIAL ) &&pte_special (pte)) { *pfn = range->values[HMM_PFN_SPECIAL ]; -return -EFAULT ; +return is_zero_pfn (pte_pfn (pte)) ?0 : -EFAULT ;Any chance to just use a normal if here:
Quoted 4 lines
if (!is_zero_pfn(pte_pfn(pte))) return -EFAULT; return 0;
Sure, no problem.
PATCH 3/4 mm/hmm: allow hmm_range_fault() of mmap(PROT_NONE)
In reply to Ralph CampbellAllow hmm_range_fault() to return success (0) when the range has no access (!(vma->vm_flags & VM_READ)). The range->pfns[] array will be filled with range->values[HMM_PFN_NONE] in this case.
This allows the caller to get a snapshot of a range without having to lookup the vma before calling hmm_range_fault().
If the call to hmm_range_fault() is not a snapshot, the caller can still check that pfns have the desired access permissions.
--- mm/hmm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
mm/hmm.c +3 -1
@@ -795,7 +795,9 @@ static int hmm_vma_walk_test(unsigned long start,*/ if (!(vma->vm_flags &VM_READ )) { (void )hmm_pfns_fill (start, end, range,HMM_PFN_NONE ); -return -EPERM ; + +/* Skip this vma and continue processing the next vma. */ +return 1 ; }return 0 ;
Signature · 1 line
2.20.1
PATCH 4/4 mm/hmm/test: add self tests for HMM
In reply to Ralph CampbellAdd self tests for HMM.
--- MAINTAINERS | 3 + drivers/char/Kconfig | 11 + drivers/char/Makefile | 1 + drivers/char/hmm_dmirror.c | 1504 ++++++++++++++++++++++++ include/Kbuild | 1 + include/uapi/linux/hmm_dmirror.h | 74 ++ tools/testing/selftests/vm/.gitignore | 1 + tools/testing/selftests/vm/Makefile | 3 + tools/testing/selftests/vm/config | 3 + tools/testing/selftests/vm/hmm-tests.c | 1304 ++++++++++++++++++++ tools/testing/selftests/vm/run_vmtests | 16 + tools/testing/selftests/vm/test_hmm.sh | 105 ++ 12 files changed, 3026 insertions(+) create mode 100644 drivers/char/hmm_dmirror.c create mode 100644 include/uapi/linux/hmm_dmirror.h create mode 100644 tools/testing/selftests/vm/hmm-tests.c create mode 100755 tools/testing/selftests/vm/test_hmm.sh
MAINTAINERS +3 -0
@@ -7318,8 +7318,11 @@ M: Jérôme Glisse <[email protected]> L: [email protected] S: Maintained F: mm/hmm* +F: drivers/char/hmm* F: include/linux/hmm* +F: include/uapi/linux/hmm* F: Documentation/vm/hmm.rst +F: tools/testing/selftests/vm/*hmm* HOST AP DRIVER M: Jouni Malinen <[email protected]>
drivers/char/Kconfig +11 -0
@@ -557,6 +557,17 @@ config ADI and SSM (Silicon Secured Memory). Intended consumers of this driver include crash and makedumpfile. +config HMM_DMIRROR + tristate "HMM driver for testing Heterogeneous Memory Management" + depends on HMM_MIRROR + depends on DEVICE_PRIVATE + help + This is a pseudo device driver solely for testing HMM. + Say Y here if you want to build the HMM test driver. + Doing so will allow you to run tools/testing/selftest/vm/hmm-tests. + + If in doubt, say "N". + endmenu config RANDOM_TRUST_CPU
drivers/char/Makefile +1 -0
@@ -54,3 +54,4 @@ js-rtc-y = rtc.o obj-$(CONFIG_XILLYBUS) += xillybus/ obj-$(CONFIG_POWERNV_OP_PANEL) += powernv-op-panel.o obj-$(CONFIG_ADI) += adi.o +obj-$(CONFIG_HMM_DMIRROR) += hmm_dmirror.o
drivers/char/hmm_dmirror.c new file +1504 -0
1,504 lines not on this page · raw message
include/Kbuild +1 -0
@@ -1134,6 +1134,7 @@ header-test- += uapi/linux/coda_psdev.h header-test- += uapi/linux/errqueue.h header-test- += uapi/linux/eventpoll.h header-test- += uapi/linux/hdlc/ioctl.h +header-test- += uapi/linux/hmm_dmirror.h header-test- += uapi/linux/input.h header-test- += uapi/linux/kvm.h header-test- += uapi/linux/kvm_para.h
include/uapi/linux/hmm_dmirror.h new file +74 -0
74 lines not on this page · raw message
tools/testing/selftests/vm/.gitignore +1 -0
@@ -14,3 +14,4 @@ virtual_address_range gup_benchmark va_128TBswitch map_fixed_noreplace +hmm-tests
tools/testing/selftests/vm/Makefile +3 -0
@@ -5,6 +5,7 @@ CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS) LDLIBS = -lrt TEST_GEN_FILES = compaction_test TEST_GEN_FILES += gup_benchmark +TEST_GEN_FILES += hmm-tests TEST_GEN_FILES += hugepage-mmap TEST_GEN_FILES += hugepage-shm TEST_GEN_FILES += map_hugetlb
@@ -26,6 +27,8 @@ TEST_FILES := test_vmalloc.sh KSFT_KHDR_INSTALL := 1 include ../lib.mk +$(OUTPUT)/hmm-tests: LDLIBS += -lhugetlbfs -lpthread + $(OUTPUT)/userfaultfd: LDLIBS += -lpthread $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
tools/testing/selftests/vm/config +3 -0
@@ -1,2 +1,5 @@ CONFIG_SYSVIPC=y CONFIG_USERFAULTFD=y +CONFIG_HMM_MIRROR=y +CONFIG_DEVICE_PRIVATE=y +CONFIG_HMM_DMIRROR=m
tools/testing/selftests/vm/hmm-tests.c new file +1304 -0
1,304 lines not on this page · raw message
tools/testing/selftests/vm/run_vmtests +16 -0
@@ -227,4 +227,20 @@ else exitcode=1 fi +echo "------------------------------------" +echo "running HMM smoke test" +echo "------------------------------------" +./test_hmm.sh smoke +ret_val=$? + +if [ $ret_val -eq 0 ]; then + echo "[PASS]" +elif [ $ret_val -eq $ksft_skip ]; then + echo "[SKIP]" + exitcode=$ksft_skip +else + echo "[FAIL]" + exitcode=1 +fi + exit $exitcode
tools/testing/selftests/vm/test_hmm.sh new file +105 -0
35 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.20.1
Part of the diff is not shown here. View the raw message.