[PATCH] fs: dax: Adding new return type vm_fault_t

[PATCH] fs: dax: Adding new return type vm_fault_t

[Souptick Joarder]

Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.

Reference id -> 1c8f422059ae ("mm: change return type to
vm_fault_t")

There was an existing bug inside dax_load_hole()
if vm_insert_mixed had failed to allocate a page table,
we'd return VM_FAULT_NOPAGE instead of VM_FAULT_OOM.
With vmf_insert_mixed this issue is addressed.

vmf_insert_mixed_mkwrite() is the new wrapper function
which will convert err returned from vm_insert_mixed_
mkwrite() to vm_fault_t type.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
---
 fs/dax.c            | 56 +++++++++++++++++++++++++++++++----------------------
 include/linux/dax.h |  4 ++--
 2 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 0276df9..d7112ad 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -869,12 +869,12 @@ static int dax_iomap_pfn(struct iomap *iomap, loff_t pos, size_t size,
  * If this page is ever written to we will re-fault and change the mapping to
  * point to real DAX storage instead.
  */
-static int dax_load_hole(struct address_space *mapping, void *entry,
+static vm_fault_t dax_load_hole(struct address_space *mapping, void *entry,
 			 struct vm_fault *vmf)
 {
 	struct inode *inode = mapping->host;
 	unsigned long vaddr = vmf->address;
-	int ret = VM_FAULT_NOPAGE;
+	vm_fault_t ret = VM_FAULT_NOPAGE;
 	struct page *zero_page;
 	void *entry2;
 
@@ -891,7 +891,7 @@ static int dax_load_hole(struct address_space *mapping, void *entry,
 		goto out;
 	}
 
-	vm_insert_mixed(vmf->vma, vaddr, page_to_pfn_t(zero_page));
+	ret = vmf_insert_mixed(vmf->vma, vaddr, page_to_pfn_t(zero_page));
 out:
 	trace_dax_load_hole(inode, vmf, ret);
 	return ret;
@@ -1074,7 +1074,7 @@ int __dax_zero_page_range(struct block_device *bdev,
 }
 EXPORT_SYMBOL_GPL(dax_iomap_rw);
 
-static int dax_fault_return(int error)
+static vm_fault_t dax_fault_return(int error)
 {
 	if (error == 0)
 		return VM_FAULT_NOPAGE;
@@ -1094,6 +1094,18 @@ static bool dax_fault_is_synchronous(unsigned long flags,
 		&& (iomap->flags & IOMAP_F_DIRTY);
 }
 
+static vm_fault_t vmf_insert_mixed_mkwrite(struct vm_area_struct *vma,
+			unsigned long addr, pfn_t pfn)
+{
+	int error;
+	vm_fault_t vmf_ret;
+
+	error = vm_insert_mixed_mkwrite(vma, addr, pfn);
+	vmf_ret = dax_fault_return(error);
+
+	return vmf_ret;
+}
+
 static int dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
 			       int *iomap_errp, const struct iomap_ops *ops)
 {
@@ -1107,7 +1119,7 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
 	int error, major = 0;
 	bool write = vmf->flags & FAULT_FLAG_WRITE;
 	bool sync;
-	int vmf_ret = 0;
+	vm_fault_t vmf_ret = 0;
 	void *entry;
 	pfn_t pfn;
 
@@ -1225,14 +1237,12 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
 		}
 		trace_dax_insert_mapping(inode, vmf, entry);
 		if (write)
-			error = vm_insert_mixed_mkwrite(vma, vaddr, pfn);
+			vmf_ret = vmf_insert_mixed_mkwrite(vma, vaddr, pfn);
 		else
-			error = vm_insert_mixed(vma, vaddr, pfn);
+			vmf_ret = vmf_insert_mixed(vma, vaddr, pfn);
+
+		goto finish_iomap;
 
-		/* -EBUSY is fine, somebody else faulted on the same PTE */
-		if (error == -EBUSY)
-			error = 0;
-		break;
 	case IOMAP_UNWRITTEN:
 	case IOMAP_HOLE:
 		if (!write) {
@@ -1270,7 +1280,7 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
 }
 
 #ifdef CONFIG_FS_DAX_PMD
-static int dax_pmd_load_hole(struct vm_fault *vmf, struct iomap *iomap,
+static vm_fault_t dax_pmd_load_hole(struct vm_fault *vmf, struct iomap *iomap,
 		void *entry)
 {
 	struct address_space *mapping = vmf->vma->vm_file->f_mapping;
@@ -1309,7 +1319,7 @@ static int dax_pmd_load_hole(struct vm_fault *vmf, struct iomap *iomap,
 	return VM_FAULT_FALLBACK;
 }
 
-static int dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
+static vm_fault_t dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
 			       const struct iomap_ops *ops)
 {
 	struct vm_area_struct *vma = vmf->vma;
@@ -1319,7 +1329,7 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
 	bool sync;
 	unsigned int iomap_flags = (write ? IOMAP_WRITE : 0) | IOMAP_FAULT;
 	struct inode *inode = mapping->host;
-	int result = VM_FAULT_FALLBACK;
+	vm_fault_t result = VM_FAULT_FALLBACK;
 	struct iomap iomap = { 0 };
 	pgoff_t max_pgoff, pgoff;
 	void *entry;
@@ -1471,7 +1481,7 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
 	return result;
 }
 #else
-static int dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
+static vm_fault_t dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
 			       const struct iomap_ops *ops)
 {
 	return VM_FAULT_FALLBACK;
@@ -1491,7 +1501,7 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
  * has done all the necessary locking for page fault to proceed
  * successfully.
  */
-int dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
+vm_fault_t dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
 		    pfn_t *pfnp, int *iomap_errp, const struct iomap_ops *ops)
 {
 	switch (pe_size) {
@@ -1515,14 +1525,14 @@ int dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
  * DAX file.  It takes care of marking corresponding radix tree entry as dirty
  * as well.
  */
-static int dax_insert_pfn_mkwrite(struct vm_fault *vmf,
+static vm_fault_t dax_insert_pfn_mkwrite(struct vm_fault *vmf,
 				  enum page_entry_size pe_size,
 				  pfn_t pfn)
 {
 	struct address_space *mapping = vmf->vma->vm_file->f_mapping;
 	void *entry, **slot;
 	pgoff_t index = vmf->pgoff;
-	int vmf_ret, error;
+	vm_fault_t vmf_ret;
 
 	spin_lock_irq(&mapping->tree_lock);
 	entry = get_unlocked_mapping_entry(mapping, index, &slot);
@@ -1541,8 +1551,7 @@ static int dax_insert_pfn_mkwrite(struct vm_fault *vmf,
 	spin_unlock_irq(&mapping->tree_lock);
 	switch (pe_size) {
 	case PE_SIZE_PTE:
-		error = vm_insert_mixed_mkwrite(vmf->vma, vmf->address, pfn);
-		vmf_ret = dax_fault_return(error);
+		vmf_ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address, pfn);
 		break;
 #ifdef CONFIG_FS_DAX_PMD
 	case PE_SIZE_PMD:
@@ -1568,8 +1577,8 @@ static int dax_insert_pfn_mkwrite(struct vm_fault *vmf,
  * stored persistently on the media and handles inserting of appropriate page
  * table entry.
  */
-int dax_finish_sync_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
-			  pfn_t pfn)
+vm_fault_t dax_finish_sync_fault(struct vm_fault *vmf,
+		enum page_entry_size pe_size, pfn_t pfn)
 {
 	int err;
 	loff_t start = ((loff_t)vmf->pgoff) << PAGE_SHIFT;
@@ -1581,7 +1590,8 @@ int dax_finish_sync_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
 		len = PMD_SIZE;
 	else
 		WARN_ON_ONCE(1);
-	err = vfs_fsync_range(vmf->vma->vm_file, start, start + len - 1, 1);
+	err = vfs_fsync_range(vmf->vma->vm_file,
+			start, start + len - 1, 1);
 	if (err)
 		return VM_FAULT_SIGBUS;
 	return dax_insert_pfn_mkwrite(vmf, pe_size, pfn);
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 0185ecd..5b3a5ff 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -97,8 +97,8 @@ ssize_t dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter,
 		const struct iomap_ops *ops);
 int dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
 		    pfn_t *pfnp, int *errp, const struct iomap_ops *ops);
-int dax_finish_sync_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
-			  pfn_t pfn);
+vm_fault_t dax_finish_sync_fault(struct vm_fault *vmf,
+		enum page_entry_size pe_size, pfn_t pfn);
 int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index);
 int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
 				      pgoff_t index);
-- 
1.9.1

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Ross Zwisler]

On Tue, Apr 17, 2018 at 09:05:07PM +0530, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
> 
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
> 
> There was an existing bug inside dax_load_hole()
> if vm_insert_mixed had failed to allocate a page table,
> we'd return VM_FAULT_NOPAGE instead of VM_FAULT_OOM.
> With vmf_insert_mixed this issue is addressed.
> 
> vmf_insert_mixed_mkwrite() is the new wrapper function
> which will convert err returned from vm_insert_mixed_
> mkwrite() to vm_fault_t type.

This doesn't apply cleanly to v4.17-rc1, as it collides with patches from Dan,
and it doesn't work when applied to v4.16 because we're missing the
vmf_insert_mixed() wrapper.  Generally when I have an odd baseline I provide a
link to a publicly accessible tree so people can grab a working version of my
patches, but in this case you probably just need to merge forward to the
latest linux/master.

> @@ -1094,6 +1094,18 @@ static bool dax_fault_is_synchronous(unsigned long flags,
>  		&& (iomap->flags & IOMAP_F_DIRTY);
>  }
>  
> +static vm_fault_t vmf_insert_mixed_mkwrite(struct vm_area_struct *vma,
> +			unsigned long addr, pfn_t pfn)
> +{
> +	int error;
> +	vm_fault_t vmf_ret;
> +
> +	error = vm_insert_mixed_mkwrite(vma, addr, pfn);
> +	vmf_ret = dax_fault_return(error);

No need for all the temp variables and extra lines.  This is simpler as:
	error = vm_insert_mixed_mkwrite(vma, addr, pfn);
	return dax_fault_return(error);

Obviating the need for vmf_ret, or just:

	return dax_fault_return(vm_insert_mixed_mkwrite(vma, addr, pfn));

Though this may change once you address the -EBUSY handling mentioned below.

> @@ -1225,14 +1237,12 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
>  		}
>  		trace_dax_insert_mapping(inode, vmf, entry);
>  		if (write)
> -			error = vm_insert_mixed_mkwrite(vma, vaddr, pfn);
> +			vmf_ret = vmf_insert_mixed_mkwrite(vma, vaddr, pfn);
>  		else
> -			error = vm_insert_mixed(vma, vaddr, pfn);
> +			vmf_ret = vmf_insert_mixed(vma, vaddr, pfn);
> +
> +		goto finish_iomap;
>  
> -		/* -EBUSY is fine, somebody else faulted on the same PTE */
> -		if (error == -EBUSY)
> -			error = 0;

In this rework you've lost this special case handling for -EBUSY, which means
that with your code users hitting -EBUSY (which is a non-error) will get a
VM_FAULT_SIGBUS.

> @@ -1568,8 +1577,8 @@ static int dax_insert_pfn_mkwrite(struct vm_fault *vmf,
>   * stored persistently on the media and handles inserting of appropriate page
>   * table entry.
>   */
> -int dax_finish_sync_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
> -			  pfn_t pfn)
> +vm_fault_t dax_finish_sync_fault(struct vm_fault *vmf,
> +		enum page_entry_size pe_size, pfn_t pfn)
>  {
>  	int err;
>  	loff_t start = ((loff_t)vmf->pgoff) << PAGE_SHIFT;
> @@ -1581,7 +1590,8 @@ int dax_finish_sync_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
>  		len = PMD_SIZE;
>  	else
>  		WARN_ON_ONCE(1);
> -	err = vfs_fsync_range(vmf->vma->vm_file, start, start + len - 1, 1);
> +	err = vfs_fsync_range(vmf->vma->vm_file,
> +			start, start + len - 1, 1);

Unrelated and unneeded whitespace change.

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Matthew Wilcox]

On Tue, Apr 17, 2018 at 09:05:07PM +0530, Souptick Joarder wrote:
> There was an existing bug inside dax_load_hole()
> if vm_insert_mixed had failed to allocate a page table,
> we'd return VM_FAULT_NOPAGE instead of VM_FAULT_OOM.
> With vmf_insert_mixed this issue is addressed.
> 
> vmf_insert_mixed_mkwrite() is the new wrapper function
> which will convert err returned from vm_insert_mixed_
> mkwrite() to vm_fault_t type.

Since dax is the only caller of vm_insert_mixed_mkwrite(), rather
than introducing a wrapper function, you should just convert
vm_insert_mixed_mkwrite() to vmf_insert_mixed_mkwrite().

>  	int error, major = 0;
>  	bool write = vmf->flags & FAULT_FLAG_WRITE;
>  	bool sync;
> -	int vmf_ret = 0;
> +	vm_fault_t vmf_ret = 0;

I'd rename this to just 'ret' like everywhere else.

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

Hi Souptick,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16]
[cannot apply to linus/master v4.17-rc1 next-20180418]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Souptick-Joarder/fs-dax-Adding-new-return-type-vm_fault_t/20180418-152151
config: i386-randconfig-s1-201815 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from fs//ext2/file.c:24:0:
>> include/linux/dax.h:100:1: error: unknown type name 'vm_fault_t'
    vm_fault_t dax_finish_sync_fault(struct vm_fault *vmf,
    ^~~~~~~~~~

vim +/vm_fault_t +100 include/linux/dax.h

    95	
    96	ssize_t dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter,
    97			const struct iomap_ops *ops);
    98	int dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
    99			    pfn_t *pfnp, int *errp, const struct iomap_ops *ops);
 > 100	vm_fault_t dax_finish_sync_fault(struct vm_fault *vmf,
   101			enum page_entry_size pe_size, pfn_t pfn);
   102	int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index);
   103	int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
   104					      pgoff_t index);
   105	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29446 bytes --]

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

Hi Souptick,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16]
[cannot apply to linus/master v4.17-rc1 next-20180418]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Souptick-Joarder/fs-dax-Adding-new-return-type-vm_fault_t/20180418-152151
config: i386-randconfig-x014-201815 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/linux/mempolicy.h:11:0,
                    from include/linux/migrate.h:6,
                    from fs//btrfs/disk-io.c:29:
>> include/linux/dax.h:100:1: error: unknown type name 'vm_fault_t'; did you mean 'vm_flags_t'?
    vm_fault_t dax_finish_sync_fault(struct vm_fault *vmf,
    ^~~~~~~~~~
    vm_flags_t

vim +100 include/linux/dax.h

    95	
    96	ssize_t dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter,
    97			const struct iomap_ops *ops);
    98	int dax_iomap_fault(struct vm_fault *vmf, enum page_entry_size pe_size,
    99			    pfn_t *pfnp, int *errp, const struct iomap_ops *ops);
 > 100	vm_fault_t dax_finish_sync_fault(struct vm_fault *vmf,
   101			enum page_entry_size pe_size, pfn_t pfn);
   102	int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index);
   103	int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
   104					      pgoff_t index);
   105	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29322 bytes --]

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Souptick Joarder]

Hi Ross,

On Wed, Apr 18, 2018 at 2:48 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Souptick,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on v4.16]
> [cannot apply to linus/master v4.17-rc1 next-20180418]

Does it mean patch was initially applied in v4.16 ?
As v4.16 build breaks it fails to apply in v4.17-rc1 and next-20180418 ?

Even for another patch in gma500 I receive similar error
"vm_fault_t not defined".

These patches should work from 4.17-rc1 onward.

Whom to reach out for kbuild test failures ?

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Fengguang Wu]

On Wed, Apr 18, 2018 at 05:08:33PM +0530, Souptick Joarder wrote:
>Hi Ross,
>
>On Wed, Apr 18, 2018 at 2:48 PM, kbuild test robot <lkp@intel.com> wrote:
>> Hi Souptick,
>>
>> Thank you for the patch! Yet something to improve:
>>
>> [auto build test ERROR on v4.16]
>> [cannot apply to linus/master v4.17-rc1 next-20180418]
>
>Does it mean patch was initially applied in v4.16 ?

Yes.

>As v4.16 build breaks it fails to apply in v4.17-rc1 and next-20180418 ?

When there is a build fail the robot will apply-and-test the patch on
more base trees. Here the patch cannot apply to linus/master,
v4.17-rc1 and next-20180418, so cannot be further tested there.

Thanks,
Fengguang

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Ross Zwisler]

On Wed, Apr 18, 2018 at 08:13:12PM +0800, Fengguang Wu wrote:
> On Wed, Apr 18, 2018 at 05:08:33PM +0530, Souptick Joarder wrote:
> > Hi Ross,
> > 
> > On Wed, Apr 18, 2018 at 2:48 PM, kbuild test robot <lkp@intel.com> wrote:
> > > Hi Souptick,
> > > 
> > > Thank you for the patch! Yet something to improve:
> > > 
> > > [auto build test ERROR on v4.16]
> > > [cannot apply to linus/master v4.17-rc1 next-20180418]
> > 
> > Does it mean patch was initially applied in v4.16 ?
> 
> Yes.
> 
> > As v4.16 build breaks it fails to apply in v4.17-rc1 and next-20180418 ?
> 
> When there is a build fail the robot will apply-and-test the patch on
> more base trees. Here the patch cannot apply to linus/master,
> v4.17-rc1 and next-20180418, so cannot be further tested there.

Right - this needs to be rebased onto the current linux/master, as it
currently collides with a patch from Dan which went into v4.17-rc1.

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Souptick Joarder]

On Wed, Apr 18, 2018 at 2:30 AM, Matthew Wilcox <willy@infradead.org> wrote:
> On Tue, Apr 17, 2018 at 09:05:07PM +0530, Souptick Joarder wrote:
>> There was an existing bug inside dax_load_hole()
>> if vm_insert_mixed had failed to allocate a page table,
>> we'd return VM_FAULT_NOPAGE instead of VM_FAULT_OOM.
>> With vmf_insert_mixed this issue is addressed.
>>
>> vmf_insert_mixed_mkwrite() is the new wrapper function
>> which will convert err returned from vm_insert_mixed_
>> mkwrite() to vm_fault_t type.
>
> Since dax is the only caller of vm_insert_mixed_mkwrite(), rather
> than introducing a wrapper function, you should just convert
> vm_insert_mixed_mkwrite() to vmf_insert_mixed_mkwrite().
>

Although vm_insert_mixed_mkwrite () is getting called only from dax,
but if we directly change it to vmf_insert_mixed_mkwrite()
with return type vm_fault_t, we end with changing multiple functions
recursively. In my opinion, it will complicate things.

It's better to go with current inline vmf_insert_mixed_mkwrite() approach.

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Matthew Wilcox]

On Sat, Apr 21, 2018 at 12:51:44AM +0530, Souptick Joarder wrote:
> On Wed, Apr 18, 2018 at 2:30 AM, Matthew Wilcox <willy@infradead.org> wrote:
> > On Tue, Apr 17, 2018 at 09:05:07PM +0530, Souptick Joarder wrote:
> >> There was an existing bug inside dax_load_hole()
> >> if vm_insert_mixed had failed to allocate a page table,
> >> we'd return VM_FAULT_NOPAGE instead of VM_FAULT_OOM.
> >> With vmf_insert_mixed this issue is addressed.
> >>
> >> vmf_insert_mixed_mkwrite() is the new wrapper function
> >> which will convert err returned from vm_insert_mixed_
> >> mkwrite() to vm_fault_t type.
> >
> > Since dax is the only caller of vm_insert_mixed_mkwrite(), rather
> > than introducing a wrapper function, you should just convert
> > vm_insert_mixed_mkwrite() to vmf_insert_mixed_mkwrite().
> >
> 
> Although vm_insert_mixed_mkwrite () is getting called only from dax,
> but if we directly change it to vmf_insert_mixed_mkwrite()
> with return type vm_fault_t, we end with changing multiple functions
> recursively. In my opinion, it will complicate things.
> 
> It's better to go with current inline vmf_insert_mixed_mkwrite() approach.

No, it's not.  The point is to create patches which are self-contained
and don't break anything.  So we can't change vm_insert_mixed because
it has so many users, and we need to transition to it gradually.
But vm_insert_mixed_mkwrite only has one user, and we can just change
both at the same time.

Re: [PATCH] fs: dax: Adding new return type vm_fault_t

[Souptick Joarder]

>> Although vm_insert_mixed_mkwrite () is getting called only from dax,
>> but if we directly change it to vmf_insert_mixed_mkwrite()
>> with return type vm_fault_t, we end with changing multiple functions
>> recursively. In my opinion, it will complicate things.
>>
>> It's better to go with current inline vmf_insert_mixed_mkwrite() approach.
>
> No, it's not.  The point is to create patches which are self-contained
> and don't break anything.  So we can't change vm_insert_mixed because
> it has so many users, and we need to transition to it gradually.
> But vm_insert_mixed_mkwrite only has one user, and we can just change
> both at the same time.

Agree with you. I have send both the patches cc'ing mm and fsdevel list.
vmf_insert_mixed_mkwrite  patch has to go first in linus tree.