[PATCH 0/6] btrfs: add io_uring for encoded reads

[PATCH 0/6] btrfs: add io_uring for encoded reads

[Mark Harmstone]

This patch series adds io_uring support for btrfs encoded reads,
complementing the ioctl we already have. The first few patches refactor
the ioctl code so that the bio wait is moved to the outer function, and
so that we can share as much code as possible between the two
interfaces.

Mark Harmstone (6):
  btrfs: remove iocb from btrfs_encoded_read
  btrfs: store encoded read state in struct btrfs_encoded_read_private
  btrfs: add btrfs_encoded_read_finish
  btrfs: add btrfs_prepare_encoded_read
  btrfs: move wait out of btrfs_encoded_read
  btrfs: add io_uring interface for encoded reads

 fs/btrfs/btrfs_inode.h |  23 +++-
 fs/btrfs/file.c        |   1 +
 fs/btrfs/inode.c       | 292 ++++++++++++++++++++++++-----------------
 fs/btrfs/ioctl.c       | 194 +++++++++++++++++++--------
 fs/btrfs/ioctl.h       |   3 +
 5 files changed, 337 insertions(+), 176 deletions(-)

-- 
2.44.2

[PATCH 1/6] btrfs: remove iocb from btrfs_encoded_read

[Mark Harmstone]

We initialize a struct kiocb for encoded reads, but this never gets
passed outside the driver. Simplify things by replacing it with a file,
offset pair.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 fs/btrfs/btrfs_inode.h |  3 ++-
 fs/btrfs/inode.c       | 59 ++++++++++++++++++++----------------------
 fs/btrfs/ioctl.c       |  6 +----
 3 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 3056c8aed8ef..affe70929234 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -605,7 +605,8 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 					  u64 file_offset, u64 disk_bytenr,
 					  u64 disk_io_size,
 					  struct page **pages);
-ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
+ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
+			   struct iov_iter *iter,
 			   struct btrfs_ioctl_encoded_io_args *encoded);
 ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
 			       const struct btrfs_ioctl_encoded_io_args *encoded);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b1b6564ab68f..a0cc029d95ed 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8993,7 +8993,7 @@ int btrfs_encoded_io_compression_from_extent(struct btrfs_fs_info *fs_info,
 }
 
 static ssize_t btrfs_encoded_read_inline(
-				struct kiocb *iocb,
+				struct btrfs_inode *inode, loff_t pos,
 				struct iov_iter *iter, u64 start,
 				u64 lockend,
 				struct extent_state **cached_state,
@@ -9001,7 +9001,6 @@ static ssize_t btrfs_encoded_read_inline(
 				struct btrfs_ioctl_encoded_io_args *encoded,
 				bool *unlocked)
 {
-	struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
 	struct btrfs_root *root = inode->root;
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct extent_io_tree *io_tree = &inode->io_tree;
@@ -9034,7 +9033,7 @@ static ssize_t btrfs_encoded_read_inline(
 	ptr = btrfs_file_extent_inline_start(item);
 
 	encoded->len = min_t(u64, extent_start + ram_bytes,
-			     inode->vfs_inode.i_size) - iocb->ki_pos;
+			     inode->vfs_inode.i_size) - pos;
 	ret = btrfs_encoded_io_compression_from_extent(fs_info,
 				 btrfs_file_extent_compression(leaf, item));
 	if (ret < 0)
@@ -9051,12 +9050,12 @@ static ssize_t btrfs_encoded_read_inline(
 		}
 		count = inline_size;
 		encoded->unencoded_len = ram_bytes;
-		encoded->unencoded_offset = iocb->ki_pos - extent_start;
+		encoded->unencoded_offset = pos - extent_start;
 	} else {
 		count = min_t(u64, count, encoded->len);
 		encoded->len = count;
 		encoded->unencoded_len = count;
-		ptr += iocb->ki_pos - extent_start;
+		ptr += pos - extent_start;
 	}
 
 	tmp = kmalloc(count, GFP_NOFS);
@@ -9151,15 +9150,14 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 	return blk_status_to_errno(READ_ONCE(priv.status));
 }
 
-static ssize_t btrfs_encoded_read_regular(struct kiocb *iocb,
-					  struct iov_iter *iter,
+static ssize_t btrfs_encoded_read_regular(struct btrfs_inode *inode,
+					  loff_t offset, struct iov_iter *iter,
 					  u64 start, u64 lockend,
 					  struct extent_state **cached_state,
 					  u64 disk_bytenr, u64 disk_io_size,
 					  size_t count, bool compressed,
 					  bool *unlocked)
 {
-	struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
 	struct extent_io_tree *io_tree = &inode->io_tree;
 	struct page **pages;
 	unsigned long nr_pages, i;
@@ -9190,8 +9188,8 @@ static ssize_t btrfs_encoded_read_regular(struct kiocb *iocb,
 		i = 0;
 		page_offset = 0;
 	} else {
-		i = (iocb->ki_pos - start) >> PAGE_SHIFT;
-		page_offset = (iocb->ki_pos - start) & (PAGE_SIZE - 1);
+		i = (offset - start) >> PAGE_SHIFT;
+		page_offset = (offset - start) & (PAGE_SIZE - 1);
 	}
 	cur = 0;
 	while (cur < count) {
@@ -9217,10 +9215,11 @@ static ssize_t btrfs_encoded_read_regular(struct kiocb *iocb,
 	return ret;
 }
 
-ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
+ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
+			   struct iov_iter *iter,
 			   struct btrfs_ioctl_encoded_io_args *encoded)
 {
-	struct btrfs_inode *inode = BTRFS_I(file_inode(iocb->ki_filp));
+	struct btrfs_inode *inode = BTRFS_I(file_inode(file));
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct extent_io_tree *io_tree = &inode->io_tree;
 	ssize_t ret;
@@ -9230,17 +9229,17 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
 	struct extent_map *em;
 	bool unlocked = false;
 
-	file_accessed(iocb->ki_filp);
+	file_accessed(file);
 
 	btrfs_inode_lock(inode, BTRFS_ILOCK_SHARED);
 
-	if (iocb->ki_pos >= inode->vfs_inode.i_size) {
+	if (offset >= inode->vfs_inode.i_size) {
 		btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
 		return 0;
 	}
-	start = ALIGN_DOWN(iocb->ki_pos, fs_info->sectorsize);
+	start = ALIGN_DOWN(offset, fs_info->sectorsize);
 	/*
-	 * We don't know how long the extent containing iocb->ki_pos is, but if
+	 * We don't know how long the extent containing offset is, but if
 	 * it's compressed we know that it won't be longer than this.
 	 */
 	lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
@@ -9277,10 +9276,11 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
 		 */
 		free_extent_map(em);
 		em = NULL;
-		ret = btrfs_encoded_read_inline(iocb, iter, start, lockend,
-						&cached_state, extent_start,
-						count, encoded, &unlocked);
-		goto out;
+		ret = btrfs_encoded_read_inline(inode, offset, iter, start,
+						lockend, &cached_state,
+						extent_start, count, encoded,
+						&unlocked);
+		goto out_em;
 	}
 
 	/*
@@ -9288,7 +9288,7 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
 	 * that.
 	 */
 	encoded->len = min_t(u64, extent_map_end(em),
-			     inode->vfs_inode.i_size) - iocb->ki_pos;
+			     inode->vfs_inode.i_size) - offset;
 	if (em->disk_bytenr == EXTENT_MAP_HOLE ||
 	    (em->flags & EXTENT_FLAG_PREALLOC)) {
 		disk_bytenr = EXTENT_MAP_HOLE;
@@ -9308,7 +9308,7 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
 		disk_io_size = em->disk_num_bytes;
 		count = em->disk_num_bytes;
 		encoded->unencoded_len = em->ram_bytes;
-		encoded->unencoded_offset = iocb->ki_pos - (em->start - em->offset);
+		encoded->unencoded_offset = offset - (em->start - em->offset);
 		ret = btrfs_encoded_io_compression_from_extent(fs_info,
 							       extent_map_compression(em));
 		if (ret < 0)
@@ -9322,8 +9322,8 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
 		 * Don't read beyond what we locked. This also limits the page
 		 * allocations that we'll do.
 		 */
-		disk_io_size = min(lockend + 1, iocb->ki_pos + encoded->len) - start;
-		count = start + disk_io_size - iocb->ki_pos;
+		disk_io_size = min(lockend + 1, offset + encoded->len) - start;
+		count = start + disk_io_size - offset;
 		encoded->len = count;
 		encoded->unencoded_len = count;
 		disk_io_size = ALIGN(disk_io_size, fs_info->sectorsize);
@@ -9339,16 +9339,13 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
 		if (ret != count)
 			ret = -EFAULT;
 	} else {
-		ret = btrfs_encoded_read_regular(iocb, iter, start, lockend,
-						 &cached_state, disk_bytenr,
-						 disk_io_size, count,
-						 encoded->compression,
+		ret = btrfs_encoded_read_regular(inode, offset, iter, start,
+						 lockend, &cached_state,
+						 disk_bytenr, disk_io_size,
+						 count, encoded->compression,
 						 &unlocked);
 	}
 
-out:
-	if (ret >= 0)
-		iocb->ki_pos += encoded->len;
 out_em:
 	free_extent_map(em);
 out_unlock_extent:
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e0a664b8a46a..406ed70814f5 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4520,7 +4520,6 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 	struct iovec *iov = iovstack;
 	struct iov_iter iter;
 	loff_t pos;
-	struct kiocb kiocb;
 	ssize_t ret;
 
 	if (!capable(CAP_SYS_ADMIN)) {
@@ -4571,10 +4570,7 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 	if (ret < 0)
 		goto out_iov;
 
-	init_sync_kiocb(&kiocb, file);
-	kiocb.ki_pos = pos;
-
-	ret = btrfs_encoded_read(&kiocb, &iter, &args);
+	ret = btrfs_encoded_read(file, pos, &iter, &args);
 	if (ret >= 0) {
 		fsnotify_access(file);
 		if (copy_to_user(argp + copy_end,
-- 
2.44.2

[PATCH 2/6] btrfs: store encoded read state in struct btrfs_encoded_read_private

[Mark Harmstone]

Move the various stack variables needed for encoded reads into struct
btrfs_encoded_read_private, so that we can split it into several
functions.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 fs/btrfs/btrfs_inode.h |  20 ++++-
 fs/btrfs/inode.c       | 170 +++++++++++++++++++++--------------------
 fs/btrfs/ioctl.c       |  60 ++++++++-------
 3 files changed, 135 insertions(+), 115 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index affe70929234..5cd4308bd337 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -605,9 +605,23 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 					  u64 file_offset, u64 disk_bytenr,
 					  u64 disk_io_size,
 					  struct page **pages);
-ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
-			   struct iov_iter *iter,
-			   struct btrfs_ioctl_encoded_io_args *encoded);
+
+struct btrfs_encoded_read_private {
+	wait_queue_head_t wait;
+	atomic_t pending;
+	blk_status_t status;
+	unsigned long nr_pages;
+	struct page **pages;
+	struct extent_state *cached_state;
+	size_t count;
+	struct iovec iovstack[UIO_FASTIOV];
+	struct iovec *iov;
+	struct iov_iter iter;
+	struct btrfs_ioctl_encoded_io_args args;
+	struct file *file;
+};
+
+ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv);
 ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
 			       const struct btrfs_ioctl_encoded_io_args *encoded);
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a0cc029d95ed..c1292e58366a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9078,12 +9078,6 @@ static ssize_t btrfs_encoded_read_inline(
 	return ret;
 }
 
-struct btrfs_encoded_read_private {
-	wait_queue_head_t wait;
-	atomic_t pending;
-	blk_status_t status;
-};
-
 static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
 {
 	struct btrfs_encoded_read_private *priv = bbio->private;
@@ -9104,33 +9098,31 @@ static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
 	bio_put(&bbio->bio);
 }
 
-int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
-					  u64 file_offset, u64 disk_bytenr,
-					  u64 disk_io_size, struct page **pages)
+static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
+					u64 file_offset, u64 disk_bytenr,
+					u64 disk_io_size,
+					struct btrfs_encoded_read_private *priv)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
-	struct btrfs_encoded_read_private priv = {
-		.pending = ATOMIC_INIT(1),
-	};
 	unsigned long i = 0;
 	struct btrfs_bio *bbio;
 
-	init_waitqueue_head(&priv.wait);
+	init_waitqueue_head(&priv->wait);
 
 	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info,
-			       btrfs_encoded_read_endio, &priv);
+			       btrfs_encoded_read_endio, priv);
 	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 	bbio->inode = inode;
 
 	do {
 		size_t bytes = min_t(u64, disk_io_size, PAGE_SIZE);
 
-		if (bio_add_page(&bbio->bio, pages[i], bytes, 0) < bytes) {
-			atomic_inc(&priv.pending);
+		if (bio_add_page(&bbio->bio, priv->pages[i], bytes, 0) < bytes) {
+			atomic_inc(&priv->pending);
 			btrfs_submit_bio(bbio, 0);
 
 			bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info,
-					       btrfs_encoded_read_endio, &priv);
+					       btrfs_encoded_read_endio, priv);
 			bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 			bbio->inode = inode;
 			continue;
@@ -9141,8 +9133,21 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 		disk_io_size -= bytes;
 	} while (disk_io_size);
 
-	atomic_inc(&priv.pending);
+	atomic_inc(&priv->pending);
 	btrfs_submit_bio(bbio, 0);
+}
+
+int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
+					  u64 file_offset, u64 disk_bytenr,
+					  u64 disk_io_size, struct page **pages)
+{
+	struct btrfs_encoded_read_private priv = {
+		.pending = ATOMIC_INIT(1),
+		.pages = pages,
+	};
+
+	_btrfs_encoded_read_regular_fill_pages(inode, file_offset, disk_bytenr,
+					       disk_io_size, &priv);
 
 	if (atomic_dec_return(&priv.pending))
 		io_wait_event(priv.wait, !atomic_read(&priv.pending));
@@ -9150,54 +9155,56 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 	return blk_status_to_errno(READ_ONCE(priv.status));
 }
 
-static ssize_t btrfs_encoded_read_regular(struct btrfs_inode *inode,
-					  loff_t offset, struct iov_iter *iter,
+static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *priv,
 					  u64 start, u64 lockend,
-					  struct extent_state **cached_state,
 					  u64 disk_bytenr, u64 disk_io_size,
-					  size_t count, bool compressed,
 					  bool *unlocked)
 {
+	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
 	struct extent_io_tree *io_tree = &inode->io_tree;
-	struct page **pages;
-	unsigned long nr_pages, i;
+	unsigned long i;
 	u64 cur;
 	size_t page_offset;
 	ssize_t ret;
 
-	nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
-	pages = kcalloc(nr_pages, sizeof(struct page *), GFP_NOFS);
-	if (!pages)
+	priv->nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
+	priv->pages = kcalloc(priv->nr_pages, sizeof(struct page *), GFP_NOFS);
+	if (!priv->pages)
 		return -ENOMEM;
-	ret = btrfs_alloc_page_array(nr_pages, pages, false);
+	ret = btrfs_alloc_page_array(priv->nr_pages, priv->pages, false);
 	if (ret) {
 		ret = -ENOMEM;
 		goto out;
 		}
 
-	ret = btrfs_encoded_read_regular_fill_pages(inode, start, disk_bytenr,
-						    disk_io_size, pages);
+	_btrfs_encoded_read_regular_fill_pages(inode, start, disk_bytenr,
+					       disk_io_size, priv);
+
+	if (atomic_dec_return(&priv->pending))
+		io_wait_event(priv->wait, !atomic_read(&priv->pending));
+
+	ret = blk_status_to_errno(READ_ONCE(priv->status));
 	if (ret)
 		goto out;
 
-	unlock_extent(io_tree, start, lockend, cached_state);
+	unlock_extent(io_tree, start, lockend, &priv->cached_state);
 	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
 	*unlocked = true;
 
-	if (compressed) {
+	if (priv->args.compression) {
 		i = 0;
 		page_offset = 0;
 	} else {
-		i = (offset - start) >> PAGE_SHIFT;
-		page_offset = (offset - start) & (PAGE_SIZE - 1);
+		i = (priv->args.offset - start) >> PAGE_SHIFT;
+		page_offset = (priv->args.offset - start) & (PAGE_SIZE - 1);
 	}
 	cur = 0;
-	while (cur < count) {
-		size_t bytes = min_t(size_t, count - cur,
+	while (cur < priv->count) {
+		size_t bytes = min_t(size_t, priv->count - cur,
 				     PAGE_SIZE - page_offset);
 
-		if (copy_page_to_iter(pages[i], page_offset, bytes,
-				      iter) != bytes) {
+		if (copy_page_to_iter(priv->pages[i], page_offset, bytes,
+				      &priv->iter) != bytes) {
 			ret = -EFAULT;
 			goto out;
 		}
@@ -9205,42 +9212,40 @@ static ssize_t btrfs_encoded_read_regular(struct btrfs_inode *inode,
 		cur += bytes;
 		page_offset = 0;
 	}
-	ret = count;
+	ret = priv->count;
 out:
-	for (i = 0; i < nr_pages; i++) {
-		if (pages[i])
-			__free_page(pages[i]);
+	for (i = 0; i < priv->nr_pages; i++) {
+		if (priv->pages[i])
+			__free_page(priv->pages[i]);
 	}
-	kfree(pages);
+	kfree(priv->pages);
 	return ret;
 }
 
-ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
-			   struct iov_iter *iter,
-			   struct btrfs_ioctl_encoded_io_args *encoded)
+ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv)
 {
-	struct btrfs_inode *inode = BTRFS_I(file_inode(file));
+	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct extent_io_tree *io_tree = &inode->io_tree;
 	ssize_t ret;
-	size_t count = iov_iter_count(iter);
 	u64 start, lockend, disk_bytenr, disk_io_size;
-	struct extent_state *cached_state = NULL;
 	struct extent_map *em;
 	bool unlocked = false;
 
-	file_accessed(file);
+	priv->count = iov_iter_count(&priv->iter);
+
+	file_accessed(priv->file);
 
 	btrfs_inode_lock(inode, BTRFS_ILOCK_SHARED);
 
-	if (offset >= inode->vfs_inode.i_size) {
+	if (priv->args.offset >= inode->vfs_inode.i_size) {
 		btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
 		return 0;
 	}
-	start = ALIGN_DOWN(offset, fs_info->sectorsize);
+	start = ALIGN_DOWN(priv->args.offset, fs_info->sectorsize);
 	/*
-	 * We don't know how long the extent containing offset is, but if
-	 * it's compressed we know that it won't be longer than this.
+	 * We don't know how long the extent containing priv->args.offset is,
+	 * but if it's compressed we know that it won't be longer than this.
 	 */
 	lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
 
@@ -9251,13 +9256,13 @@ ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
 					       lockend - start + 1);
 		if (ret)
 			goto out_unlock_inode;
-		lock_extent(io_tree, start, lockend, &cached_state);
+		lock_extent(io_tree, start, lockend, &priv->cached_state);
 		ordered = btrfs_lookup_ordered_range(inode, start,
 						     lockend - start + 1);
 		if (!ordered)
 			break;
 		btrfs_put_ordered_extent(ordered);
-		unlock_extent(io_tree, start, lockend, &cached_state);
+		unlock_extent(io_tree, start, lockend, &priv->cached_state);
 		cond_resched();
 	}
 
@@ -9276,10 +9281,11 @@ ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
 		 */
 		free_extent_map(em);
 		em = NULL;
-		ret = btrfs_encoded_read_inline(inode, offset, iter, start,
-						lockend, &cached_state,
-						extent_start, count, encoded,
-						&unlocked);
+		ret = btrfs_encoded_read_inline(inode, priv->args.offset,
+						&priv->iter, start,
+						lockend, &priv->cached_state,
+						extent_start, priv->count,
+						&priv->args, &unlocked);
 		goto out_em;
 	}
 
@@ -9287,62 +9293,60 @@ ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
 	 * We only want to return up to EOF even if the extent extends beyond
 	 * that.
 	 */
-	encoded->len = min_t(u64, extent_map_end(em),
-			     inode->vfs_inode.i_size) - offset;
+	priv->args.len = min_t(u64, extent_map_end(em),
+			     inode->vfs_inode.i_size) - priv->args.offset;
 	if (em->disk_bytenr == EXTENT_MAP_HOLE ||
 	    (em->flags & EXTENT_FLAG_PREALLOC)) {
 		disk_bytenr = EXTENT_MAP_HOLE;
-		count = min_t(u64, count, encoded->len);
-		encoded->len = count;
-		encoded->unencoded_len = count;
+		priv->count = min_t(u64, priv->count, priv->args.len);
+		priv->args.len = priv->count;
+		priv->args.unencoded_len = priv->count;
 	} else if (extent_map_is_compressed(em)) {
 		disk_bytenr = em->disk_bytenr;
 		/*
 		 * Bail if the buffer isn't large enough to return the whole
 		 * compressed extent.
 		 */
-		if (em->disk_num_bytes > count) {
+		if (em->disk_num_bytes > priv->count) {
 			ret = -ENOBUFS;
 			goto out_em;
 		}
 		disk_io_size = em->disk_num_bytes;
-		count = em->disk_num_bytes;
-		encoded->unencoded_len = em->ram_bytes;
-		encoded->unencoded_offset = offset - (em->start - em->offset);
+		priv->count = em->disk_num_bytes;
+		priv->args.unencoded_len = em->ram_bytes;
+		priv->args.unencoded_offset = priv->args.offset - (em->start - em->offset);
 		ret = btrfs_encoded_io_compression_from_extent(fs_info,
 							       extent_map_compression(em));
 		if (ret < 0)
 			goto out_em;
-		encoded->compression = ret;
+		priv->args.compression = ret;
 	} else {
 		disk_bytenr = extent_map_block_start(em) + (start - em->start);
-		if (encoded->len > count)
-			encoded->len = count;
+		if (priv->args.len > priv->count)
+			priv->args.len = priv->count;
 		/*
 		 * Don't read beyond what we locked. This also limits the page
 		 * allocations that we'll do.
 		 */
-		disk_io_size = min(lockend + 1, offset + encoded->len) - start;
-		count = start + disk_io_size - offset;
-		encoded->len = count;
-		encoded->unencoded_len = count;
+		disk_io_size = min(lockend + 1, priv->args.offset + priv->args.len) - start;
+		priv->count = start + disk_io_size - priv->args.offset;
+		priv->args.len = priv->count;
+		priv->args.unencoded_len = priv->count;
 		disk_io_size = ALIGN(disk_io_size, fs_info->sectorsize);
 	}
 	free_extent_map(em);
 	em = NULL;
 
 	if (disk_bytenr == EXTENT_MAP_HOLE) {
-		unlock_extent(io_tree, start, lockend, &cached_state);
+		unlock_extent(io_tree, start, lockend, &priv->cached_state);
 		btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
 		unlocked = true;
-		ret = iov_iter_zero(count, iter);
-		if (ret != count)
+		ret = iov_iter_zero(priv->count, &priv->iter);
+		if (ret != priv->count)
 			ret = -EFAULT;
 	} else {
-		ret = btrfs_encoded_read_regular(inode, offset, iter, start,
-						 lockend, &cached_state,
+		ret = btrfs_encoded_read_regular(priv, start, lockend,
 						 disk_bytenr, disk_io_size,
-						 count, encoded->compression,
 						 &unlocked);
 	}
 
@@ -9350,7 +9354,7 @@ ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
 	free_extent_map(em);
 out_unlock_extent:
 	if (!unlocked)
-		unlock_extent(io_tree, start, lockend, &cached_state);
+		unlock_extent(io_tree, start, lockend, &priv->cached_state);
 out_unlock_inode:
 	if (!unlocked)
 		btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 406ed70814f5..770bd609f386 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4512,19 +4512,19 @@ static int _btrfs_ioctl_send(struct btrfs_inode *inode, void __user *argp, bool
 static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 				    bool compat)
 {
-	struct btrfs_ioctl_encoded_io_args args = { 0 };
 	size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args,
 					     flags);
 	size_t copy_end;
-	struct iovec iovstack[UIO_FASTIOV];
-	struct iovec *iov = iovstack;
-	struct iov_iter iter;
 	loff_t pos;
 	ssize_t ret;
+	struct btrfs_encoded_read_private priv = {
+		.pending = ATOMIC_INIT(1),
+		.file = file,
+	};
 
 	if (!capable(CAP_SYS_ADMIN)) {
 		ret = -EPERM;
-		goto out_acct;
+		goto out;
 	}
 
 	if (compat) {
@@ -4535,53 +4535,55 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 				       flags);
 		if (copy_from_user(&args32, argp, copy_end)) {
 			ret = -EFAULT;
-			goto out_acct;
+			goto out;
 		}
-		args.iov = compat_ptr(args32.iov);
-		args.iovcnt = args32.iovcnt;
-		args.offset = args32.offset;
-		args.flags = args32.flags;
+		priv.args.iov = compat_ptr(args32.iov);
+		priv.args.iovcnt = args32.iovcnt;
+		priv.args.offset = args32.offset;
+		priv.args.flags = args32.flags;
 #else
 		return -ENOTTY;
 #endif
 	} else {
 		copy_end = copy_end_kernel;
-		if (copy_from_user(&args, argp, copy_end)) {
+		if (copy_from_user(&priv.args, argp, copy_end)) {
 			ret = -EFAULT;
-			goto out_acct;
+			goto out;
 		}
 	}
-	if (args.flags != 0) {
+	if (priv.args.flags != 0) {
 		ret = -EINVAL;
-		goto out_acct;
+		goto out;
 	}
 
-	ret = import_iovec(ITER_DEST, args.iov, args.iovcnt, ARRAY_SIZE(iovstack),
-			   &iov, &iter);
-	if (ret < 0)
-		goto out_acct;
+	priv.iov = priv.iovstack;
+	ret = import_iovec(ITER_DEST, priv.args.iov, priv.args.iovcnt,
+			   ARRAY_SIZE(priv.iovstack), &priv.iov, &priv.iter);
+	if (ret < 0) {
+		priv.iov = NULL;
+		goto out;
+	}
 
-	if (iov_iter_count(&iter) == 0) {
+	if (iov_iter_count(&priv.iter) == 0) {
 		ret = 0;
-		goto out_iov;
+		goto out;
 	}
-	pos = args.offset;
-	ret = rw_verify_area(READ, file, &pos, args.len);
+	pos = priv.args.offset;
+	ret = rw_verify_area(READ, file, &pos, priv.args.len);
 	if (ret < 0)
-		goto out_iov;
+		goto out;
 
-	ret = btrfs_encoded_read(file, pos, &iter, &args);
+	ret = btrfs_encoded_read(&priv);
 	if (ret >= 0) {
 		fsnotify_access(file);
 		if (copy_to_user(argp + copy_end,
-				 (char *)&args + copy_end_kernel,
-				 sizeof(args) - copy_end_kernel))
+				 (char *)&priv.args + copy_end_kernel,
+				 sizeof(priv.args) - copy_end_kernel))
 			ret = -EFAULT;
 	}
 
-out_iov:
-	kfree(iov);
-out_acct:
+out:
+	kfree(priv.iov);
 	if (ret > 0)
 		add_rchar(current, ret);
 	inc_syscr(current);
-- 
2.44.2

[PATCH 3/6] btrfs: add btrfs_encoded_read_finish

[Mark Harmstone]

Move the end of btrfs_ioctl_encoded_read, responsible for copying to
userspace and cleanup, into its own function.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 fs/btrfs/btrfs_inode.h |  1 +
 fs/btrfs/inode.c       | 29 +++++++++++-----------------
 fs/btrfs/ioctl.c       | 43 ++++++++++++++++++++++++++++++------------
 3 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 5cd4308bd337..f4d77c3bb544 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -619,6 +619,7 @@ struct btrfs_encoded_read_private {
 	struct iov_iter iter;
 	struct btrfs_ioctl_encoded_io_args args;
 	struct file *file;
+	void __user *copy_out;
 };
 
 ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c1292e58366a..1e53977a4854 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9169,13 +9169,13 @@ static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *pri
 
 	priv->nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
 	priv->pages = kcalloc(priv->nr_pages, sizeof(struct page *), GFP_NOFS);
-	if (!priv->pages)
+	if (!priv->pages) {
+		priv->nr_pages = 0;
 		return -ENOMEM;
+	}
 	ret = btrfs_alloc_page_array(priv->nr_pages, priv->pages, false);
-	if (ret) {
-		ret = -ENOMEM;
-		goto out;
-		}
+	if (ret)
+		return -ENOMEM;
 
 	_btrfs_encoded_read_regular_fill_pages(inode, start, disk_bytenr,
 					       disk_io_size, priv);
@@ -9185,7 +9185,7 @@ static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *pri
 
 	ret = blk_status_to_errno(READ_ONCE(priv->status));
 	if (ret)
-		goto out;
+		return ret;
 
 	unlock_extent(io_tree, start, lockend, &priv->cached_state);
 	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
@@ -9204,22 +9204,15 @@ static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *pri
 				     PAGE_SIZE - page_offset);
 
 		if (copy_page_to_iter(priv->pages[i], page_offset, bytes,
-				      &priv->iter) != bytes) {
-			ret = -EFAULT;
-			goto out;
-		}
+				      &priv->iter) != bytes)
+			return -EFAULT;
+
 		i++;
 		cur += bytes;
 		page_offset = 0;
 	}
-	ret = priv->count;
-out:
-	for (i = 0; i < priv->nr_pages; i++) {
-		if (priv->pages[i])
-			__free_page(priv->pages[i]);
-	}
-	kfree(priv->pages);
-	return ret;
+
+	return priv->count;
 }
 
 ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 770bd609f386..3fa661322c26 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4509,6 +4509,34 @@ static int _btrfs_ioctl_send(struct btrfs_inode *inode, void __user *argp, bool
 	return ret;
 }
 
+static ssize_t btrfs_encoded_read_finish(struct btrfs_encoded_read_private *priv,
+					 ssize_t ret)
+{
+	size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args,
+					     flags);
+	unsigned long i;
+
+	if (ret >= 0) {
+		fsnotify_access(priv->file);
+		if (copy_to_user(priv->copy_out,
+				 (char *)&priv->args + copy_end_kernel,
+				 sizeof(priv->args) - copy_end_kernel))
+			ret = -EFAULT;
+	}
+
+	for (i = 0; i < priv->nr_pages; i++) {
+		if (priv->pages[i])
+			__free_page(priv->pages[i]);
+	}
+	kfree(priv->pages);
+	kfree(priv->iov);
+
+	if (ret > 0)
+		add_rchar(current, ret);
+	inc_syscr(current);
+	return ret;
+}
+
 static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 				    bool compat)
 {
@@ -4573,21 +4601,12 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 	if (ret < 0)
 		goto out;
 
+	priv.copy_out = argp + copy_end;
+
 	ret = btrfs_encoded_read(&priv);
-	if (ret >= 0) {
-		fsnotify_access(file);
-		if (copy_to_user(argp + copy_end,
-				 (char *)&priv.args + copy_end_kernel,
-				 sizeof(priv.args) - copy_end_kernel))
-			ret = -EFAULT;
-	}
 
 out:
-	kfree(priv.iov);
-	if (ret > 0)
-		add_rchar(current, ret);
-	inc_syscr(current);
-	return ret;
+	return btrfs_encoded_read_finish(&priv, ret);
 }
 
 static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp, bool compat)
-- 
2.44.2

[PATCH 4/6] btrfs: add btrfs_prepare_encoded_read

[Mark Harmstone]

Move the beginning of btrfs_ioctl_encoded_read, responsible for
initialization of priv and validation, into its own function.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 fs/btrfs/ioctl.c | 84 ++++++++++++++++++++++++++----------------------
 1 file changed, 46 insertions(+), 38 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 3fa661322c26..d2658508e055 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4537,23 +4537,23 @@ static ssize_t btrfs_encoded_read_finish(struct btrfs_encoded_read_private *priv
 	return ret;
 }
 
-static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
-				    bool compat)
+static ssize_t btrfs_prepare_encoded_read(struct btrfs_encoded_read_private *priv,
+					  struct file *file, bool compat,
+					  void __user *argp)
 {
 	size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args,
 					     flags);
 	size_t copy_end;
 	loff_t pos;
 	ssize_t ret;
-	struct btrfs_encoded_read_private priv = {
-		.pending = ATOMIC_INIT(1),
-		.file = file,
-	};
 
-	if (!capable(CAP_SYS_ADMIN)) {
-		ret = -EPERM;
-		goto out;
-	}
+	memset(priv, 0, sizeof(*priv));
+
+	atomic_set(&priv->pending, 1);
+	priv->file = file;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
 
 	if (compat) {
 #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
@@ -4561,47 +4561,55 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 
 		copy_end = offsetofend(struct btrfs_ioctl_encoded_io_args_32,
 				       flags);
-		if (copy_from_user(&args32, argp, copy_end)) {
-			ret = -EFAULT;
-			goto out;
-		}
-		priv.args.iov = compat_ptr(args32.iov);
-		priv.args.iovcnt = args32.iovcnt;
-		priv.args.offset = args32.offset;
-		priv.args.flags = args32.flags;
+		if (copy_from_user(&args32, argp, copy_end))
+			return -EFAULT;
+
+		priv->args.iov = compat_ptr(args32.iov);
+		priv->args.iovcnt = args32.iovcnt;
+		priv->args.offset = args32.offset;
+		priv->args.flags = args32.flags;
 #else
 		return -ENOTTY;
 #endif
 	} else {
 		copy_end = copy_end_kernel;
-		if (copy_from_user(&priv.args, argp, copy_end)) {
-			ret = -EFAULT;
-			goto out;
-		}
-	}
-	if (priv.args.flags != 0) {
-		ret = -EINVAL;
-		goto out;
+		if (copy_from_user(&priv->args, argp, copy_end))
+			return -EFAULT;
 	}
 
-	priv.iov = priv.iovstack;
-	ret = import_iovec(ITER_DEST, priv.args.iov, priv.args.iovcnt,
-			   ARRAY_SIZE(priv.iovstack), &priv.iov, &priv.iter);
+	if (priv->args.flags != 0)
+		return -EINVAL;
+
+	priv->iov = priv->iovstack;
+	ret = import_iovec(ITER_DEST, priv->args.iov, priv->args.iovcnt,
+			   ARRAY_SIZE(priv->iovstack), &priv->iov, &priv->iter);
 	if (ret < 0) {
-		priv.iov = NULL;
-		goto out;
+		priv->iov = NULL;
+		return ret;
 	}
 
-	if (iov_iter_count(&priv.iter) == 0) {
-		ret = 0;
-		goto out;
-	}
-	pos = priv.args.offset;
-	ret = rw_verify_area(READ, file, &pos, priv.args.len);
+	pos = priv->args.offset;
+	ret = rw_verify_area(READ, priv->file, &pos, priv->args.len);
 	if (ret < 0)
+		return ret;
+
+	priv->copy_out = argp + copy_end;
+
+	return 0;
+}
+
+static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
+				    bool compat)
+{
+	ssize_t ret;
+	struct btrfs_encoded_read_private priv;
+
+	ret = btrfs_prepare_encoded_read(&priv, file, compat, argp);
+	if (ret)
 		goto out;
 
-	priv.copy_out = argp + copy_end;
+	if (iov_iter_count(&priv.iter) == 0)
+		goto out;
 
 	ret = btrfs_encoded_read(&priv);
 
-- 
2.44.2

[PATCH 5/6] btrfs: move wait out of btrfs_encoded_read

[Mark Harmstone]

Makes it so that if btrfs_encoded_read has launched a bio, rather than
waiting for it to complete it leaves the extent and inode locked and returns
-EIOCBQUEUED. The caller is responsible for waiting on the bio, and
calling the completion code in the new btrfs_encoded_read_regular_end.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 fs/btrfs/btrfs_inode.h |  1 +
 fs/btrfs/inode.c       | 81 +++++++++++++++++++++++-------------------
 fs/btrfs/ioctl.c       |  8 +++++
 3 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index f4d77c3bb544..a5d786c6d7d4 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -623,6 +623,7 @@ struct btrfs_encoded_read_private {
 };
 
 ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv);
+ssize_t btrfs_encoded_read_regular_end(struct btrfs_encoded_read_private *priv);
 ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
 			       const struct btrfs_ioctl_encoded_io_args *encoded);
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1e53977a4854..1bd4c74e8c51 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8999,7 +8999,7 @@ static ssize_t btrfs_encoded_read_inline(
 				struct extent_state **cached_state,
 				u64 extent_start, size_t count,
 				struct btrfs_ioctl_encoded_io_args *encoded,
-				bool *unlocked)
+				bool *need_unlock)
 {
 	struct btrfs_root *root = inode->root;
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -9067,7 +9067,7 @@ static ssize_t btrfs_encoded_read_inline(
 	btrfs_release_path(path);
 	unlock_extent(io_tree, start, lockend, cached_state);
 	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
-	*unlocked = true;
+	*need_unlock = false;
 
 	ret = copy_to_iter(tmp, count, iter);
 	if (ret != count)
@@ -9155,42 +9155,26 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 	return blk_status_to_errno(READ_ONCE(priv.status));
 }
 
-static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *priv,
-					  u64 start, u64 lockend,
-					  u64 disk_bytenr, u64 disk_io_size,
-					  bool *unlocked)
+ssize_t btrfs_encoded_read_regular_end(struct btrfs_encoded_read_private *priv)
 {
-	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
-	struct extent_io_tree *io_tree = &inode->io_tree;
+	u64 cur, start, lockend;
 	unsigned long i;
-	u64 cur;
 	size_t page_offset;
-	ssize_t ret;
-
-	priv->nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
-	priv->pages = kcalloc(priv->nr_pages, sizeof(struct page *), GFP_NOFS);
-	if (!priv->pages) {
-		priv->nr_pages = 0;
-		return -ENOMEM;
-	}
-	ret = btrfs_alloc_page_array(priv->nr_pages, priv->pages, false);
-	if (ret)
-		return -ENOMEM;
+	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
+	struct btrfs_fs_info *fs_info = inode->root->fs_info;
+	struct extent_io_tree *io_tree = &inode->io_tree;
+	int ret;
 
-	_btrfs_encoded_read_regular_fill_pages(inode, start, disk_bytenr,
-					       disk_io_size, priv);
+	start = ALIGN_DOWN(priv->args.offset, fs_info->sectorsize);
+	lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
 
-	if (atomic_dec_return(&priv->pending))
-		io_wait_event(priv->wait, !atomic_read(&priv->pending));
+	unlock_extent(io_tree, start, lockend, &priv->cached_state);
+	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
 
 	ret = blk_status_to_errno(READ_ONCE(priv->status));
 	if (ret)
 		return ret;
 
-	unlock_extent(io_tree, start, lockend, &priv->cached_state);
-	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
-	*unlocked = true;
-
 	if (priv->args.compression) {
 		i = 0;
 		page_offset = 0;
@@ -9215,6 +9199,30 @@ static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *pri
 	return priv->count;
 }
 
+static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *priv,
+					  u64 disk_bytenr, u64 disk_io_size)
+{
+	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
+	struct btrfs_fs_info *fs_info = inode->root->fs_info;
+	u64 start = ALIGN_DOWN(priv->args.offset, fs_info->sectorsize);
+	ssize_t ret;
+
+	priv->nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
+	priv->pages = kcalloc(priv->nr_pages, sizeof(struct page *), GFP_NOFS);
+	if (!priv->pages) {
+		priv->nr_pages = 0;
+		return -ENOMEM;
+	}
+	ret = btrfs_alloc_page_array(priv->nr_pages, priv->pages, false);
+	if (ret)
+		return -ENOMEM;
+
+	_btrfs_encoded_read_regular_fill_pages(inode, start, disk_bytenr,
+					       disk_io_size, priv);
+
+	return -EIOCBQUEUED;
+}
+
 ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv)
 {
 	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
@@ -9223,7 +9231,7 @@ ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv)
 	ssize_t ret;
 	u64 start, lockend, disk_bytenr, disk_io_size;
 	struct extent_map *em;
-	bool unlocked = false;
+	bool need_unlock = true;
 
 	priv->count = iov_iter_count(&priv->iter);
 
@@ -9278,7 +9286,7 @@ ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv)
 						&priv->iter, start,
 						lockend, &priv->cached_state,
 						extent_start, priv->count,
-						&priv->args, &unlocked);
+						&priv->args, &need_unlock);
 		goto out_em;
 	}
 
@@ -9333,23 +9341,24 @@ ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv)
 	if (disk_bytenr == EXTENT_MAP_HOLE) {
 		unlock_extent(io_tree, start, lockend, &priv->cached_state);
 		btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
-		unlocked = true;
+		need_unlock = false;
 		ret = iov_iter_zero(priv->count, &priv->iter);
 		if (ret != priv->count)
 			ret = -EFAULT;
 	} else {
-		ret = btrfs_encoded_read_regular(priv, start, lockend,
-						 disk_bytenr, disk_io_size,
-						 &unlocked);
+		ret = btrfs_encoded_read_regular(priv, disk_bytenr, disk_io_size);
+
+		if (ret == -EIOCBQUEUED)
+			need_unlock = false;
 	}
 
 out_em:
 	free_extent_map(em);
 out_unlock_extent:
-	if (!unlocked)
+	if (need_unlock)
 		unlock_extent(io_tree, start, lockend, &priv->cached_state);
 out_unlock_inode:
-	if (!unlocked)
+	if (need_unlock)
 		btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
 	return ret;
 }
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index d2658508e055..c1886209933a 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4516,6 +4516,9 @@ static ssize_t btrfs_encoded_read_finish(struct btrfs_encoded_read_private *priv
 					     flags);
 	unsigned long i;
 
+	if (ret == -EIOCBQUEUED)
+		ret = btrfs_encoded_read_regular_end(priv);
+
 	if (ret >= 0) {
 		fsnotify_access(priv->file);
 		if (copy_to_user(priv->copy_out,
@@ -4613,6 +4616,11 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
 
 	ret = btrfs_encoded_read(&priv);
 
+	if (ret == -EIOCBQUEUED) {
+		if (atomic_dec_return(&priv.pending))
+			io_wait_event(priv.wait, !atomic_read(&priv.pending));
+	}
+
 out:
 	return btrfs_encoded_read_finish(&priv, ret);
 }
-- 
2.44.2

[PATCH 6/6] btrfs: add io_uring interface for encoded reads

[Mark Harmstone]

Adds an io_uring interface for asynchronous encoded reads, using the
same interface as for the ioctl. To use this you would use an SQE opcode
of IORING_OP_URING_CMD, the cmd_op would be BTRFS_IOC_ENCODED_READ, and
addr would point to the userspace address of the
btrfs_ioctl_encoded_io_args struct. As with the ioctl, you need to have
CAP_SYS_ADMIN for this to work.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 fs/btrfs/btrfs_inode.h |  2 ++
 fs/btrfs/file.c        |  1 +
 fs/btrfs/inode.c       | 57 ++++++++++++++++++++++++++++++++++++----
 fs/btrfs/ioctl.c       | 59 ++++++++++++++++++++++++++++++++++++++++--
 fs/btrfs/ioctl.h       |  3 +++
 5 files changed, 115 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index a5d786c6d7d4..62e5757d3ba2 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -620,6 +620,8 @@ struct btrfs_encoded_read_private {
 	struct btrfs_ioctl_encoded_io_args args;
 	struct file *file;
 	void __user *copy_out;
+	struct io_uring_cmd *cmd;
+	unsigned int issue_flags;
 };
 
 ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 9914419f3b7d..2db76422d126 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -3769,6 +3769,7 @@ const struct file_operations btrfs_file_operations = {
 	.compat_ioctl	= btrfs_compat_ioctl,
 #endif
 	.remap_file_range = btrfs_remap_file_range,
+	.uring_cmd	= btrfs_uring_cmd,
 	.fop_flags	= FOP_BUFFER_RASYNC | FOP_BUFFER_WASYNC,
 };
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 1bd4c74e8c51..e4458168c340 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -34,6 +34,7 @@
 #include <linux/iomap.h>
 #include <asm/unaligned.h>
 #include <linux/fsverity.h>
+#include <linux/io_uring/cmd.h>
 #include "misc.h"
 #include "ctree.h"
 #include "disk-io.h"
@@ -9078,7 +9079,7 @@ static ssize_t btrfs_encoded_read_inline(
 	return ret;
 }
 
-static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
+static void btrfs_encoded_read_ioctl_endio(struct btrfs_bio *bbio)
 {
 	struct btrfs_encoded_read_private *priv = bbio->private;
 
@@ -9098,6 +9099,47 @@ static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
 	bio_put(&bbio->bio);
 }
 
+static inline struct btrfs_encoded_read_private *btrfs_uring_encoded_read_pdu(
+		struct io_uring_cmd *cmd)
+{
+	return *(struct btrfs_encoded_read_private **)cmd->pdu;
+}
+static void btrfs_finish_uring_encoded_read(struct io_uring_cmd *cmd,
+					    unsigned int issue_flags)
+{
+	struct btrfs_encoded_read_private *priv;
+	ssize_t ret;
+
+	priv = btrfs_uring_encoded_read_pdu(cmd);
+	ret = btrfs_encoded_read_finish(priv, -EIOCBQUEUED);
+
+	io_uring_cmd_done(priv->cmd, ret, 0, priv->issue_flags);
+
+	kfree(priv);
+}
+
+static void btrfs_encoded_read_uring_endio(struct btrfs_bio *bbio)
+{
+	struct btrfs_encoded_read_private *priv = bbio->private;
+
+	if (bbio->bio.bi_status) {
+		/*
+		 * The memory barrier implied by the atomic_dec_return() here
+		 * pairs with the memory barrier implied by the
+		 * atomic_dec_return() or io_wait_event() in
+		 * btrfs_encoded_read_regular_fill_pages() to ensure that this
+		 * write is observed before the load of status in
+		 * btrfs_encoded_read_regular_fill_pages().
+		 */
+		WRITE_ONCE(priv->status, bbio->bio.bi_status);
+	}
+	if (!atomic_dec_return(&priv->pending)) {
+		io_uring_cmd_complete_in_task(priv->cmd,
+					      btrfs_finish_uring_encoded_read);
+	}
+	bio_put(&bbio->bio);
+}
+
 static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 					u64 file_offset, u64 disk_bytenr,
 					u64 disk_io_size,
@@ -9106,11 +9148,16 @@ static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	unsigned long i = 0;
 	struct btrfs_bio *bbio;
+	btrfs_bio_end_io_t cb;
 
-	init_waitqueue_head(&priv->wait);
+	if (priv->cmd) {
+		cb = btrfs_encoded_read_uring_endio;
+	} else {
+		init_waitqueue_head(&priv->wait);
+		cb = btrfs_encoded_read_ioctl_endio;
+	}
 
-	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info,
-			       btrfs_encoded_read_endio, priv);
+	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info, cb, priv);
 	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 	bbio->inode = inode;
 
@@ -9122,7 +9169,7 @@ static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
 			btrfs_submit_bio(bbio, 0);
 
 			bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info,
-					       btrfs_encoded_read_endio, priv);
+					       cb, priv);
 			bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
 			bbio->inode = inode;
 			continue;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c1886209933a..85a512a9ca64 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -29,6 +29,7 @@
 #include <linux/fileattr.h>
 #include <linux/fsverity.h>
 #include <linux/sched/xacct.h>
+#include <linux/io_uring/cmd.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "export.h"
@@ -4509,8 +4510,8 @@ static int _btrfs_ioctl_send(struct btrfs_inode *inode, void __user *argp, bool
 	return ret;
 }
 
-static ssize_t btrfs_encoded_read_finish(struct btrfs_encoded_read_private *priv,
-					 ssize_t ret)
+ssize_t btrfs_encoded_read_finish(struct btrfs_encoded_read_private *priv,
+				  ssize_t ret)
 {
 	size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args,
 					     flags);
@@ -4725,6 +4726,60 @@ static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp, bool
 	return ret;
 }
 
+static void btrfs_uring_encoded_read(struct io_uring_cmd *cmd,
+				     unsigned int issue_flags)
+{
+	ssize_t ret;
+	void __user *argp = (void __user *)(uintptr_t)cmd->sqe->addr;
+	struct btrfs_encoded_read_private *priv;
+	bool compat = issue_flags & IO_URING_F_COMPAT;
+
+	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out_uring;
+	}
+
+	ret = btrfs_prepare_encoded_read(priv, cmd->file, compat, argp);
+	if (ret)
+		goto out_finish;
+
+	if (iov_iter_count(&priv->iter) == 0) {
+		ret = 0;
+		goto out_finish;
+	}
+
+	*(struct btrfs_encoded_read_private **)cmd->pdu = priv;
+	priv->cmd = cmd;
+	priv->issue_flags = issue_flags;
+	ret = btrfs_encoded_read(priv);
+
+	if (ret == -EIOCBQUEUED && atomic_dec_return(&priv->pending))
+		return;
+
+out_finish:
+	ret = btrfs_encoded_read_finish(priv, ret);
+	kfree(priv);
+
+out_uring:
+	io_uring_cmd_done(cmd, ret, 0, issue_flags);
+}
+
+int btrfs_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
+{
+	switch (cmd->cmd_op) {
+	case BTRFS_IOC_ENCODED_READ:
+#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
+	case BTRFS_IOC_ENCODED_READ_32:
+#endif
+		btrfs_uring_encoded_read(cmd, issue_flags);
+		return -EIOCBQUEUED;
+	}
+
+	io_uring_cmd_done(cmd, -EINVAL, 0, issue_flags);
+	return -EIOCBQUEUED;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 19cd26b0244a..9d1522de79d3 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -22,5 +22,8 @@ void btrfs_sync_inode_flags_to_i_flags(struct inode *inode);
 int __pure btrfs_is_empty_uuid(const u8 *uuid);
 void btrfs_update_ioctl_balance_args(struct btrfs_fs_info *fs_info,
 				     struct btrfs_ioctl_balance_args *bargs);
+int btrfs_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags);
+ssize_t btrfs_encoded_read_finish(struct btrfs_encoded_read_private *priv,
+				  ssize_t ret);
 
 #endif
-- 
2.44.2

Re: [PATCH 2/6] btrfs: store encoded read state in struct btrfs_encoded_read_private

[David Sterba]

On Fri, Aug 23, 2024 at 05:27:44PM +0100, Mark Harmstone wrote:
> Move the various stack variables needed for encoded reads into struct
> btrfs_encoded_read_private, so that we can split it into several
> functions.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
>  fs/btrfs/btrfs_inode.h |  20 ++++-
>  fs/btrfs/inode.c       | 170 +++++++++++++++++++++--------------------
>  fs/btrfs/ioctl.c       |  60 ++++++++-------
>  3 files changed, 135 insertions(+), 115 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index affe70929234..5cd4308bd337 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -605,9 +605,23 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>  					  u64 file_offset, u64 disk_bytenr,
>  					  u64 disk_io_size,
>  					  struct page **pages);
> -ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
> -			   struct iov_iter *iter,
> -			   struct btrfs_ioctl_encoded_io_args *encoded);
> +
> +struct btrfs_encoded_read_private {
> +	wait_queue_head_t wait;
> +	atomic_t pending;
> +	blk_status_t status;
> +	unsigned long nr_pages;
> +	struct page **pages;
> +	struct extent_state *cached_state;
> +	size_t count;
> +	struct iovec iovstack[UIO_FASTIOV];
> +	struct iovec *iov;
> +	struct iov_iter iter;
> +	struct btrfs_ioctl_encoded_io_args args;
> +	struct file *file;
> +};
> +
> +ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv);
>  ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
>  			       const struct btrfs_ioctl_encoded_io_args *encoded);
>  
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index a0cc029d95ed..c1292e58366a 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -9078,12 +9078,6 @@ static ssize_t btrfs_encoded_read_inline(
>  	return ret;
>  }
>  
> -struct btrfs_encoded_read_private {
> -	wait_queue_head_t wait;
> -	atomic_t pending;
> -	blk_status_t status;
> -};
> -
>  static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
>  {
>  	struct btrfs_encoded_read_private *priv = bbio->private;
> @@ -9104,33 +9098,31 @@ static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
>  	bio_put(&bbio->bio);
>  }
>  
> -int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
> -					  u64 file_offset, u64 disk_bytenr,
> -					  u64 disk_io_size, struct page **pages)
> +static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,

Please don't use underscores in function names

https://btrfs.readthedocs.io/en/latest/dev/Development-notes.html#function-declarations

Re: [PATCH 2/6] btrfs: store encoded read state in struct btrfs_encoded_read_private

[David Sterba]

On Fri, Aug 23, 2024 at 05:27:44PM +0100, Mark Harmstone wrote:
> Move the various stack variables needed for encoded reads into struct
> btrfs_encoded_read_private, so that we can split it into several
> functions.

Moving local variables makes sense in some cases but I don't see reason
to move all of them.

> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
>  fs/btrfs/btrfs_inode.h |  20 ++++-
>  fs/btrfs/inode.c       | 170 +++++++++++++++++++++--------------------
>  fs/btrfs/ioctl.c       |  60 ++++++++-------
>  3 files changed, 135 insertions(+), 115 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index affe70929234..5cd4308bd337 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -605,9 +605,23 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>  					  u64 file_offset, u64 disk_bytenr,
>  					  u64 disk_io_size,
>  					  struct page **pages);
> -ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
> -			   struct iov_iter *iter,
> -			   struct btrfs_ioctl_encoded_io_args *encoded);
> +
> +struct btrfs_encoded_read_private {
> +	wait_queue_head_t wait;
> +	atomic_t pending;
> +	blk_status_t status;
> +	unsigned long nr_pages;
> +	struct page **pages;
> +	struct extent_state *cached_state;

The cached state is used as a local variable that is not reused by other
functions that also take the private structure, so what's the reason to
store it here?

> +	size_t count;
> +	struct iovec iovstack[UIO_FASTIOV];
> +	struct iovec *iov;

Same, this is used in the leaf functions and not passed around.

> +	struct iov_iter iter;
> +	struct btrfs_ioctl_encoded_io_args args;
> +	struct file *file;
> +};

As a coding pattern, the structure should store data that would be
otherwise passed as parameters or repeatedly derived from the
parameters.

Re: [PATCH 1/6] btrfs: remove iocb from btrfs_encoded_read

[David Sterba]

On Fri, Aug 23, 2024 at 05:27:43PM +0100, Mark Harmstone wrote:
> We initialize a struct kiocb for encoded reads, but this never gets
> passed outside the driver. Simplify things by replacing it with a file,
> offset pair.

Please change the subject so it matches the description, so
"btrfs: replace iocb with file and offset in btrfs_encoded_read()"

Re: [PATCH 6/6] btrfs: add io_uring interface for encoded reads

[Pavel Begunkov]

On 8/23/24 17:27, Mark Harmstone wrote:
> Adds an io_uring interface for asynchronous encoded reads, using the
> same interface as for the ioctl. To use this you would use an SQE opcode
> of IORING_OP_URING_CMD, the cmd_op would be BTRFS_IOC_ENCODED_READ, and
> addr would point to the userspace address of the
> btrfs_ioctl_encoded_io_args struct. As with the ioctl, you need to have
> CAP_SYS_ADMIN for this to work.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
...
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index a5d786c6d7d4..62e5757d3ba2 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -620,6 +620,8 @@ struct btrfs_encoded_read_private {
>   	struct btrfs_ioctl_encoded_io_args args;
>   	struct file *file;
>   	void __user *copy_out;
> +	struct io_uring_cmd *cmd;
> +	unsigned int issue_flags;
>   };
>   
...
> +static inline struct btrfs_encoded_read_private *btrfs_uring_encoded_read_pdu(
> +		struct io_uring_cmd *cmd)
> +{
> +	return *(struct btrfs_encoded_read_private **)cmd->pdu;
> +}
> +static void btrfs_finish_uring_encoded_read(struct io_uring_cmd *cmd,
> +					    unsigned int issue_flags)
> +{
> +	struct btrfs_encoded_read_private *priv;
> +	ssize_t ret;
> +
> +	priv = btrfs_uring_encoded_read_pdu(cmd);
> +	ret = btrfs_encoded_read_finish(priv, -EIOCBQUEUED);
> +
> +	io_uring_cmd_done(priv->cmd, ret, 0, priv->issue_flags);

s/priv->issue_flags/issue_flags/

You can't use flags that you got somewhere before from a completely
different execution context.

> +
> +	kfree(priv);
> +}
> +
> +static void btrfs_encoded_read_uring_endio(struct btrfs_bio *bbio)
> +{
> +	struct btrfs_encoded_read_private *priv = bbio->private;

Might be cleaner if you combine it with btrfs_encoded_read_ioctl_endio

btrfs_encoded_read_endio()
{
	...
	if (!atomic_dec_return(&priv->pending)) {
		if (priv->cmd)
			io_uring_cmd_complete_in_task();
		else
			wake_up();
	...
}



> +
> +	if (bbio->bio.bi_status) {
> +		/*
> +		 * The memory barrier implied by the atomic_dec_return() here
> +		 * pairs with the memory barrier implied by the
> +		 * atomic_dec_return() or io_wait_event() in
> +		 * btrfs_encoded_read_regular_fill_pages() to ensure that this
> +		 * write is observed before the load of status in
> +		 * btrfs_encoded_read_regular_fill_pages().
> +		 */
> +		WRITE_ONCE(priv->status, bbio->bio.bi_status);
> +	}
> +	if (!atomic_dec_return(&priv->pending)) {
> +		io_uring_cmd_complete_in_task(priv->cmd,
> +					      btrfs_finish_uring_encoded_read);
> +	}
> +	bio_put(&bbio->bio);
> +}
> +
>   static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   					u64 file_offset, u64 disk_bytenr,
>   					u64 disk_io_size,
> @@ -9106,11 +9148,16 @@ static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	unsigned long i = 0;
>   	struct btrfs_bio *bbio;
> +	btrfs_bio_end_io_t cb;
>   
> -	init_waitqueue_head(&priv->wait);
> +	if (priv->cmd) {
> +		cb = btrfs_encoded_read_uring_endio;
> +	} else {
> +		init_waitqueue_head(&priv->wait);
> +		cb = btrfs_encoded_read_ioctl_endio;
> +	}
>   
> -	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info,
> -			       btrfs_encoded_read_endio, priv);
> +	bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info, cb, priv);
>   	bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
>   	bbio->inode = inode;
>   
> @@ -9122,7 +9169,7 @@ static void _btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   			btrfs_submit_bio(bbio, 0);
>   
>   			bbio = btrfs_bio_alloc(BIO_MAX_VECS, REQ_OP_READ, fs_info,
> -					       btrfs_encoded_read_endio, priv);
> +					       cb, priv);
>   			bbio->bio.bi_iter.bi_sector = disk_bytenr >> SECTOR_SHIFT;
>   			bbio->inode = inode;
>   			continue;
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index c1886209933a..85a512a9ca64 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
...
> +static void btrfs_uring_encoded_read(struct io_uring_cmd *cmd,
> +				     unsigned int issue_flags)
> +{
> +	ssize_t ret;
> +	void __user *argp = (void __user *)(uintptr_t)cmd->sqe->addr;

u64_to_user_ptr()

> +	struct btrfs_encoded_read_private *priv;
> +	bool compat = issue_flags & IO_URING_F_COMPAT;
> +
> +	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;
> +		goto out_uring;
> +	}
> +
> +	ret = btrfs_prepare_encoded_read(priv, cmd->file, compat, argp);
> +	if (ret)
> +		goto out_finish;
> +
> +	if (iov_iter_count(&priv->iter) == 0) {
> +		ret = 0;
> +		goto out_finish;
> +	}
> +
> +	*(struct btrfs_encoded_read_private **)cmd->pdu = priv;
> +	priv->cmd = cmd;
> +	priv->issue_flags = issue_flags;

You shouldn't be stashing issue_flags, it almost always wrong.
Looks btrfs_finish_uring_encoded_read() is the only user, and
with my comment above you can kill priv->issue_flags

> +	ret = btrfs_encoded_read(priv);
> +
> +	if (ret == -EIOCBQUEUED && atomic_dec_return(&priv->pending))
> +		return;

Is gating on -EIOCBQUEUED just an optimisation? I.e. it's fine
to swap checks

if (atomic_dec_return(&priv->pending) && ret == -EIOCBQUEUED)

but we know that unless it returned -EIOCBQUEUED nobody
should've touched ->pending.

> +
> +out_finish:
> +	ret = btrfs_encoded_read_finish(priv, ret);
> +	kfree(priv);
> +
> +out_uring:
> +	io_uring_cmd_done(cmd, ret, 0, issue_flags);
> +}
> +

-- 
Pavel Begunkov

Re: [PATCH 5/6] btrfs: move wait out of btrfs_encoded_read

[Pavel Begunkov]

On 8/23/24 17:27, Mark Harmstone wrote:
> Makes it so that if btrfs_encoded_read has launched a bio, rather than
> waiting for it to complete it leaves the extent and inode locked and returns
> -EIOCBQUEUED. The caller is responsible for waiting on the bio, and
> calling the completion code in the new btrfs_encoded_read_regular_end.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
>   fs/btrfs/btrfs_inode.h |  1 +
>   fs/btrfs/inode.c       | 81 +++++++++++++++++++++++-------------------
>   fs/btrfs/ioctl.c       |  8 +++++
>   3 files changed, 54 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index f4d77c3bb544..a5d786c6d7d4 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -623,6 +623,7 @@ struct btrfs_encoded_read_private {
>   };
>   
>   ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv);
> +ssize_t btrfs_encoded_read_regular_end(struct btrfs_encoded_read_private *priv);
>   ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
>   			       const struct btrfs_ioctl_encoded_io_args *encoded);
>   
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 1e53977a4854..1bd4c74e8c51 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8999,7 +8999,7 @@ static ssize_t btrfs_encoded_read_inline(
>   				struct extent_state **cached_state,
>   				u64 extent_start, size_t count,
>   				struct btrfs_ioctl_encoded_io_args *encoded,
> -				bool *unlocked)
> +				bool *need_unlock)
>   {
>   	struct btrfs_root *root = inode->root;
>   	struct btrfs_fs_info *fs_info = root->fs_info;
> @@ -9067,7 +9067,7 @@ static ssize_t btrfs_encoded_read_inline(
>   	btrfs_release_path(path);
>   	unlock_extent(io_tree, start, lockend, cached_state);
>   	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
> -	*unlocked = true;
> +	*need_unlock = false;
>   
>   	ret = copy_to_iter(tmp, count, iter);
>   	if (ret != count)
> @@ -9155,42 +9155,26 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
>   	return blk_status_to_errno(READ_ONCE(priv.status));
>   }
>   
> -static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *priv,
> -					  u64 start, u64 lockend,
> -					  u64 disk_bytenr, u64 disk_io_size,
> -					  bool *unlocked)
> +ssize_t btrfs_encoded_read_regular_end(struct btrfs_encoded_read_private *priv)
> -	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
> -	struct extent_io_tree *io_tree = &inode->io_tree;
> +	u64 cur, start, lockend;
>   	unsigned long i;
> -	u64 cur;
>   	size_t page_offset;
> -	ssize_t ret;
> -
> -	priv->nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
> -	priv->pages = kcalloc(priv->nr_pages, sizeof(struct page *), GFP_NOFS);
> -	if (!priv->pages) {
> -		priv->nr_pages = 0;
> -		return -ENOMEM;
> -	}
> -	ret = btrfs_alloc_page_array(priv->nr_pages, priv->pages, false);
> -	if (ret)
> -		return -ENOMEM;
> +	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
> +	struct btrfs_fs_info *fs_info = inode->root->fs_info;
> +	struct extent_io_tree *io_tree = &inode->io_tree;
> +	int ret;
> -	_btrfs_encoded_read_regular_fill_pages(inode, start, disk_bytenr,
> -					       disk_io_size, priv);
> +	start = ALIGN_DOWN(priv->args.offset, fs_info->sectorsize);
> +	lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
>   
> -	if (atomic_dec_return(&priv->pending))
> -		io_wait_event(priv->wait, !atomic_read(&priv->pending));
> +	unlock_extent(io_tree, start, lockend, &priv->cached_state);
> +	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);

AFAIS, btrfs_encoded_read_regular_end is here to be used asynchronously
in a later patch, specifically via bio callback -> io_uring tw callback,
and that io_uring tw callback might not get run in any reasonable amount
of time as it's controlled by the user space.

So, it takes the inode lock in one context (task executing the
request) and releases it in another, it's usually is not allowed.
And it also holds the locks potentially semi indefinitely (guaranteed
to be executed when io_uring and/or task die). There is also
unlock_extent() which I'd assume we don't want to hold locked
for too long.

It's in general a bad practice having is_locked variables, especially
when it's not on stack, even more so when it's implicit like

regular_read() {
	(ret == -EIOCBQUEUED)
		need_unlock = false; // delay it
}
btrfs_encoded_read_finish() {
	if (ret == -EIOCBQUEUED)
		unlock();
}

That just too easy to miss when you do anything with the code
afterwards.

I hope Josef and btrfs folks can comment what would be a way
here, does the inode lock has to be held for the entire
duration of IO?

>   
>   	ret = blk_status_to_errno(READ_ONCE(priv->status));
>   	if (ret)
>   		return ret;
>   
> -	unlock_extent(io_tree, start, lockend, &priv->cached_state);
> -	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
> -	*unlocked = true;
> -
>   	if (priv->args.compression) {
>   		i = 0;
>   		page_offset = 0;
> @@ -9215,6 +9199,30 @@ static ssize_t btrfs_encoded_read_regular(struct btrfs_encoded_read_private *pri
>   	return priv->count;
>   }

-- 
Pavel Begunkov

Re: [PATCH 2/6] btrfs: store encoded read state in struct btrfs_encoded_read_private

[Pavel Begunkov]

On 8/23/24 17:27, Mark Harmstone wrote:
> Move the various stack variables needed for encoded reads into struct
> btrfs_encoded_read_private, so that we can split it into several
> functions.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
>   fs/btrfs/btrfs_inode.h |  20 ++++-
>   fs/btrfs/inode.c       | 170 +++++++++++++++++++++--------------------
>   fs/btrfs/ioctl.c       |  60 ++++++++-------
>   3 files changed, 135 insertions(+), 115 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index affe70929234..5cd4308bd337 100644
...
>   
> -ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
> -			   struct iov_iter *iter,
> -			   struct btrfs_ioctl_encoded_io_args *encoded)
> +ssize_t btrfs_encoded_read(struct btrfs_encoded_read_private *priv)
>   {
> -	struct btrfs_inode *inode = BTRFS_I(file_inode(file));
> +	struct btrfs_inode *inode = BTRFS_I(file_inode(priv->file));
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	struct extent_io_tree *io_tree = &inode->io_tree;
>   	ssize_t ret;
> -	size_t count = iov_iter_count(iter);
>   	u64 start, lockend, disk_bytenr, disk_io_size;
> -	struct extent_state *cached_state = NULL;
>   	struct extent_map *em;
>   	bool unlocked = false;
>   
> -	file_accessed(file);
> +	priv->count = iov_iter_count(&priv->iter);
> +
> +	file_accessed(priv->file);
>   
>   	btrfs_inode_lock(inode, BTRFS_ILOCK_SHARED);

Request submission should usually be short and not block
on IO or any locks that might wait for IO.

bool nowait = issue_flags & IO_URING_F_NONBLOCK;

btrfs_encoded_read(..., nowait) {
	f = BTRFS_ILOCK_SHARED;
	if (nowait)
		f |= BTRFS_ILOCK_TRY;
	if (btrfs_inode_lock(inode, f))
		return -EAGAIN; // io_uring will retry from a blocking context
}

so it might need sth like this here as well as for
filemap waiting and possibly other places.

>   
> -	if (offset >= inode->vfs_inode.i_size) {
> +	if (priv->args.offset >= inode->vfs_inode.i_size) {
>   		btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
>   		return 0;
>   	}
> -	start = ALIGN_DOWN(offset, fs_info->sectorsize);
> +	start = ALIGN_DOWN(priv->args.offset, fs_info->sectorsize);
>   	/*
> -	 * We don't know how long the extent containing offset is, but if
> -	 * it's compressed we know that it won't be longer than this.
> +	 * We don't know how long the extent containing priv->args.offset is,
> +	 * but if it's compressed we know that it won't be longer than this.
>   	 */
>   	lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;
>   
...

-- 
Pavel Begunkov

Re: [PATCH 6/6] btrfs: add io_uring interface for encoded reads

[Pavel Begunkov]

On 8/23/24 17:27, Mark Harmstone wrote:
> Adds an io_uring interface for asynchronous encoded reads, using the
> same interface as for the ioctl. To use this you would use an SQE opcode
> of IORING_OP_URING_CMD, the cmd_op would be BTRFS_IOC_ENCODED_READ, and
> addr would point to the userspace address of the
> btrfs_ioctl_encoded_io_args struct. As with the ioctl, you need to have
> CAP_SYS_ADMIN for this to work.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
...
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 1bd4c74e8c51..e4458168c340 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -34,6 +34,7 @@
>   #include <linux/iomap.h>
>   #include <asm/unaligned.h>
>   #include <linux/fsverity.h>
> +#include <linux/io_uring/cmd.h>
>   #include "misc.h"
>   #include "ctree.h"
>   #include "disk-io.h"
> @@ -9078,7 +9079,7 @@ static ssize_t btrfs_encoded_read_inline(
>   	return ret;
>   }
>   
> -static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
> +static void btrfs_encoded_read_ioctl_endio(struct btrfs_bio *bbio)
>   {
>   	struct btrfs_encoded_read_private *priv = bbio->private;
>   
> @@ -9098,6 +9099,47 @@ static void btrfs_encoded_read_endio(struct btrfs_bio *bbio)
>   	bio_put(&bbio->bio);
>   }
>   
> +static inline struct btrfs_encoded_read_private *btrfs_uring_encoded_read_pdu(
> +		struct io_uring_cmd *cmd)
> +{
> +	return *(struct btrfs_encoded_read_private **)cmd->pdu;
> +}
> +static void btrfs_finish_uring_encoded_read(struct io_uring_cmd *cmd,
> +					    unsigned int issue_flags)
> +{
> +	struct btrfs_encoded_read_private *priv;
> +	ssize_t ret;
> +
> +	priv = btrfs_uring_encoded_read_pdu(cmd);
> +	ret = btrfs_encoded_read_finish(priv, -EIOCBQUEUED);

tw callback -> btrfs_encoded_read_finish() -> copy_to_user()

That's usually fine except cases when the task and/or io_uring are dying
and the callback executes not from a user task context. Same problem as
with fuse, we can pass a flag from io_uring into the callback telling
btrfs that it should terminate the request and not rely on mm or any
other task related pieces.


> +
> +	io_uring_cmd_done(priv->cmd, ret, 0, priv->issue_flags);
> +
> +	kfree(priv);
> +}
> +

...

-- 
Pavel Begunkov