Re: [PATCH] fs: btrfs: Change return type to vm_fault_t

From: David Sterba
Date: Wed Jun 06 2018 - 10:48:27 EST


On Wed, Jun 06, 2018 at 07:39:53PM +0530, Souptick Joarder wrote:
> >> @@ -8885,8 +8885,9 @@ int btrfs_page_mkwrite(struct vm_fault *vmf)
> >> char *kaddr;
> >> unsigned long zero_start;
> >> loff_t size;
> >> - int ret;
> >> + int err;
> >
> > Please use ret2 for the variable name. We're leaving the err/ret naming
> > pattern and use 'ret' to match the function return value and ret2, ret3
> > for the temporary return values.
>
> Is this new pattern specific to fs/btrfs file system ?

Yes it is, you're not expected to know it in advance but as this was not
the only thing to update in the patch I did not fix it myself.

> >> - ret = 0;
> >> + err = 0;
> >>
> >> /* page is wholly or partially inside EOF */
> >> if (page_start + PAGE_SIZE > size)
> >> @@ -9009,11 +9007,11 @@ int btrfs_page_mkwrite(struct vm_fault *vmf)
> >> unlock_extent_cached(io_tree, page_start, page_end, &cached_state);
> >>
> >> out_unlock:
> >> - if (!ret) {
> >> + if (!err) {
> >> btrfs_delalloc_release_extents(BTRFS_I(inode), PAGE_SIZE, true);
> >> sb_end_pagefault(inode->i_sb);
> >> extent_changeset_free(data_reserved);
> >> - return VM_FAULT_LOCKED;
> >> + ret = VM_FAULT_LOCKED;
> >
> > That's not a direct change as it changes the code flow and lets the
> > branch continue. As the error handling and cleanup code is partially
> > done in this branch continuing wrong AFAICS.
>
> Sorry about it, I will correct and send v2.
> Is it possible to get it merge in 4.18 / 4.18-rc-X ?

Ok.