On 05/24/2017 10:20 AM, Jérôme Glisse wrote:
[...]
+/*
+ * hmm_devmem_fault_range() - migrate back a virtual range of memory
+ *
+ * @devmem: hmm_devmem struct use to track and manage the ZONE_DEVICE memory
+ * @vma: virtual memory area containing the range to be migrated
+ * @ops: migration callback for allocating destination memory and copying
+ * @src: array of unsigned long containing source pfns
+ * @dst: array of unsigned long containing destination pfns
+ * @start: start address of the range to migrate (inclusive)
+ * @addr: fault address (must be inside the range)
+ * @end: end address of the range to migrate (exclusive)
+ * @private: pointer passed back to each of the callback
+ * Returns: 0 on success, VM_FAULT_SIGBUS on error
+ *
+ * This is a wrapper around migrate_vma() which checks the migration status
+ * for a given fault address and returns the corresponding page fault handler
+ * status. That will be 0 on success, or VM_FAULT_SIGBUS if migration failed
+ * for the faulting address.
+ *
+ * This is a helper intendend to be used by the ZONE_DEVICE fault handler.
+ */
+int hmm_devmem_fault_range(struct hmm_devmem *devmem,
+                          struct vm_area_struct *vma,
+                          const struct migrate_vma_ops *ops,
+                          unsigned long *src,
+                          unsigned long *dst,
+                          unsigned long start,
+                          unsigned long addr,
+                          unsigned long end,
+                          void *private)
+{
+       if (migrate_vma(ops, vma, start, end, src, dst, private))
+               return VM_FAULT_SIGBUS;
+
+       if (dst[(addr - start) >> PAGE_SHIFT] & MIGRATE_PFN_ERROR)
+               return VM_FAULT_SIGBUS;
+
+       return 0;
+}
+EXPORT_SYMBOL(hmm_devmem_fault_range);
+#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */


Hi Jerome (+Evgeny),

After some time and testing, I'd like to recommend that we delete the above hmm_dev_fault_range() function from the patchset. Reasons:

1. Our driver code is actually easier to follow if we call migrate_vma() directly, for CPU faults. That's because there are a lot of hmm_* calls in both directions (driver <--> core), already, and it takes some time to remember which direction each one goes.

2. The helper is a little confusing to use, what with a start, end, *and* an 
addr argument.

3. ...and it doesn't add anything that the driver can't trivially do itself.

So, let's just remove it. Less is more this time. :)

thanks,
--
John Hubbard
NVIDIA

Reply via email to