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;
 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to