Re: [PATCH v5 1/2] mm, dax: make pmd_fault() and friends to be the same as fault()

[Dave Jiang]

On 12/19/2016 11:27 AM, Dave Jiang wrote:
> Instead of passing in multiple parameters in the pmd_fault() handler,
> a vmf can be passed in just like a fault() handler. This will simplify
> code and remove the need for the actual pmd fault handlers to allocate a
> vmf. Related functions are also modified to do the same.
> 
> Signed-off-by: Dave Jiang 
> Reviewed-by: Ross Zwisler 
> Reviewed-by: Jan Kara 

Andrew,
I haven't seen any more comments on these 2 patches. Will you pick them
up for the mm tree? Thanks!

> ---
>  drivers/dax/dax.c |   16 +++-
>  fs/dax.c  |   42 
> ++---
>  fs/ext4/file.c|9 -
>  fs/xfs/xfs_file.c |   10 --
>  include/linux/dax.h   |7 +++
>  include/linux/mm.h|3 +--
>  include/trace/events/fs_dax.h |   15 +++
>  mm/memory.c   |6 ++
>  8 files changed, 46 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
> index c753a4c..947e49a 100644
> --- a/drivers/dax/dax.c
> +++ b/drivers/dax/dax.c
> @@ -379,10 +379,9 @@ static int dax_dev_fault(struct vm_area_struct *vma, 
> struct vm_fault *vmf)
>  }
>  
>  static int __dax_dev_pmd_fault(struct dax_dev *dax_dev,
> - struct vm_area_struct *vma, unsigned long addr, pmd_t *pmd,
> - unsigned int flags)
> + struct vm_area_struct *vma, struct vm_fault *vmf)
>  {
> - unsigned long pmd_addr = addr & PMD_MASK;
> + unsigned long pmd_addr = vmf->address & PMD_MASK;
>   struct device *dev = _dev->dev;
>   struct dax_region *dax_region;
>   phys_addr_t phys;
> @@ -414,23 +413,22 @@ static int __dax_dev_pmd_fault(struct dax_dev *dax_dev,
>  
>   pfn = phys_to_pfn_t(phys, dax_region->pfn_flags);
>  
> - return vmf_insert_pfn_pmd(vma, addr, pmd, pfn,
> - flags & FAULT_FLAG_WRITE);
> + return vmf_insert_pfn_pmd(vma, vmf->address, vmf->pmd, pfn,
> + vmf->flags & FAULT_FLAG_WRITE);
>  }
>  
> -static int dax_dev_pmd_fault(struct vm_area_struct *vma, unsigned long addr,
> - pmd_t *pmd, unsigned int flags)
> +static int dax_dev_pmd_fault(struct vm_area_struct *vma, struct vm_fault 
> *vmf)
>  {
>   int rc;
>   struct file *filp = vma->vm_file;
>   struct dax_dev *dax_dev = filp->private_data;
>  
>   dev_dbg(_dev->dev, "%s: %s: %s (%#lx - %#lx)\n", __func__,
> - current->comm, (flags & FAULT_FLAG_WRITE)
> + current->comm, (vmf->flags & FAULT_FLAG_WRITE)
>   ? "write" : "read", vma->vm_start, vma->vm_end);
>  
>   rcu_read_lock();
> - rc = __dax_dev_pmd_fault(dax_dev, vma, addr, pmd, flags);
> + rc = __dax_dev_pmd_fault(dax_dev, vma, vmf);
>   rcu_read_unlock();
>  
>   return rc;
> diff --git a/fs/dax.c b/fs/dax.c
> index d3fe880..446e861 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1310,18 +1310,17 @@ static int dax_pmd_load_hole(struct vm_area_struct 
> *vma, pmd_t *pmd,
>   return VM_FAULT_FALLBACK;
>  }
>  
> -int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address,
> - pmd_t *pmd, unsigned int flags, struct iomap_ops *ops)
> +int dax_iomap_pmd_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
> + struct iomap_ops *ops)
>  {
>   struct address_space *mapping = vma->vm_file->f_mapping;
> - unsigned long pmd_addr = address & PMD_MASK;
> - bool write = flags & FAULT_FLAG_WRITE;
> + unsigned long pmd_addr = vmf->address & PMD_MASK;
> + bool write = vmf->flags & FAULT_FLAG_WRITE;
>   unsigned int iomap_flags = (write ? IOMAP_WRITE : 0) | IOMAP_FAULT;
>   struct inode *inode = mapping->host;
>   int result = VM_FAULT_FALLBACK;
>   struct iomap iomap = { 0 };
> - pgoff_t max_pgoff, pgoff;
> - struct vm_fault vmf;
> + pgoff_t max_pgoff;
>   void *entry;
>   loff_t pos;
>   int error;
> @@ -1331,10 +1330,10 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, 
> unsigned long address,
>* supposed to hold locks serializing us with truncate / punch hole so
>* this is a reliable test.
>*/
> - pgoff = linear_page_index(vma, pmd_addr);
> + vmf->pgoff = linear_page_index(vma, pmd_addr);
>   max_pgoff = (i_size_read(inode) - 1) >> PAGE_SHIFT;
>  
> - trace_dax_pmd_fault(inode, vma, address, flags, pgoff, max_pgoff, 0);
> + trace_dax_pmd_fault(inode, vma, vmf, max_pgoff, 0);
>  
>   /* Fall back to PTEs if we're going to COW */
>   if (write && !(vma->vm_flags & VM_SHARED))
> @@ -1346,13 +1345,13 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, 
> unsigned long address,
>   if ((pmd_addr + PMD_SIZE) > vma->vm_end)
>   goto fallback;
>  
> - if (pgoff > max_pgoff) {
> + if 

[PATCH v5 1/2] mm, dax: make pmd_fault() and friends to be the same as fault()

[Dave Jiang]

Instead of passing in multiple parameters in the pmd_fault() handler,
a vmf can be passed in just like a fault() handler. This will simplify
code and remove the need for the actual pmd fault handlers to allocate a
vmf. Related functions are also modified to do the same.

Signed-off-by: Dave Jiang 
Reviewed-by: Ross Zwisler 
Reviewed-by: Jan Kara 
---
 drivers/dax/dax.c |   16 +++-
 fs/dax.c  |   42 ++---
 fs/ext4/file.c|9 -
 fs/xfs/xfs_file.c |   10 --
 include/linux/dax.h   |7 +++
 include/linux/mm.h|3 +--
 include/trace/events/fs_dax.h |   15 +++
 mm/memory.c   |6 ++
 8 files changed, 46 insertions(+), 62 deletions(-)

diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
index c753a4c..947e49a 100644
--- a/drivers/dax/dax.c
+++ b/drivers/dax/dax.c
@@ -379,10 +379,9 @@ static int dax_dev_fault(struct vm_area_struct *vma, 
struct vm_fault *vmf)
 }
 
 static int __dax_dev_pmd_fault(struct dax_dev *dax_dev,
-   struct vm_area_struct *vma, unsigned long addr, pmd_t *pmd,
-   unsigned int flags)
+   struct vm_area_struct *vma, struct vm_fault *vmf)
 {
-   unsigned long pmd_addr = addr & PMD_MASK;
+   unsigned long pmd_addr = vmf->address & PMD_MASK;
struct device *dev = _dev->dev;
struct dax_region *dax_region;
phys_addr_t phys;
@@ -414,23 +413,22 @@ static int __dax_dev_pmd_fault(struct dax_dev *dax_dev,
 
pfn = phys_to_pfn_t(phys, dax_region->pfn_flags);
 
-   return vmf_insert_pfn_pmd(vma, addr, pmd, pfn,
-   flags & FAULT_FLAG_WRITE);
+   return vmf_insert_pfn_pmd(vma, vmf->address, vmf->pmd, pfn,
+   vmf->flags & FAULT_FLAG_WRITE);
 }
 
-static int dax_dev_pmd_fault(struct vm_area_struct *vma, unsigned long addr,
-   pmd_t *pmd, unsigned int flags)
+static int dax_dev_pmd_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
int rc;
struct file *filp = vma->vm_file;
struct dax_dev *dax_dev = filp->private_data;
 
dev_dbg(_dev->dev, "%s: %s: %s (%#lx - %#lx)\n", __func__,
-   current->comm, (flags & FAULT_FLAG_WRITE)
+   current->comm, (vmf->flags & FAULT_FLAG_WRITE)
? "write" : "read", vma->vm_start, vma->vm_end);
 
rcu_read_lock();
-   rc = __dax_dev_pmd_fault(dax_dev, vma, addr, pmd, flags);
+   rc = __dax_dev_pmd_fault(dax_dev, vma, vmf);
rcu_read_unlock();
 
return rc;
diff --git a/fs/dax.c b/fs/dax.c
index d3fe880..446e861 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1310,18 +1310,17 @@ static int dax_pmd_load_hole(struct vm_area_struct 
*vma, pmd_t *pmd,
return VM_FAULT_FALLBACK;
 }
 
-int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address,
-   pmd_t *pmd, unsigned int flags, struct iomap_ops *ops)
+int dax_iomap_pmd_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
+   struct iomap_ops *ops)
 {
struct address_space *mapping = vma->vm_file->f_mapping;
-   unsigned long pmd_addr = address & PMD_MASK;
-   bool write = flags & FAULT_FLAG_WRITE;
+   unsigned long pmd_addr = vmf->address & PMD_MASK;
+   bool write = vmf->flags & FAULT_FLAG_WRITE;
unsigned int iomap_flags = (write ? IOMAP_WRITE : 0) | IOMAP_FAULT;
struct inode *inode = mapping->host;
int result = VM_FAULT_FALLBACK;
struct iomap iomap = { 0 };
-   pgoff_t max_pgoff, pgoff;
-   struct vm_fault vmf;
+   pgoff_t max_pgoff;
void *entry;
loff_t pos;
int error;
@@ -1331,10 +1330,10 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, 
unsigned long address,
 * supposed to hold locks serializing us with truncate / punch hole so
 * this is a reliable test.
 */
-   pgoff = linear_page_index(vma, pmd_addr);
+   vmf->pgoff = linear_page_index(vma, pmd_addr);
max_pgoff = (i_size_read(inode) - 1) >> PAGE_SHIFT;
 
-   trace_dax_pmd_fault(inode, vma, address, flags, pgoff, max_pgoff, 0);
+   trace_dax_pmd_fault(inode, vma, vmf, max_pgoff, 0);
 
/* Fall back to PTEs if we're going to COW */
if (write && !(vma->vm_flags & VM_SHARED))
@@ -1346,13 +1345,13 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, 
unsigned long address,
if ((pmd_addr + PMD_SIZE) > vma->vm_end)
goto fallback;
 
-   if (pgoff > max_pgoff) {
+   if (vmf->pgoff > max_pgoff) {
result = VM_FAULT_SIGBUS;
goto out;
}
 
/* If the PMD would extend beyond the file size */
-   if ((pgoff | PG_PMD_COLOUR) > max_pgoff)
+   if ((vmf->pgoff | PG_PMD_COLOUR) > max_pgoff)