[PATCH] btrfs: Use iocb to derive pos instead of passing a separate parameter

[PATCH] btrfs: Use iocb to derive pos instead of passing a separate parameter

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues <rgoldwyn@suse.com>

struct kiocb carries the ki_pos, so there is no need to pass it as
a separate function parameter.

generic_file_direct_write() increments ki_pos, so we now assign pos
after the function.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/file.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index f660ba1e5e58..f84100a60cec 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1569,10 +1569,11 @@ static noinline int check_can_nocow(struct btrfs_inode *inode, loff_t pos,
 	return ret;
 }
 
-static noinline ssize_t __btrfs_buffered_write(struct file *file,
-					       struct iov_iter *i,
-					       loff_t pos)
+static noinline ssize_t __btrfs_buffered_write(struct kiocb *iocb,
+					       struct iov_iter *i)
 {
+	struct file *file = iocb->ki_filp;
+	loff_t pos = iocb->ki_pos;
 	struct inode *inode = file_inode(file);
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	struct btrfs_root *root = BTRFS_I(inode)->root;
@@ -1804,7 +1805,7 @@ static ssize_t __btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file_inode(file);
-	loff_t pos = iocb->ki_pos;
+	loff_t pos;
 	ssize_t written;
 	ssize_t written_buffered;
 	loff_t endbyte;
@@ -1815,8 +1816,8 @@ static ssize_t __btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 	if (written < 0 || !iov_iter_count(from))
 		return written;
 
-	pos += written;
-	written_buffered = __btrfs_buffered_write(file, from, pos);
+	pos = iocb->ki_pos;
+	written_buffered = __btrfs_buffered_write(iocb, from);
 	if (written_buffered < 0) {
 		err = written_buffered;
 		goto out;
@@ -1953,7 +1954,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 	if (iocb->ki_flags & IOCB_DIRECT) {
 		num_written = __btrfs_direct_write(iocb, from);
 	} else {
-		num_written = __btrfs_buffered_write(file, from, pos);
+		num_written = __btrfs_buffered_write(iocb, from);
 		if (num_written > 0)
 			iocb->ki_pos = pos + num_written;
 		if (clean_page)
-- 
2.16.4

Re: [PATCH] btrfs: Use iocb to derive pos instead of passing a separate parameter

[Misono Tomohiro]

So, this is the updated version of https://patchwork.kernel.org/patch/10063039/

This time xfstest is ok and
 Reviewed-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

On 2018/06/18 2:39, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> struct kiocb carries the ki_pos, so there is no need to pass it as
> a separate function parameter.
> 
> generic_file_direct_write() increments ki_pos, so we now assign pos
> after the function.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/btrfs/file.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index f660ba1e5e58..f84100a60cec 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1569,10 +1569,11 @@ static noinline int check_can_nocow(struct btrfs_inode *inode, loff_t pos,
>  	return ret;
>  }
>  
> -static noinline ssize_t __btrfs_buffered_write(struct file *file,
> -					       struct iov_iter *i,
> -					       loff_t pos)
> +static noinline ssize_t __btrfs_buffered_write(struct kiocb *iocb,
> +					       struct iov_iter *i)
>  {
> +	struct file *file = iocb->ki_filp;
> +	loff_t pos = iocb->ki_pos;
>  	struct inode *inode = file_inode(file);
>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>  	struct btrfs_root *root = BTRFS_I(inode)->root;
> @@ -1804,7 +1805,7 @@ static ssize_t __btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
>  {
>  	struct file *file = iocb->ki_filp;
>  	struct inode *inode = file_inode(file);
> -	loff_t pos = iocb->ki_pos;
> +	loff_t pos;
>  	ssize_t written;
>  	ssize_t written_buffered;
>  	loff_t endbyte;
> @@ -1815,8 +1816,8 @@ static ssize_t __btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
>  	if (written < 0 || !iov_iter_count(from))
>  		return written;
>  
> -	pos += written;
> -	written_buffered = __btrfs_buffered_write(file, from, pos);
> +	pos = iocb->ki_pos;
> +	written_buffered = __btrfs_buffered_write(iocb, from);
>  	if (written_buffered < 0) {
>  		err = written_buffered;
>  		goto out;
> @@ -1953,7 +1954,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
>  	if (iocb->ki_flags & IOCB_DIRECT) {
>  		num_written = __btrfs_direct_write(iocb, from);
>  	} else {
> -		num_written = __btrfs_buffered_write(file, from, pos);
> +		num_written = __btrfs_buffered_write(iocb, from);
>  		if (num_written > 0)
>  			iocb->ki_pos = pos + num_written;
>  		if (clean_page)
> 

Re: [PATCH] btrfs: Use iocb to derive pos instead of passing a separate parameter

[David Sterba]

On Mon, Jun 25, 2018 at 01:58:58PM +0900, Misono Tomohiro wrote:
> So, this is the updated version of https://patchwork.kernel.org/patch/10063039/
> 
> This time xfstest is ok and
>  Reviewed-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

Your comment about invalidate_mapping_pages is also ok, right? As
filemap_fdatawait_range and invalidate_mapping_pages use the same
start/end of the range.

Re: [PATCH] btrfs: Use iocb to derive pos instead of passing a separate parameter

[Goldwyn Rodrigues]

On 06-25 18:20, David Sterba wrote:
> On Mon, Jun 25, 2018 at 01:58:58PM +0900, Misono Tomohiro wrote:
> > So, this is the updated version of https://patchwork.kernel.org/patch/10063039/
> > 
> > This time xfstest is ok and
> >  Reviewed-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

Yes, thats right.

> 
> Your comment about invalidate_mapping_pages is also ok, right? As
> filemap_fdatawait_range and invalidate_mapping_pages use the same
> start/end of the range.

I did not mess around with other functions which are affected by
iocb->ki_pos (as opposed to local pos). So, this should be safe with
respect to invalidate_mapping_pages().

-- 
Goldwyn

Re: [PATCH] btrfs: Use iocb to derive pos instead of passing a separate parameter

[Misono Tomohiro]

On 2018/06/26 1:20, David Sterba wrote:
> On Mon, Jun 25, 2018 at 01:58:58PM +0900, Misono Tomohiro wrote:
>> So, this is the updated version of https://patchwork.kernel.org/patch/10063039/
>>
>> This time xfstest is ok and
>>  Reviewed-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
> 
> Your comment about invalidate_mapping_pages is also ok, right? As
> filemap_fdatawait_range and invalidate_mapping_pages use the same
> start/end of the range.
> 

This time local variable 'pos' is kept to have the same value before and
invalidate_mapping_pages() uses it, so it should be ok.