search for: hmm_vma_walk_hole

I have been working on converting Jerome's hmm_dummy driver and self tests into a stand-alone set of tests to be included in tools/testing/selftests/vm and came across these two bug fixes in the process. The tests aren't quite ready to be posted as a patch. I'm posting the fixes now since I thought they shouldn't wait. They should probably have a fixes line but with all the HMM

On Fri, Aug 23, 2019 at 03:17:52PM -0700, Ralph Campbell wrote: > Although hmm_range_fault() calls find_vma() to make sure that a vma exists > before calling walk_page_range(), hmm_vma_walk_hole() can still be called > with walk->vma == NULL if the start and end address are not contained > within the vma range. Should we convert to walk_vma_range instead? Or keep walk_page_range but drop searching the vma ourselves? Except for that the patch looks good to me: Reviewed-by: Chri...

Although hmm_range_fault() calls find_vma() to make sure that a vma exists before calling walk_page_range(), hmm_vma_walk_hole() can still be called with walk->vma == NULL if the start and end address are not contained within the vma range. hmm_range_fault() /* calls find_vma() but no range check */ walk_page_range() /* calls find_vma(), sets walk->vma = NULL */ __walk_page_range() walk_pgd_range() wa...

...M -0700, Ralph Campbell wrote: > > On 8/24/19 3:37 PM, Christoph Hellwig wrote: > > On Fri, Aug 23, 2019 at 03:17:52PM -0700, Ralph Campbell wrote: > > > Although hmm_range_fault() calls find_vma() to make sure that a vma exists > > > before calling walk_page_range(), hmm_vma_walk_hole() can still be called > > > with walk->vma == NULL if the start and end address are not contained > > > within the vma range. > > > > Should we convert to walk_vma_range instead? Or keep walk_page_range > > but drop searching the vma ourselves? > > &g...

...gt;page_shift && - range->page_shift != PAGE_SHIFT) - return -EINVAL; - } else { - if (range->page_shift != PAGE_SHIFT) - return -EINVAL; - } + mm_walk.mm = hmm->mm; + mm_walk.pud_entry = hmm_vma_walk_pud; + mm_walk.pmd_entry = hmm_vma_walk_pmd; + mm_walk.pte_hole = hmm_vma_walk_hole; + mm_walk.hugetlb_entry = hmm_vma_walk_hugetlb_entry; + mm_walk.test_walk = hmm_vma_walk_test; - 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...

Here are seven more patches for things I found to clean up. This was based on top of Christoph's seven patches: "hmm_range_fault related fixes and legacy API removal v3". I assume this will go into Jason's tree since there will likely be more HMM changes in this cycle. Changes from v1 to v2: Added AMD GPU to hmm_update removal. Added 2 patches from Christoph. Added 2 patches as

...m> Cc: Christoph Hellwig <hch at lst.de> --- mm/hmm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 838cd1d50497..29f322ca5d58 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -630,9 +630,6 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp, if (pmd_none(pmd)) return hmm_vma_walk_hole(start, end, walk); - if (pmd_huge(pmd) && (range->vma->vm_flags & VM_HUGETLB)) - return hmm_pfns_bad(start, end, walk); - if (thp_migration_supported() && is_pmd_migration_entry(pmd)) { bool fault, write_fault; unsigned long npages; -- 2.20.1

...mm_range_fault() checks for VM_READ but doesn't check for VM_WRITE if the caller requests a page to be faulted in with write permission (via the hmm_range.pfns[] value). If the vma is write protected, this can result in an infinite loop: hmm_range_fault() walk_page_range() ... hmm_vma_walk_hole() hmm_vma_walk_hole_() hmm_vma_do_fault() handle_mm_fault(FAULT_FLAG_WRITE) /* returns VM_FAULT_WRITE */ /* returns -EBUSY */ /* returns -EBUSY */ /* returns -EBUSY */ /* loops on -EBUSY and range->valid */ Prevent this by che...

On 8/24/19 3:37 PM, Christoph Hellwig wrote: > On Fri, Aug 23, 2019 at 03:17:52PM -0700, Ralph Campbell wrote: >> Although hmm_range_fault() calls find_vma() to make sure that a vma exists >> before calling walk_page_range(), hmm_vma_walk_hole() can still be called >> with walk->vma == NULL if the start and end address are not contained >> within the vma range. > > Should we convert to walk_vma_range instead? Or keep walk_page_range > but drop searching the vma ourselves? > > Except for that the patch loo...

...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_MI...

...i < npages; ++i) { - required_fault |= - hmm_pte_need_fault(hmm_vma_walk, pfns[i], cpu_flags); + required_fault |= hmm_pte_need_fault(hmm_vma_walk, hmm_pfns[i], + cpu_flags); if (required_fault == HMM_NEED_ALL_BITS) return required_fault; } @@ -161,12 +148,13 @@ static int hmm_vma_walk_hole(unsigned long addr, unsigned long end, struct hmm_range *range = hmm_vma_walk->range; unsigned int required_fault; unsigned long i, npages; - uint64_t *pfns; + unsigned long *hmm_pfns; i = (addr - range->start) >> PAGE_SHIFT; npages = (end - addr) >> PAGE_SHIFT; - pfns...

...i < npages; ++i) { - required_fault |= - hmm_pte_need_fault(hmm_vma_walk, pfns[i], cpu_flags); + required_fault |= hmm_pte_need_fault(hmm_vma_walk, hmm_pfns[i], + cpu_flags); if (required_fault == HMM_NEED_ALL_BITS) return required_fault; } @@ -161,12 +148,13 @@ static int hmm_vma_walk_hole(unsigned long addr, unsigned long end, struct hmm_range *range = hmm_vma_walk->range; unsigned int required_fault; unsigned long i, npages; - uint64_t *pfns; + unsigned long *hmm_pfns; i = (addr - range->start) >> PAGE_SHIFT; npages = (end - addr) >> PAGE_SHIFT; - pfns...

...ault |= > - hmm_pte_need_fault(hmm_vma_walk, pfns[i], cpu_flags); > + required_fault |= hmm_pte_need_fault(hmm_vma_walk, hmm_pfns[i], > + cpu_flags); > if (required_fault == HMM_NEED_ALL_BITS) > return required_fault; > } > @@ -161,12 +148,13 @@ static int hmm_vma_walk_hole(unsigned long addr, unsigned long end, > struct hmm_range *range = hmm_vma_walk->range; > unsigned int required_fault; > unsigned long i, npages; > - uint64_t *pfns; > + unsigned long *hmm_pfns; > > i = (addr - range->start) >> PAGE_SHIFT; > npages =...

...cing the existing + * Return: an HMM object, either by referencing the existing * (per-process) object, or by creating a new one. * * This is not intended to be used directly by device drivers. If mm already @@ -325,8 +325,8 @@ static int hmm_pfns_bad(unsigned long addr, } /* - * hmm_vma_walk_hole() - handle a range lacking valid pmd or pte(s) - * @start: range virtual start address (inclusive) + * hmm_vma_walk_hole_() - handle a range lacking valid pmd or pte(s) + * @addr: range virtual start address (inclusive) * @end: range virtual end address (exclusive) * @fault: should we fault or...

From: Jason Gunthorpe <jgg at mellanox.com> The API is a bit complicated for the uses we actually have, and disucssions for simplifying have come up a number of times. This small series removes the customizable pfn format and simplifies the return code of hmm_range_fault() All the drivers are adjusted to process in the simplified format. I would appreciated tested-by's for the two

...ault(struct hmm_range *range); +int hmm_range_fault(struct hmm_range *range); /* * HMM_RANGE_DEFAULT_TIMEOUT - default timeout (ms) when waiting for a range diff --git a/mm/hmm.c b/mm/hmm.c index 280585833adfc1..4c7c396655b528 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -174,7 +174,6 @@ static int hmm_vma_walk_hole(unsigned long addr, unsigned long end, } if (required_fault) return hmm_vma_fault(addr, end, required_fault, walk); - hmm_vma_walk->last = addr; return hmm_pfns_fill(addr, end, range, HMM_PFN_NONE); } @@ -207,7 +206,6 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk, unsigned lo...

...ault(struct hmm_range *range); +int hmm_range_fault(struct hmm_range *range); /* * HMM_RANGE_DEFAULT_TIMEOUT - default timeout (ms) when waiting for a range diff --git a/mm/hmm.c b/mm/hmm.c index 280585833adfc1..f06bcac948a79b 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -174,7 +174,6 @@ static int hmm_vma_walk_hole(unsigned long addr, unsigned long end, } if (required_fault) return hmm_vma_fault(addr, end, required_fault, walk); - hmm_vma_walk->last = addr; return hmm_pfns_fill(addr, end, range, HMM_PFN_NONE); } @@ -207,7 +206,6 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk, unsigned lo...

From: Jason Gunthorpe <jgg at mellanox.com> The API is a bit complicated for the uses we actually have, and disucssions for simplifying have come up a number of times. This small series removes the customizable pfn format and simplifies the return code of hmm_range_fault() All the drivers are adjusted to process in the simplified format. I would appreciated tested-by's for the two

...{ Let's add: /* Output flags: */ > + HMM_PFN_VALID = 1UL << (BITS_PER_LONG - 1), > + HMM_PFN_WRITE = 1UL << (BITS_PER_LONG - 2), > + HMM_PFN_ERROR = 1UL << (BITS_PER_LONG - 3), > + /* Input flags: */ ... > @@ -174,44 +162,44 @@ static int hmm_vma_walk_hole(unsigned long addr, unsigned long end, > } > if (required_fault) > return hmm_vma_fault(addr, end, required_fault, walk); > - return hmm_pfns_fill(addr, end, range, HMM_PFN_NONE); > + return hmm_pfns_fill(addr, end, range, 0); > } > > -static inline uint64_t p...

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/20190726005650.2566-6-rcampbell at nvidia.com/ Ralph Campbell