Re: [PATCH v6 11/11] btrfs: implement RWF_ENCODED writes

From: Josef Bacik <josef@toxicpanda.com>
To: Omar Sandoval <osandov@osandov.com>,
	linux-fsdevel@vger.kernel.org, linux-btrfs@vger.kernel.org,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>
Cc: Dave Chinner <david@fromorbit.com>, Jann Horn <jannh@google.com>,
	Amir Goldstein <amir73il@gmail.com>,
	Aleksa Sarai <cyphar@cyphar.com>,
	linux-api@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v6 11/11] btrfs: implement RWF_ENCODED writes
Date: Wed, 2 Dec 2020 17:03:14 -0500	[thread overview]
Message-ID: <1336bfb3-b1e7-62ea-a9c4-21fdf344d1f8@toxicpanda.com> (raw)
In-Reply-To: <fe58a0fe2c1455567fe1e9e62232e8b711797a93.1605723568.git.osandov@fb.com>

On 11/18/20 2:18 PM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> The implementation resembles direct I/O: we have to flush any ordered
> extents, invalidate the page cache, and do the io tree/delalloc/extent
> map/ordered extent dance. From there, we can reuse the compression code
> with a minor modification to distinguish the write from writeback. This
> also creates inline extents when possible.
> 
> Now that read and write are implemented, this also sets the
> FMODE_ENCODED_IO flag in btrfs_file_open().
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>   fs/btrfs/compression.c  |   7 +-
>   fs/btrfs/compression.h  |   6 +-
>   fs/btrfs/ctree.h        |   2 +
>   fs/btrfs/file.c         |  37 +++++-
>   fs/btrfs/inode.c        | 259 +++++++++++++++++++++++++++++++++++++++-
>   fs/btrfs/ordered-data.c |  12 +-
>   fs/btrfs/ordered-data.h |   2 +
>   7 files changed, 313 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index eaa6fe21c08e..015c9e5d75b9 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -336,7 +336,8 @@ static void end_compressed_bio_write(struct bio *bio)
>   			bio->bi_status == BLK_STS_OK);
>   	cb->compressed_pages[0]->mapping = NULL;
>   
> -	end_compressed_writeback(inode, cb);
> +	if (cb->writeback)
> +		end_compressed_writeback(inode, cb);
>   	/* note, our inode could be gone now */
>   
>   	/*
> @@ -372,7 +373,8 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start,
>   				 struct page **compressed_pages,
>   				 unsigned long nr_pages,
>   				 unsigned int write_flags,
> -				 struct cgroup_subsys_state *blkcg_css)
> +				 struct cgroup_subsys_state *blkcg_css,
> +				 bool writeback)
>   {
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	struct bio *bio = NULL;
> @@ -396,6 +398,7 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start,
>   	cb->mirror_num = 0;
>   	cb->compressed_pages = compressed_pages;
>   	cb->compressed_len = compressed_len;
> +	cb->writeback = writeback;
>   	cb->orig_bio = NULL;
>   	cb->nr_pages = nr_pages;
>   
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 8001b700ea3a..f95cdc16f503 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -49,6 +49,9 @@ struct compressed_bio {
>   	/* the compression algorithm for this bio */
>   	int compress_type;
>   
> +	/* Whether this is a write for writeback. */
> +	bool writeback;
> +
>   	/* number of compressed pages in the array */
>   	unsigned long nr_pages;
>   
> @@ -96,7 +99,8 @@ blk_status_t btrfs_submit_compressed_write(struct btrfs_inode *inode, u64 start,
>   				  struct page **compressed_pages,
>   				  unsigned long nr_pages,
>   				  unsigned int write_flags,
> -				  struct cgroup_subsys_state *blkcg_css);
> +				  struct cgroup_subsys_state *blkcg_css,
> +				  bool writeback);
>   blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>   				 int mirror_num, unsigned long bio_flags);
>   
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index ce78424f1d98..9b585ac9c7a9 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3134,6 +3134,8 @@ int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end);
>   void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start,
>   					  u64 end, int uptodate);
>   ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter);
> +ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
> +			       struct encoded_iov *encoded);
>   
>   extern const struct dentry_operations btrfs_dentry_operations;
>   extern const struct iomap_ops btrfs_dio_iomap_ops;
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 193477565200..f815ffb93d43 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1994,6 +1994,32 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
>   	return written ? written : err;
>   }
>   
> +static ssize_t btrfs_encoded_write(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	struct file *file = iocb->ki_filp;
> +	struct inode *inode = file_inode(file);
> +	struct encoded_iov encoded;
> +	ssize_t ret;
> +
> +	ret = copy_encoded_iov_from_iter(&encoded, from);
> +	if (ret)
> +		return ret;
> +
> +	btrfs_inode_lock(inode, 0);
> +	ret = generic_encoded_write_checks(iocb, &encoded);
> +	if (ret || encoded.len == 0)
> +		goto out;
> +
> +	ret = btrfs_write_check(iocb, from, encoded.len);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = btrfs_do_encoded_write(iocb, from, &encoded);
> +out:
> +	btrfs_inode_unlock(inode, 0);
> +	return ret;
> +}
> +
>   static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
>   				    struct iov_iter *from)
>   {
> @@ -2012,14 +2038,17 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
>   	if (test_bit(BTRFS_FS_STATE_ERROR, &fs_info->fs_state))
>   		return -EROFS;
>   
> -	if (!(iocb->ki_flags & IOCB_DIRECT) &&
> -	    (iocb->ki_flags & IOCB_NOWAIT))
> +	if ((iocb->ki_flags & IOCB_NOWAIT) &&
> +	    (!(iocb->ki_flags & IOCB_DIRECT) ||
> +	     (iocb->ki_flags & IOCB_ENCODED)))
>   		return -EOPNOTSUPP;
>   
>   	if (sync)
>   		atomic_inc(&BTRFS_I(inode)->sync_writers);
>   
> -	if (iocb->ki_flags & IOCB_DIRECT)
> +	if (iocb->ki_flags & IOCB_ENCODED)
> +		num_written = btrfs_encoded_write(iocb, from);
> +	else if (iocb->ki_flags & IOCB_DIRECT)
>   		num_written = btrfs_direct_write(iocb, from);
>   	else
>   		num_written = btrfs_buffered_write(iocb, from);
> @@ -3586,7 +3615,7 @@ static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int whence)
>   
>   static int btrfs_file_open(struct inode *inode, struct file *filp)
>   {
> -	filp->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC;
> +	filp->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC | FMODE_ENCODED_IO;
>   	return generic_file_open(inode, filp);
>   }
>   
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index b0e800897b3b..2bf7b487939f 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -935,7 +935,7 @@ static noinline void submit_compressed_extents(struct async_chunk *async_chunk)
>   				    ins.offset, async_extent->pages,
>   				    async_extent->nr_pages,
>   				    async_chunk->write_flags,
> -				    async_chunk->blkcg_css)) {
> +				    async_chunk->blkcg_css, true)) {
>   			struct page *p = async_extent->pages[0];
>   			const u64 start = async_extent->start;
>   			const u64 end = start + async_extent->ram_size - 1;
> @@ -2703,6 +2703,7 @@ static int insert_ordered_extent_file_extent(struct btrfs_trans_handle *trans,
>   	 * except if the ordered extent was truncated.
>   	 */
>   	update_inode_bytes = test_bit(BTRFS_ORDERED_DIRECT, &oe->flags) ||
> +	                     test_bit(BTRFS_ORDERED_ENCODED, &oe->flags) ||

Gotta use our git hooks, checkpatch caught the spaces here.  Thanks,

Josef