On Wed, Aug 14, 2019 at 09:38:54AM +0200, Christoph Hellwig wrote: > On Tue, Aug 13, 2019 at 06:36:33PM -0700, Dan Williams wrote: > > Section alignment constraints somewhat save us here. The only example > > I can think of a PMD not containing a uniform pgmap association for > > each pte is the case when the pgmap overlaps normal dram, i.e. shares > > the same 'struct memory_section' for a given span. Otherwise, distinct > > pgmaps arrange to manage their own exclusive sections (and now > > subsections as of v5.3). Otherwise the implementation could not > > guarantee different mapping lifetimes. > > > > That said, this seems to want a better mechanism to determine "pfn is > > ZONE_DEVICE". > > So I guess this patch is fine for now, and once you provide a better > mechanism we can switch over to it? What about the version I sent to just get rid of all the strange put_dev_pagemaps while scanning? Odds are good we will work with only a single pagemap, so it makes some sense to cache it once we find it? diff --git a/mm/hmm.c b/mm/hmm.c index 9a908902e4cc38..4e30128c23a505 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -497,10 +497,6 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk, } pfns[i] = hmm_device_entry_from_pfn(range, pfn) | cpu_flags; } - if (hmm_vma_walk->pgmap) { - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } hmm_vma_walk->last = end; return 0; #else @@ -604,10 +600,6 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr, return 0; fault: - if (hmm_vma_walk->pgmap) { - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } pte_unmap(ptep); /* Fault any virtual address we were asked to fault */ return hmm_vma_walk_hole_(addr, end, fault, write_fault, walk); @@ -690,16 +682,6 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp, return r; } } - if (hmm_vma_walk->pgmap) { - /* - * We do put_dev_pagemap() here and not in hmm_vma_handle_pte() - * so that we can leverage get_dev_pagemap() optimization which - * will not re-take a reference on a pgmap if we already have - * one. - */ - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } pte_unmap(ptep - 1); hmm_vma_walk->last = addr; @@ -751,10 +733,6 @@ static int hmm_vma_walk_pud(pud_t *pudp, pfns[i] = hmm_device_entry_from_pfn(range, pfn) | cpu_flags; } - if (hmm_vma_walk->pgmap) { - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } hmm_vma_walk->last = end; return 0; } @@ -1026,6 +1004,14 @@ long hmm_range_fault(struct hmm_range *range, unsigned int flags) /* Keep trying while the range is valid. */ } while (ret == -EBUSY && range->valid); + /* + * We do put_dev_pagemap() here so that we can leverage + * get_dev_pagemap() optimization which will not re-take a + * reference on a pgmap if we already have one. + */ + if (hmm_vma_walk->pgmap) + put_dev_pagemap(hmm_vma_walk->pgmap); + if (ret) { unsigned long i; _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel