Re: [PATCH hmm 4/6] mm/hmm: remove HMM_FAULT_SNAPSHOT

2020-03-21 Thread Christoph Hellwig
On Fri, Mar 20, 2020 at 01:49:03PM -0300, Jason Gunthorpe wrote:
> + struct hmm_range *range = hmm_vma_walk->range;
>   unsigned int required_fault = 0;
>   unsigned long i;
>  
> - if (hmm_vma_walk->flags & HMM_FAULT_SNAPSHOT)
> + /*
> +  * If there is no way for valid to be set in hmm_pte_need_fault() then
> +  * don't bother to call it.
> +  */
> + if (!(((range->flags[HMM_PFN_VALID] & range->pfn_flags_mask) |
> +range->default_flags) &
> +   range->flags[HMM_PFN_VALID]))

I think this needs to be split out into a well documented helper
function, as it is pretty much unreadable all cramed into a complex
conditional.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH hmm 4/6] mm/hmm: remove HMM_FAULT_SNAPSHOT

2020-03-20 Thread Jason Gunthorpe
From: Jason Gunthorpe 

Now that flags are handled on a fine-grained per-page basis this global
flag is redundant and has a confusing overlap with the pfn_flags_mask and
default_flags.

Normalize the HMM_FAULT_SNAPSHOT behavior into one place. Callers needing
the SNAPSHOT behavior should set a pfn_flags_mask and default_flags that
always results in a cleared HMM_PFN_REQ_FAULT. Then no pages will be
faulted, and HMM_FAULT_SNAPSHOT is not a special flow that overrides the
masking mechanism.

As this is the last flag, also remove the flags argument. If future flags
are needed they can be part of the struct hmm_range function arguments.

Signed-off-by: Jason Gunthorpe 
---
 Documentation/vm/hmm.rst| 12 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_svm.c   |  2 +-
 include/linux/hmm.h |  5 +
 mm/hmm.c| 17 +
 5 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/Documentation/vm/hmm.rst b/Documentation/vm/hmm.rst
index 95fec596836262..4e3e9362afeb10 100644
--- a/Documentation/vm/hmm.rst
+++ b/Documentation/vm/hmm.rst
@@ -161,13 +161,11 @@ device must complete the update before the driver 
callback returns.
 When the device driver wants to populate a range of virtual addresses, it can
 use::
 
-  long hmm_range_fault(struct hmm_range *range, unsigned int flags);
+  long hmm_range_fault(struct hmm_range *range);
 
-With the HMM_RANGE_SNAPSHOT flag, it will only fetch present CPU page table
-entries and will not trigger a page fault on missing or non-present entries.
-Without that flag, it does trigger a page fault on missing or read-only entries
-if write access is requested (see below). Page faults use the generic mm page
-fault code path just like a CPU page fault.
+It will trigger a page fault on missing or read-only entries if write access is
+requested (see below). Page faults use the generic mm page fault code path just
+like a CPU page fault.
 
 Both functions copy CPU page table entries into their pfns array argument. Each
 entry in that array corresponds to an address in the virtual range. HMM
@@ -197,7 +195,7 @@ The usage pattern is::
  again:
   range.notifier_seq = mmu_interval_read_begin(&interval_sub);
   down_read(&mm->mmap_sem);
-  ret = hmm_range_fault(&range, HMM_RANGE_SNAPSHOT);
+  ret = hmm_range_fault(&range);
   if (ret) {
   up_read(&mm->mmap_sem);
   if (ret == -EBUSY)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 90821ce5e6cad0..c520290709371b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -856,7 +856,7 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, 
struct page **pages)
range->notifier_seq = mmu_interval_read_begin(&bo->notifier);
 
down_read(&mm->mmap_sem);
-   r = hmm_range_fault(range, 0);
+   r = hmm_range_fault(range);
up_read(&mm->mmap_sem);
if (unlikely(r <= 0)) {
/*
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 39c731a99937c6..e3797b2d4d1759 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -540,7 +540,7 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm,
range.default_flags = 0;
range.pfn_flags_mask = -1UL;
down_read(&mm->mmap_sem);
-   ret = hmm_range_fault(&range, 0);
+   ret = hmm_range_fault(&range);
up_read(&mm->mmap_sem);
if (ret <= 0) {
if (ret == 0 || ret == -EBUSY)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 184a8633260f9d..6b4004905aac89 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -167,13 +167,10 @@ static inline struct page *hmm_device_entry_to_page(const 
struct hmm_range *rang
return pfn_to_page(entry >> range->pfn_shift);
 }
 
-/* Don't fault in missing PTEs, just snapshot the current state. */
-#define HMM_FAULT_SNAPSHOT (1 << 1)
-
 /*
  * Please see Documentation/vm/hmm.rst for how to use the range API.
  */
-long hmm_range_fault(struct hmm_range *range, unsigned int flags);
+long 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 687d21c675ee60..7f77fb6e35cf78 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -29,7 +29,6 @@
 struct hmm_vma_walk {
struct hmm_range*range;
unsigned long   last;
-   unsigned intflags;
 };
 
 enum {
@@ -111,9 +110,6 @@ static unsigned int hmm_pte_need_fault(const struct 
hmm_vma_walk *hmm_vma_walk,
 {
struct hmm_range *range = hmm_vma_walk->range;
 
-   if (hmm_vma_walk->flags & HMM_FAULT_SNAPSHOT)
-   return 0;
-