[PATCH v6 0/8] btrfs: add fscrypt support, PART 1

[PATCH v6 0/8] btrfs: add fscrypt support, PART 1

[Daniel Vacek]

This is a revive of former work [1] of Omar, Sweet Tea and Josef to bring
native encryption support to btrfs.

It will come in more parts. The first part this time is splitting the simple
and isolated stuff out first to reduce the size of the final patchset.

Changes since v5 [1] are mostly rebase to the latest for-next and cleaning
up the conflicts.

The remaining part needs further cleanup and a bit of redesign and it will
follow later.

[1] https://lore.kernel.org/linux-btrfs/cover.1706116485.git.josef@toxicpanda.com/

Josef Bacik (6):
  btrfs: add a bio argument to btrfs_csum_one_bio
  btrfs: add orig_logical to btrfs_bio
  btrfs: don't rewrite ret from inode_permission
  btrfs: move inode_to_path higher in backref.c
  btrfs: don't search back for dir inode item in INO_LOOKUP_USER
  btrfs: set the appropriate free space settings in reconfigure

Omar Sandoval (1):
  btrfs: disable various operations on encrypted inodes

Sweet Tea Dorminy (1):
  btrfs: disable verity on encrypted inodes

 fs/btrfs/backref.c   | 68 +++++++++++++++++++++-----------------------
 fs/btrfs/bio.c       | 14 +++++++--
 fs/btrfs/bio.h       |  3 ++
 fs/btrfs/disk-io.c   |  2 +-
 fs/btrfs/file-item.c | 19 ++++++-------
 fs/btrfs/file-item.h |  2 +-
 fs/btrfs/inode.c     |  4 +++
 fs/btrfs/ioctl.c     | 27 +++---------------
 fs/btrfs/reflink.c   |  7 +++++
 fs/btrfs/super.c     | 28 +++++++++---------
 fs/btrfs/super.h     |  3 +-
 fs/btrfs/verity.c    |  3 ++
 12 files changed, 93 insertions(+), 87 deletions(-)

-- 
2.51.0

[PATCH v6 1/8] btrfs: disable various operations on encrypted inodes

[Daniel Vacek]

From: Omar Sandoval <osandov@osandov.com>

Initially, only normal data extents will be encrypted. This change
forbids various other bits:
- allows reflinking only if both inodes have the same encryption status
- disable inline data on encrypted inodes

Signed-off-by: Omar Sandoval <osandov@osandov.com>
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Daniel Vacek <neelx@suse.com>
---
v5 was 'Reviewed-by: Boris Burkov <boris@bur.io>' [1] but the rebase
changed the code a bit so dropping.

[1] https://lore.kernel.org/linux-btrfs/20240124195303.GC1789919@zen.localdomain/
---
 fs/btrfs/inode.c   | 4 ++++
 fs/btrfs/reflink.c | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 8737914e8552..b810e831fc23 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -592,6 +592,10 @@ static bool can_cow_file_range_inline(struct btrfs_inode *inode,
 	if (size < i_size_read(&inode->vfs_inode))
 		return false;
 
+	/* Encrypted file cannot be inlined. */
+	if (IS_ENCRYPTED(&inode->vfs_inode))
+		return false;
+
 	return true;
 }
 
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index 775a32a7953a..3c9c570d6493 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/blkdev.h>
+#include <linux/fscrypt.h>
 #include <linux/iversion.h>
 #include "ctree.h"
 #include "fs.h"
@@ -789,6 +790,12 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
 		ASSERT(inode_in->vfs_inode.i_sb == inode_out->vfs_inode.i_sb);
 	}
 
+	/*
+	 * Can only reflink encrypted files if both files are encrypted.
+	 */
+	if (IS_ENCRYPTED(&inode_in->vfs_inode) != IS_ENCRYPTED(&inode_out->vfs_inode))
+		return -EINVAL;
+
 	/* Don't make the dst file partly checksummed */
 	if ((inode_in->flags & BTRFS_INODE_NODATASUM) !=
 	    (inode_out->flags & BTRFS_INODE_NODATASUM)) {
-- 
2.51.0

[PATCH v6 2/8] btrfs: disable verity on encrypted inodes

[Daniel Vacek]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Right now there isn't a way to encrypt things that aren't either
filenames in directories or data on blocks on disk with extent
encryption, so for now, disable verity usage with encryption on btrfs.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Boris Burkov <boris@bur.io>
---
No changes since v5, just a merge conflict due to cleanup in context.
---
 fs/btrfs/verity.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 16f5580cba55..06dfcb461f53 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -578,6 +578,9 @@ static int btrfs_begin_enable_verity(struct file *filp)
 
 	btrfs_assert_inode_locked(inode);
 
+	if (IS_ENCRYPTED(&inode->vfs_inode))
+		return -EOPNOTSUPP;
+
 	if (test_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags))
 		return -EBUSY;
 
-- 
2.51.0

[PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Daniel Vacek]

From: Josef Bacik <josef@toxicpanda.com>

We only ever needed the bbio in btrfs_csum_one_bio, since that has the
bio embedded in it.  However with encryption we'll have a different bio
with the encrypted data in it, and the original bbio.  Update
btrfs_csum_one_bio to take the bio we're going to csum as an argument,
which will allow us to csum the encrypted bio and stuff the csums into
the corresponding bbio to be used later when the IO completes.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Daniel Vacek <neelx@suse.com>
---
Compared to v5 this needed to adapt to recent async csum changes.
---
 fs/btrfs/bio.c       |  4 ++--
 fs/btrfs/bio.h       |  1 +
 fs/btrfs/file-item.c | 17 ++++++++---------
 fs/btrfs/file-item.h |  2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index a73652b8724a..a69174b2b6b6 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
 	if (bbio->bio.bi_opf & REQ_META)
 		return btree_csum_one_bio(bbio);
 #ifdef CONFIG_BTRFS_EXPERIMENTAL
-	return btrfs_csum_one_bio(bbio, true);
+	return btrfs_csum_one_bio(bbio, &bbio->bio, true);
 #else
-	return btrfs_csum_one_bio(bbio, false);
+	return btrfs_csum_one_bio(bbio, &bbio->bio, false);
 #endif
 }
 
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index deaeea3becf4..c5a6c66d51a0 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -58,6 +58,7 @@ struct btrfs_bio {
 			struct btrfs_ordered_sum *sums;
 			struct work_struct csum_work;
 			struct completion csum_done;
+			struct bio *csum_bio;
 			struct bvec_iter csum_saved_iter;
 			u64 orig_physical;
 		};
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 72be3ede0edf..474949074da8 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
 	return ret;
 }
 
-static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
+static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
 {
 	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
-	struct bio *bio = &bbio->bio;
 	struct btrfs_ordered_sum *sums = bbio->sums;
-	struct bvec_iter iter = *src;
 	phys_addr_t paddr;
 	const u32 blocksize = fs_info->sectorsize;
 	int index = 0;
 
 	shash->tfm = fs_info->csum_shash;
 
-	btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
+	btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
 		btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
 		index += fs_info->csum_size;
 	}
@@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
 
 	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
 	ASSERT(bbio->async_csum == true);
-	csum_one_bio(bbio, &bbio->csum_saved_iter);
+	csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
 	complete(&bbio->csum_done);
 }
 
 /*
  * Calculate checksums of the data contained inside a bio.
  */
-int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
+int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
 {
 	struct btrfs_ordered_extent *ordered = bbio->ordered;
 	struct btrfs_inode *inode = bbio->inode;
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
-	struct bio *bio = &bbio->bio;
 	struct btrfs_ordered_sum *sums;
 	unsigned nofs_flag;
 
@@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
 	btrfs_add_ordered_sum(ordered, sums);
 
 	if (!async) {
-		csum_one_bio(bbio, &bbio->bio.bi_iter);
+		struct bvec_iter iter = bio->bi_iter;
+		csum_one_bio(bbio, bio, &iter);
 		return 0;
 	}
 	init_completion(&bbio->csum_done);
 	bbio->async_csum = true;
-	bbio->csum_saved_iter = bbio->bio.bi_iter;
+	bbio->csum_bio = bio;
+	bbio->csum_saved_iter = bio->bi_iter;
 	INIT_WORK(&bbio->csum_work, csum_one_bio_work);
 	schedule_work(&bbio->csum_work);
 	return 0;
diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
index 5645c5e3abdb..d16fd2144552 100644
--- a/fs/btrfs/file-item.h
+++ b/fs/btrfs/file-item.h
@@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
 int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root,
 			   struct btrfs_ordered_sum *sums);
-int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
+int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
 int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
 int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 			     struct list_head *list, int search_commit,
-- 
2.51.0

[PATCH v6 4/8] btrfs: add orig_logical to btrfs_bio

[Daniel Vacek]

From: Josef Bacik <josef@toxicpanda.com>

When checksumming the encrypted bio on writes we need to know which
logical address this checksum is for.  At the point where we get the
encrypted bio the bi_sector is the physical location on the target disk,
so we need to save the original logical offset in the btrfs_bio.  Then
we can use this when csum'ing the bio instead of the
bio->iter.bi_sector.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
No code changes other than context since v5.
---
 fs/btrfs/bio.c       | 10 ++++++++++
 fs/btrfs/bio.h       |  2 ++
 fs/btrfs/file-item.c |  2 +-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index a69174b2b6b6..aba452dd9904 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -94,6 +94,8 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
 	if (bbio_has_ordered_extent(bbio)) {
 		refcount_inc(&orig_bbio->ordered->refs);
 		bbio->ordered = orig_bbio->ordered;
+		bbio->orig_logical = orig_bbio->orig_logical;
+		orig_bbio->orig_logical += map_length;
 	}
 	bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
 	atomic_inc(&orig_bbio->pending_ios);
@@ -726,6 +728,14 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 		goto end_bbio;
 	}
 
+	/*
+	 * For fscrypt writes we will get the encrypted bio after we've
+	 * remapped our bio to the physical disk location, so we need to
+	 * save the original bytenr so we know what we're checksumming.
+	 */
+	if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
+		bbio->orig_logical = logical;
+
 	map_length = min(map_length, length);
 	if (use_append)
 		map_length = btrfs_append_map_length(bbio, map_length);
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index c5a6c66d51a0..5015e327dbd9 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -52,6 +52,7 @@ struct btrfs_bio {
 		 * - pointer to the checksums for this bio
 		 * - original physical address from the allocator
 		 *   (for zone append only)
+		 * - original logical address, used for checksumming fscrypt bios.
 		 */
 		struct {
 			struct btrfs_ordered_extent *ordered;
@@ -61,6 +62,7 @@ struct btrfs_bio {
 			struct bio *csum_bio;
 			struct bvec_iter csum_saved_iter;
 			u64 orig_physical;
+			u64 orig_logical;
 		};
 
 		/* For metadata reads: parentness verification. */
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 474949074da8..d2ecd26727ac 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -812,7 +812,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
 	if (!sums)
 		return -ENOMEM;
 
-	sums->logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
+	sums->logical = bbio->orig_logical;
 	sums->len = bio->bi_iter.bi_size;
 	INIT_LIST_HEAD(&sums->list);
 	bbio->sums = sums;
-- 
2.51.0

[PATCH v6 5/8] btrfs: don't rewrite ret from inode_permission

[Daniel Vacek]

From: Josef Bacik <josef@toxicpanda.com>

In our user safe ino resolve ioctl we'll just turn any ret into -EACCES
from inode_permission.  This is redundant, and could potentially be
wrong if we had an ENOMEM in the security layer or some such other
error, so simply return the actual return value.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
No changes since v5 other than context.
---
 fs/btrfs/ioctl.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b524db8a4973..2973230f2988 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1910,10 +1910,8 @@ static int btrfs_search_path_in_tree_user(struct mnt_idmap *idmap,
 			ret = inode_permission(idmap, &temp_inode->vfs_inode,
 					       MAY_READ | MAY_EXEC);
 			iput(&temp_inode->vfs_inode);
-			if (ret) {
-				ret = -EACCES;
+			if (ret)
 				goto out_put;
-			}
 
 			if (key.offset == upper_limit)
 				break;
-- 
2.51.0

[PATCH v6 6/8] btrfs: move inode_to_path higher in backref.c

[Daniel Vacek]

From: Josef Bacik <josef@toxicpanda.com>

We have a prototype and then the definition lower below, we don't need
to do this, simply move the function to where the prototype is.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
No code changes since v5.
---
 fs/btrfs/backref.c | 68 ++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index eff2d388a706..56e177941da4 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -2574,8 +2574,39 @@ int iterate_inodes_from_logical(u64 logical, struct btrfs_fs_info *fs_info,
 	return iterate_extent_inodes(&walk_ctx, false, build_ino_list, ctx);
 }
 
+/*
+ * returns 0 if the path could be dumped (probably truncated)
+ * returns <0 in case of an error
+ */
 static int inode_to_path(u64 inum, u32 name_len, unsigned long name_off,
-			 struct extent_buffer *eb, struct inode_fs_paths *ipath);
+			 struct extent_buffer *eb, struct inode_fs_paths *ipath)
+{
+	char *fspath;
+	char *fspath_min;
+	int i = ipath->fspath->elem_cnt;
+	const int s_ptr = sizeof(char *);
+	u32 bytes_left;
+
+	bytes_left = ipath->fspath->bytes_left > s_ptr ? ipath->fspath->bytes_left - s_ptr : 0;
+
+	fspath_min = (char *)ipath->fspath->val + (i + 1) * s_ptr;
+	fspath = btrfs_ref_to_path(ipath->fs_root, ipath->btrfs_path, name_len,
+				   name_off, eb, inum, fspath_min, bytes_left);
+	if (IS_ERR(fspath))
+		return PTR_ERR(fspath);
+
+	if (fspath > fspath_min) {
+		ipath->fspath->val[i] = (u64)(unsigned long)fspath;
+		++ipath->fspath->elem_cnt;
+		ipath->fspath->bytes_left = fspath - fspath_min;
+	} else {
+		++ipath->fspath->elem_missed;
+		ipath->fspath->bytes_missing += fspath_min - fspath;
+		ipath->fspath->bytes_left = 0;
+	}
+
+	return 0;
+}
 
 static int iterate_inode_refs(u64 inum, struct inode_fs_paths *ipath)
 {
@@ -2700,41 +2731,6 @@ static int iterate_inode_extrefs(u64 inum, struct inode_fs_paths *ipath)
 	return ret;
 }
 
-/*
- * returns 0 if the path could be dumped (probably truncated)
- * returns <0 in case of an error
- */
-static int inode_to_path(u64 inum, u32 name_len, unsigned long name_off,
-			 struct extent_buffer *eb, struct inode_fs_paths *ipath)
-{
-	char *fspath;
-	char *fspath_min;
-	int i = ipath->fspath->elem_cnt;
-	const int s_ptr = sizeof(char *);
-	u32 bytes_left;
-
-	bytes_left = ipath->fspath->bytes_left > s_ptr ?
-					ipath->fspath->bytes_left - s_ptr : 0;
-
-	fspath_min = (char *)ipath->fspath->val + (i + 1) * s_ptr;
-	fspath = btrfs_ref_to_path(ipath->fs_root, ipath->btrfs_path, name_len,
-				   name_off, eb, inum, fspath_min, bytes_left);
-	if (IS_ERR(fspath))
-		return PTR_ERR(fspath);
-
-	if (fspath > fspath_min) {
-		ipath->fspath->val[i] = (u64)(unsigned long)fspath;
-		++ipath->fspath->elem_cnt;
-		ipath->fspath->bytes_left = fspath - fspath_min;
-	} else {
-		++ipath->fspath->elem_missed;
-		ipath->fspath->bytes_missing += fspath_min - fspath;
-		ipath->fspath->bytes_left = 0;
-	}
-
-	return 0;
-}
-
 /*
  * this dumps all file system paths to the inode into the ipath struct, provided
  * is has been created large enough. each path is zero-terminated and accessed
-- 
2.51.0

[PATCH v6 7/8] btrfs: don't search back for dir inode item in INO_LOOKUP_USER

[Daniel Vacek]

From: Josef Bacik <josef@toxicpanda.com>

We don't need to search back to the inode item, the directory inode
number is in key.offset, so simply use that.  If we can't find the
directory we'll get an ENOENT at the iget.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
Since v5 resolved rebase merge conflicts due to changes in API,
dropping the sb parameter in btrfs_iget().
---
 fs/btrfs/ioctl.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 2973230f2988..d323fc351c00 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1822,7 +1822,7 @@ static int btrfs_search_path_in_tree_user(struct mnt_idmap *idmap,
 	struct btrfs_root_ref *rref;
 	struct btrfs_root *root = NULL;
 	struct btrfs_path *path;
-	struct btrfs_key key, key2;
+	struct btrfs_key key;
 	struct extent_buffer *leaf;
 	char *ptr;
 	int slot;
@@ -1877,24 +1877,6 @@ static int btrfs_search_path_in_tree_user(struct mnt_idmap *idmap,
 			read_extent_buffer(leaf, ptr,
 					(unsigned long)(iref + 1), len);
 
-			/* Check the read+exec permission of this directory */
-			ret = btrfs_previous_item(root, path, dirid,
-						  BTRFS_INODE_ITEM_KEY);
-			if (ret < 0) {
-				goto out_put;
-			} else if (ret > 0) {
-				ret = -ENOENT;
-				goto out_put;
-			}
-
-			leaf = path->nodes[0];
-			slot = path->slots[0];
-			btrfs_item_key_to_cpu(leaf, &key2, slot);
-			if (key2.objectid != dirid) {
-				ret = -ENOENT;
-				goto out_put;
-			}
-
 			/*
 			 * We don't need the path anymore, so release it and
 			 * avoid deadlocks and lockdep warnings in case
@@ -1902,11 +1884,12 @@ static int btrfs_search_path_in_tree_user(struct mnt_idmap *idmap,
 			 * btree and lock the same leaf.
 			 */
 			btrfs_release_path(path);
-			temp_inode = btrfs_iget(key2.objectid, root);
+			temp_inode = btrfs_iget(key.offset, root);
 			if (IS_ERR(temp_inode)) {
 				ret = PTR_ERR(temp_inode);
 				goto out_put;
 			}
+			/* Check the read+exec permission of this directory */
 			ret = inode_permission(idmap, &temp_inode->vfs_inode,
 					       MAY_READ | MAY_EXEC);
 			iput(&temp_inode->vfs_inode);
-- 
2.51.0

[PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure

[Daniel Vacek]

From: Josef Bacik <josef@toxicpanda.com>

btrfs/330 uncovered a problem where we were accidentally turning off the
free space tree when we do the transition from ro->rw.  This happens
because we don't update

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
Since v5 just a rebase conflict with changing API of mount_opt flags
being 64bit now.
---
 fs/btrfs/disk-io.c |  2 +-
 fs/btrfs/super.c   | 28 +++++++++++++++-------------
 fs/btrfs/super.h   |  3 ++-
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6a1fa3b08b3f..3bc7a773b900 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3399,7 +3399,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	 * Handle the space caching options appropriately now that we have the
 	 * super block loaded and validated.
 	 */
-	btrfs_set_free_space_cache_settings(fs_info);
+	btrfs_set_free_space_cache_settings(fs_info, &fs_info->mount_opt);
 
 	if (!btrfs_check_options(fs_info, &fs_info->mount_opt, sb->s_flags)) {
 		ret = -EINVAL;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 4ffd7059e27a..f8759b856174 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -723,10 +723,10 @@ bool btrfs_check_options(const struct btrfs_fs_info *info,
 }
 
 /*
- * This is subtle, we only call this during open_ctree().  We need to pre-load
- * the mount options with the on-disk settings.  Before the new mount API took
- * effect we would do this on mount and remount.  With the new mount API we'll
- * only do this on the initial mount.
+ * Because we have an odd set of behavior with turning on and off the space cache
+ * and free space tree we have to call this before we start the mount operation
+ * after we load the super, or before we start remount.  This is to make sure we
+ * have the proper free space settings in place if the user didn't specify any.
  *
  * This isn't a change in behavior, because we're using the current state of the
  * file system to set the current mount options.  If you mounted with special
@@ -734,15 +734,16 @@ bool btrfs_check_options(const struct btrfs_fs_info *info,
  * settings, because mounting without these features cleared the on-disk
  * settings, so this being called on re-mount is not needed.
  */
-void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
+void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info,
+					 unsigned long long *mount_opt)
 {
 	if (fs_info->sectorsize < PAGE_SIZE) {
-		btrfs_clear_opt(fs_info->mount_opt, SPACE_CACHE);
-		if (!btrfs_test_opt(fs_info, FREE_SPACE_TREE)) {
+		btrfs_clear_opt(*mount_opt, SPACE_CACHE);
+		if (!btrfs_raw_test_opt(*mount_opt, FREE_SPACE_TREE)) {
 			btrfs_info(fs_info,
 				   "forcing free space tree for sector size %u with page size %lu",
 				   fs_info->sectorsize, PAGE_SIZE);
-			btrfs_set_opt(fs_info->mount_opt, FREE_SPACE_TREE);
+			btrfs_set_opt(*mount_opt, FREE_SPACE_TREE);
 		}
 	}
 
@@ -750,7 +751,7 @@ void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
 	 * At this point our mount options are populated, so we only mess with
 	 * these settings if we don't have any settings already.
 	 */
-	if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
+	if (btrfs_raw_test_opt(*mount_opt, FREE_SPACE_TREE))
 		return;
 
 	if (btrfs_is_zoned(fs_info) &&
@@ -760,10 +761,10 @@ void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
 		return;
 	}
 
-	if (btrfs_test_opt(fs_info, SPACE_CACHE))
+	if (btrfs_raw_test_opt(*mount_opt, SPACE_CACHE))
 		return;
 
-	if (btrfs_test_opt(fs_info, NOSPACECACHE))
+	if (btrfs_raw_test_opt(*mount_opt, NOSPACECACHE))
 		return;
 
 	/*
@@ -771,9 +772,9 @@ void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info)
 	 * them ourselves based on the state of the file system.
 	 */
 	if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE))
-		btrfs_set_opt(fs_info->mount_opt, FREE_SPACE_TREE);
+		btrfs_set_opt(*mount_opt, FREE_SPACE_TREE);
 	else if (btrfs_free_space_cache_v1_active(fs_info))
-		btrfs_set_opt(fs_info->mount_opt, SPACE_CACHE);
+		btrfs_set_opt(*mount_opt, SPACE_CACHE);
 }
 
 static void set_device_specific_options(struct btrfs_fs_info *fs_info)
@@ -1523,6 +1524,7 @@ static int btrfs_reconfigure(struct fs_context *fc)
 
 	sync_filesystem(sb);
 	set_bit(BTRFS_FS_STATE_REMOUNTING, &fs_info->fs_state);
+	btrfs_set_free_space_cache_settings(fs_info, &ctx->mount_opt);
 
 	if (!btrfs_check_options(fs_info, &ctx->mount_opt, fc->sb_flags))
 		return -EINVAL;
diff --git a/fs/btrfs/super.h b/fs/btrfs/super.h
index d80a86acfbbe..584f428d36e2 100644
--- a/fs/btrfs/super.h
+++ b/fs/btrfs/super.h
@@ -16,7 +16,8 @@ bool btrfs_check_options(const struct btrfs_fs_info *info,
 int btrfs_sync_fs(struct super_block *sb, int wait);
 char *btrfs_get_subvol_name_from_objectid(struct btrfs_fs_info *fs_info,
 					  u64 subvol_objectid);
-void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info);
+void btrfs_set_free_space_cache_settings(struct btrfs_fs_info *fs_info,
+					 unsigned long long *mount_opt);
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
 {
-- 
2.51.0

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Qu Wenruo]

在 2025/11/13 06:06, Daniel Vacek 写道:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> We only ever needed the bbio in btrfs_csum_one_bio, since that has the
> bio embedded in it.  However with encryption we'll have a different bio
> with the encrypted data in it, and the original bbio.  Update
> btrfs_csum_one_bio to take the bio we're going to csum as an argument,
> which will allow us to csum the encrypted bio and stuff the csums into
> the corresponding bbio to be used later when the IO completes.

I'm wondering why we can not do it in a layered bio way.

E.g. on device-mapper based solutions, the upper layer send out the bio 
containing the plaintext data.
Then the dm-crypto send out a new bio, containing the encrypted data.

The storage layer doesn't need to bother the plaintext bio at all, they 
just write the encrypted one to disk.

And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.


So why we can not just create a new bio for the final csum caculation, 
just like compression?

Thanks,
Qu

> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Daniel Vacek <neelx@suse.com>
> ---
> Compared to v5 this needed to adapt to recent async csum changes.
> ---
>   fs/btrfs/bio.c       |  4 ++--
>   fs/btrfs/bio.h       |  1 +
>   fs/btrfs/file-item.c | 17 ++++++++---------
>   fs/btrfs/file-item.h |  2 +-
>   4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index a73652b8724a..a69174b2b6b6 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>   	if (bbio->bio.bi_opf & REQ_META)
>   		return btree_csum_one_bio(bbio);
>   #ifdef CONFIG_BTRFS_EXPERIMENTAL
> -	return btrfs_csum_one_bio(bbio, true);
> +	return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>   #else
> -	return btrfs_csum_one_bio(bbio, false);
> +	return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>   #endif
>   }
>   
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index deaeea3becf4..c5a6c66d51a0 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -58,6 +58,7 @@ struct btrfs_bio {
>   			struct btrfs_ordered_sum *sums;
>   			struct work_struct csum_work;
>   			struct completion csum_done;
> +			struct bio *csum_bio;
>   			struct bvec_iter csum_saved_iter;
>   			u64 orig_physical;
>   		};
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 72be3ede0edf..474949074da8 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
>   	return ret;
>   }
>   
> -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
> +static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
>   {
>   	struct btrfs_inode *inode = bbio->inode;
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> -	struct bio *bio = &bbio->bio;
>   	struct btrfs_ordered_sum *sums = bbio->sums;
> -	struct bvec_iter iter = *src;
>   	phys_addr_t paddr;
>   	const u32 blocksize = fs_info->sectorsize;
>   	int index = 0;
>   
>   	shash->tfm = fs_info->csum_shash;
>   
> -	btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> +	btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>   		btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>   		index += fs_info->csum_size;
>   	}
> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>   
>   	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>   	ASSERT(bbio->async_csum == true);
> -	csum_one_bio(bbio, &bbio->csum_saved_iter);
> +	csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>   	complete(&bbio->csum_done);
>   }
>   
>   /*
>    * Calculate checksums of the data contained inside a bio.
>    */
> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
>   {
>   	struct btrfs_ordered_extent *ordered = bbio->ordered;
>   	struct btrfs_inode *inode = bbio->inode;
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
> -	struct bio *bio = &bbio->bio;
>   	struct btrfs_ordered_sum *sums;
>   	unsigned nofs_flag;
>   
> @@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>   	btrfs_add_ordered_sum(ordered, sums);
>   
>   	if (!async) {
> -		csum_one_bio(bbio, &bbio->bio.bi_iter);
> +		struct bvec_iter iter = bio->bi_iter;
> +		csum_one_bio(bbio, bio, &iter);
>   		return 0;
>   	}
>   	init_completion(&bbio->csum_done);
>   	bbio->async_csum = true;
> -	bbio->csum_saved_iter = bbio->bio.bi_iter;
> +	bbio->csum_bio = bio;
> +	bbio->csum_saved_iter = bio->bi_iter;
>   	INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>   	schedule_work(&bbio->csum_work);
>   	return 0;
> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> index 5645c5e3abdb..d16fd2144552 100644
> --- a/fs/btrfs/file-item.h
> +++ b/fs/btrfs/file-item.h
> @@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>   int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>   			   struct btrfs_root *root,
>   			   struct btrfs_ordered_sum *sums);
> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
>   int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
>   int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>   			     struct list_head *list, int search_commit,

Re: [PATCH v6 4/8] btrfs: add orig_logical to btrfs_bio

[Qu Wenruo]

在 2025/11/13 06:06, Daniel Vacek 写道:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> When checksumming the encrypted bio on writes we need to know which
> logical address this checksum is for.  At the point where we get the
> encrypted bio the bi_sector is the physical location on the target disk,
> so we need to save the original logical offset in the btrfs_bio.  Then
> we can use this when csum'ing the bio instead of the
> bio->iter.bi_sector.

We have already btrfs_bio::csum_saved_iter, we can just reuse it to grab 
logical bytenr by csum_saved_iter->bi_iter.bi_sector.

Although that member is only for data writes with data checksum, it's 
not hard to adapt it for encryption.

But in that case, we may want to rename that member to something more 
generic, maybe like write_saved_iter?

Thanks,
Qu

> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> No code changes other than context since v5.
> ---
>   fs/btrfs/bio.c       | 10 ++++++++++
>   fs/btrfs/bio.h       |  2 ++
>   fs/btrfs/file-item.c |  2 +-
>   3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index a69174b2b6b6..aba452dd9904 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -94,6 +94,8 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
>   	if (bbio_has_ordered_extent(bbio)) {
>   		refcount_inc(&orig_bbio->ordered->refs);
>   		bbio->ordered = orig_bbio->ordered;
> +		bbio->orig_logical = orig_bbio->orig_logical;
> +		orig_bbio->orig_logical += map_length;
>   	}
>   	bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
>   	atomic_inc(&orig_bbio->pending_ios);
> @@ -726,6 +728,14 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>   		goto end_bbio;
>   	}
>   
> +	/*
> +	 * For fscrypt writes we will get the encrypted bio after we've
> +	 * remapped our bio to the physical disk location, so we need to
> +	 * save the original bytenr so we know what we're checksumming.
> +	 */
> +	if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
> +		bbio->orig_logical = logical;
> +
>   	map_length = min(map_length, length);
>   	if (use_append)
>   		map_length = btrfs_append_map_length(bbio, map_length);
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index c5a6c66d51a0..5015e327dbd9 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -52,6 +52,7 @@ struct btrfs_bio {
>   		 * - pointer to the checksums for this bio
>   		 * - original physical address from the allocator
>   		 *   (for zone append only)
> +		 * - original logical address, used for checksumming fscrypt bios.
>   		 */
>   		struct {
>   			struct btrfs_ordered_extent *ordered;
> @@ -61,6 +62,7 @@ struct btrfs_bio {
>   			struct bio *csum_bio;
>   			struct bvec_iter csum_saved_iter;
>   			u64 orig_physical;
> +			u64 orig_logical;
>   		};
>   
>   		/* For metadata reads: parentness verification. */
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 474949074da8..d2ecd26727ac 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -812,7 +812,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
>   	if (!sums)
>   		return -ENOMEM;
>   
> -	sums->logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
> +	sums->logical = bbio->orig_logical;
>   	sums->len = bio->bi_iter.bi_size;
>   	INIT_LIST_HEAD(&sums->list);
>   	bbio->sums = sums;

Re: [PATCH v6 1/8] btrfs: disable various operations on encrypted inodes

[Qu Wenruo]

在 2025/11/13 06:06, Daniel Vacek 写道:
> From: Omar Sandoval <osandov@osandov.com>
> 
> Initially, only normal data extents will be encrypted. This change
> forbids various other bits:
> - allows reflinking only if both inodes have the same encryption status
> - disable inline data on encrypted inodes

I'm wondering how will this affect other users of inlined data. 
Especially for symbol links.

Symbol links always store they link source inside an inline data file 
extent. Does such content also get encrypted?

Thanks,
Qu

> 
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Daniel Vacek <neelx@suse.com>
> ---
> v5 was 'Reviewed-by: Boris Burkov <boris@bur.io>' [1] but the rebase
> changed the code a bit so dropping.
> 
> [1] https://lore.kernel.org/linux-btrfs/20240124195303.GC1789919@zen.localdomain/
> ---
>   fs/btrfs/inode.c   | 4 ++++
>   fs/btrfs/reflink.c | 7 +++++++
>   2 files changed, 11 insertions(+)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 8737914e8552..b810e831fc23 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -592,6 +592,10 @@ static bool can_cow_file_range_inline(struct btrfs_inode *inode,
>   	if (size < i_size_read(&inode->vfs_inode))
>   		return false;
>   
> +	/* Encrypted file cannot be inlined. */
> +	if (IS_ENCRYPTED(&inode->vfs_inode))
> +		return false;
> +
>   	return true;
>   }
>   
> diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
> index 775a32a7953a..3c9c570d6493 100644
> --- a/fs/btrfs/reflink.c
> +++ b/fs/btrfs/reflink.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   
>   #include <linux/blkdev.h>
> +#include <linux/fscrypt.h>
>   #include <linux/iversion.h>
>   #include "ctree.h"
>   #include "fs.h"
> @@ -789,6 +790,12 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
>   		ASSERT(inode_in->vfs_inode.i_sb == inode_out->vfs_inode.i_sb);
>   	}
>   
> +	/*
> +	 * Can only reflink encrypted files if both files are encrypted.
> +	 */
> +	if (IS_ENCRYPTED(&inode_in->vfs_inode) != IS_ENCRYPTED(&inode_out->vfs_inode))
> +		return -EINVAL;
> +
>   	/* Don't make the dst file partly checksummed */
>   	if ((inode_in->flags & BTRFS_INODE_NODATASUM) !=
>   	    (inode_out->flags & BTRFS_INODE_NODATASUM)) {

Re: [PATCH v6 1/8] btrfs: disable various operations on encrypted inodes

[David Sterba]

On Thu, Nov 13, 2025 at 07:40:35AM +1030, Qu Wenruo wrote:
> 在 2025/11/13 06:06, Daniel Vacek 写道:
> > From: Omar Sandoval <osandov@osandov.com>
> > 
> > Initially, only normal data extents will be encrypted. This change
> > forbids various other bits:
> > - allows reflinking only if both inodes have the same encryption status
> > - disable inline data on encrypted inodes
> 
> I'm wondering how will this affect other users of inlined data. 
> Especially for symbol links.
> 
> Symbol links always store they link source inside an inline data file 
> extent. Does such content also get encrypted?

Symlinks are passed to the fscrypt API and encrypted if needed, using
ext4_symlink() as an example.

Re: [PATCH v6 2/8] btrfs: disable verity on encrypted inodes

[David Sterba]

On Wed, Nov 12, 2025 at 08:36:02PM +0100, Daniel Vacek wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> Right now there isn't a way to encrypt things that aren't either
> filenames in directories or data on blocks on disk with extent
> encryption, so for now, disable verity usage with encryption on btrfs.

Maybe mention that fscrypt + fsverity is possible and could be
implemented in the future.

Re: [PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure

[David Sterba]

On Wed, Nov 12, 2025 at 08:36:08PM +0100, Daniel Vacek wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> btrfs/330 uncovered a problem where we were accidentally turning off the
> free space tree when we do the transition from ro->rw.  This happens
> because we don't update

Missing text.

Re: [PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure

[Daniel Vacek]

On Thu, 13 Nov 2025 at 11:32, David Sterba <dsterba@suse.cz> wrote:
> On Wed, Nov 12, 2025 at 08:36:08PM +0100, Daniel Vacek wrote:
> > From: Josef Bacik <josef@toxicpanda.com>
> >
> > btrfs/330 uncovered a problem where we were accidentally turning off the
> > free space tree when we do the transition from ro->rw.  This happens
> > because we don't update
>
> Missing text.

Hmm, this patch is new to v5. It doesn't even look encryption related.
I have no idea what Josef really means here.

The whole idea seems to be to call
btrfs_set_free_space_cache_settings() from btrfs_reconfigure() and to
update the ctx->mount_opt instead of fs_info->mount_opt while
remounting.

And btrfs/330 is not failing even with the full patchset applied
without this patch. I'm wondering if it is still needed after those
years?

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Daniel Vacek]

On Wed, 12 Nov 2025 at 22:02, Qu Wenruo <wqu@suse.com> wrote:
> 在 2025/11/13 06:06, Daniel Vacek 写道:
> > From: Josef Bacik <josef@toxicpanda.com>
> >
> > We only ever needed the bbio in btrfs_csum_one_bio, since that has the
> > bio embedded in it.  However with encryption we'll have a different bio
> > with the encrypted data in it, and the original bbio.  Update
> > btrfs_csum_one_bio to take the bio we're going to csum as an argument,
> > which will allow us to csum the encrypted bio and stuff the csums into
> > the corresponding bbio to be used later when the IO completes.
>
> I'm wondering why we can not do it in a layered bio way.
>
> E.g. on device-mapper based solutions, the upper layer send out the bio
> containing the plaintext data.
> Then the dm-crypto send out a new bio, containing the encrypted data.

This is similar but fscrypt works on FS level. It gets the original
plaintext bio (the btrfs_bio::bio one) and gives us a callback with
the encrypted bio to calculate the checksum of the encrypted data. No
plaintext bio goes to the storage layer.

--nX

> The storage layer doesn't need to bother the plaintext bio at all, they
> just write the encrypted one to disk.
>
> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>
>
> So why we can not just create a new bio for the final csum caculation,
> just like compression?
>
> Thanks,
> Qu
>
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Daniel Vacek <neelx@suse.com>
> > ---
> > Compared to v5 this needed to adapt to recent async csum changes.
> > ---
> >   fs/btrfs/bio.c       |  4 ++--
> >   fs/btrfs/bio.h       |  1 +
> >   fs/btrfs/file-item.c | 17 ++++++++---------
> >   fs/btrfs/file-item.h |  2 +-
> >   4 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> > index a73652b8724a..a69174b2b6b6 100644
> > --- a/fs/btrfs/bio.c
> > +++ b/fs/btrfs/bio.c
> > @@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
> >       if (bbio->bio.bi_opf & REQ_META)
> >               return btree_csum_one_bio(bbio);
> >   #ifdef CONFIG_BTRFS_EXPERIMENTAL
> > -     return btrfs_csum_one_bio(bbio, true);
> > +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
> >   #else
> > -     return btrfs_csum_one_bio(bbio, false);
> > +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
> >   #endif
> >   }
> >
> > diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> > index deaeea3becf4..c5a6c66d51a0 100644
> > --- a/fs/btrfs/bio.h
> > +++ b/fs/btrfs/bio.h
> > @@ -58,6 +58,7 @@ struct btrfs_bio {
> >                       struct btrfs_ordered_sum *sums;
> >                       struct work_struct csum_work;
> >                       struct completion csum_done;
> > +                     struct bio *csum_bio;
> >                       struct bvec_iter csum_saved_iter;
> >                       u64 orig_physical;
> >               };
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 72be3ede0edf..474949074da8 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
> >       return ret;
> >   }
> >
> > -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
> > +static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
> >   {
> >       struct btrfs_inode *inode = bbio->inode;
> >       struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >       SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> > -     struct bio *bio = &bbio->bio;
> >       struct btrfs_ordered_sum *sums = bbio->sums;
> > -     struct bvec_iter iter = *src;
> >       phys_addr_t paddr;
> >       const u32 blocksize = fs_info->sectorsize;
> >       int index = 0;
> >
> >       shash->tfm = fs_info->csum_shash;
> >
> > -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> > +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
> >               btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
> >               index += fs_info->csum_size;
> >       }
> > @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
> >
> >       ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
> >       ASSERT(bbio->async_csum == true);
> > -     csum_one_bio(bbio, &bbio->csum_saved_iter);
> > +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
> >       complete(&bbio->csum_done);
> >   }
> >
> >   /*
> >    * Calculate checksums of the data contained inside a bio.
> >    */
> > -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> > +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
> >   {
> >       struct btrfs_ordered_extent *ordered = bbio->ordered;
> >       struct btrfs_inode *inode = bbio->inode;
> >       struct btrfs_fs_info *fs_info = inode->root->fs_info;
> > -     struct bio *bio = &bbio->bio;
> >       struct btrfs_ordered_sum *sums;
> >       unsigned nofs_flag;
> >
> > @@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >       btrfs_add_ordered_sum(ordered, sums);
> >
> >       if (!async) {
> > -             csum_one_bio(bbio, &bbio->bio.bi_iter);
> > +             struct bvec_iter iter = bio->bi_iter;
> > +             csum_one_bio(bbio, bio, &iter);
> >               return 0;
> >       }
> >       init_completion(&bbio->csum_done);
> >       bbio->async_csum = true;
> > -     bbio->csum_saved_iter = bbio->bio.bi_iter;
> > +     bbio->csum_bio = bio;
> > +     bbio->csum_saved_iter = bio->bi_iter;
> >       INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> >       schedule_work(&bbio->csum_work);
> >       return 0;
> > diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> > index 5645c5e3abdb..d16fd2144552 100644
> > --- a/fs/btrfs/file-item.h
> > +++ b/fs/btrfs/file-item.h
> > @@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
> >   int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
> >                          struct btrfs_root *root,
> >                          struct btrfs_ordered_sum *sums);
> > -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
> > +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
> >   int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
> >   int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
> >                            struct list_head *list, int search_commit,
>

Re: [PATCH v6 4/8] btrfs: add orig_logical to btrfs_bio

[Daniel Vacek]

On Wed, 12 Nov 2025 at 22:07, Qu Wenruo <wqu@suse.com> wrote:
> 在 2025/11/13 06:06, Daniel Vacek 写道:
> > From: Josef Bacik <josef@toxicpanda.com>
> >
> > When checksumming the encrypted bio on writes we need to know which
> > logical address this checksum is for.  At the point where we get the
> > encrypted bio the bi_sector is the physical location on the target disk,
> > so we need to save the original logical offset in the btrfs_bio.  Then
> > we can use this when csum'ing the bio instead of the
> > bio->iter.bi_sector.
>
> We have already btrfs_bio::csum_saved_iter, we can just reuse it to grab
> logical bytenr by csum_saved_iter->bi_iter.bi_sector.
>
> Although that member is only for data writes with data checksum, it's
> not hard to adapt it for encryption.
>
> But in that case, we may want to rename that member to something more
> generic, maybe like write_saved_iter?

Yeah, something like that could work. Later fscrypt change uses
bbio->saved_iter for checking the checksum on encrypted read. Perhaps
we can use the same one in both cases?

--nX

> Thanks,
> Qu
>
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> > No code changes other than context since v5.
> > ---
> >   fs/btrfs/bio.c       | 10 ++++++++++
> >   fs/btrfs/bio.h       |  2 ++
> >   fs/btrfs/file-item.c |  2 +-
> >   3 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> > index a69174b2b6b6..aba452dd9904 100644
> > --- a/fs/btrfs/bio.c
> > +++ b/fs/btrfs/bio.c
> > @@ -94,6 +94,8 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
> >       if (bbio_has_ordered_extent(bbio)) {
> >               refcount_inc(&orig_bbio->ordered->refs);
> >               bbio->ordered = orig_bbio->ordered;
> > +             bbio->orig_logical = orig_bbio->orig_logical;
> > +             orig_bbio->orig_logical += map_length;
> >       }
> >       bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
> >       atomic_inc(&orig_bbio->pending_ios);
> > @@ -726,6 +728,14 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
> >               goto end_bbio;
> >       }
> >
> > +     /*
> > +      * For fscrypt writes we will get the encrypted bio after we've
> > +      * remapped our bio to the physical disk location, so we need to
> > +      * save the original bytenr so we know what we're checksumming.
> > +      */
> > +     if (bio_op(bio) == REQ_OP_WRITE && is_data_bbio(bbio))
> > +             bbio->orig_logical = logical;
> > +
> >       map_length = min(map_length, length);
> >       if (use_append)
> >               map_length = btrfs_append_map_length(bbio, map_length);
> > diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> > index c5a6c66d51a0..5015e327dbd9 100644
> > --- a/fs/btrfs/bio.h
> > +++ b/fs/btrfs/bio.h
> > @@ -52,6 +52,7 @@ struct btrfs_bio {
> >                * - pointer to the checksums for this bio
> >                * - original physical address from the allocator
> >                *   (for zone append only)
> > +              * - original logical address, used for checksumming fscrypt bios.
> >                */
> >               struct {
> >                       struct btrfs_ordered_extent *ordered;
> > @@ -61,6 +62,7 @@ struct btrfs_bio {
> >                       struct bio *csum_bio;
> >                       struct bvec_iter csum_saved_iter;
> >                       u64 orig_physical;
> > +                     u64 orig_logical;
> >               };
> >
> >               /* For metadata reads: parentness verification. */
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 474949074da8..d2ecd26727ac 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -812,7 +812,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
> >       if (!sums)
> >               return -ENOMEM;
> >
> > -     sums->logical = bio->bi_iter.bi_sector << SECTOR_SHIFT;
> > +     sums->logical = bbio->orig_logical;
> >       sums->len = bio->bi_iter.bi_size;
> >       INIT_LIST_HEAD(&sums->list);
> >       bbio->sums = sums;
>

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Qu Wenruo]

在 2025/11/14 05:37, Daniel Vacek 写道:
> On Wed, 12 Nov 2025 at 22:02, Qu Wenruo <wqu@suse.com> wrote:
>> 在 2025/11/13 06:06, Daniel Vacek 写道:
>>> From: Josef Bacik <josef@toxicpanda.com>
>>>
>>> We only ever needed the bbio in btrfs_csum_one_bio, since that has the
>>> bio embedded in it.  However with encryption we'll have a different bio
>>> with the encrypted data in it, and the original bbio.  Update
>>> btrfs_csum_one_bio to take the bio we're going to csum as an argument,
>>> which will allow us to csum the encrypted bio and stuff the csums into
>>> the corresponding bbio to be used later when the IO completes.
>>
>> I'm wondering why we can not do it in a layered bio way.
>>
>> E.g. on device-mapper based solutions, the upper layer send out the bio
>> containing the plaintext data.
>> Then the dm-crypto send out a new bio, containing the encrypted data.
> 
> This is similar but fscrypt works on FS level. It gets the original
> plaintext bio (the btrfs_bio::bio one) and gives us a callback with
> the encrypted bio to calculate the checksum of the encrypted data. No
> plaintext bio goes to the storage layer.

Then why put the original bio pointer into the super generic btrfs_bio?

I thought it's more common to put the original plaintext into the 
encryption specific structure, like what we did for compression.

Thanks,
Qu

> 
> --nX
> 
>> The storage layer doesn't need to bother the plaintext bio at all, they
>> just write the encrypted one to disk.
>>
>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>>
>>
>> So why we can not just create a new bio for the final csum caculation,
>> just like compression?
>>
>> Thanks,
>> Qu
>>
>>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
>>> ---
>>> Compared to v5 this needed to adapt to recent async csum changes.
>>> ---
>>>    fs/btrfs/bio.c       |  4 ++--
>>>    fs/btrfs/bio.h       |  1 +
>>>    fs/btrfs/file-item.c | 17 ++++++++---------
>>>    fs/btrfs/file-item.h |  2 +-
>>>    4 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>> index a73652b8724a..a69174b2b6b6 100644
>>> --- a/fs/btrfs/bio.c
>>> +++ b/fs/btrfs/bio.c
>>> @@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>>>        if (bbio->bio.bi_opf & REQ_META)
>>>                return btree_csum_one_bio(bbio);
>>>    #ifdef CONFIG_BTRFS_EXPERIMENTAL
>>> -     return btrfs_csum_one_bio(bbio, true);
>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>>>    #else
>>> -     return btrfs_csum_one_bio(bbio, false);
>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>>>    #endif
>>>    }
>>>
>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
>>> index deaeea3becf4..c5a6c66d51a0 100644
>>> --- a/fs/btrfs/bio.h
>>> +++ b/fs/btrfs/bio.h
>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
>>>                        struct btrfs_ordered_sum *sums;
>>>                        struct work_struct csum_work;
>>>                        struct completion csum_done;
>>> +                     struct bio *csum_bio;
>>>                        struct bvec_iter csum_saved_iter;
>>>                        u64 orig_physical;
>>>                };
>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>> index 72be3ede0edf..474949074da8 100644
>>> --- a/fs/btrfs/file-item.c
>>> +++ b/fs/btrfs/file-item.c
>>> @@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
>>>        return ret;
>>>    }
>>>
>>> -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
>>> +static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
>>>    {
>>>        struct btrfs_inode *inode = bbio->inode;
>>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>        SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>>> -     struct bio *bio = &bbio->bio;
>>>        struct btrfs_ordered_sum *sums = bbio->sums;
>>> -     struct bvec_iter iter = *src;
>>>        phys_addr_t paddr;
>>>        const u32 blocksize = fs_info->sectorsize;
>>>        int index = 0;
>>>
>>>        shash->tfm = fs_info->csum_shash;
>>>
>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>>>                btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>>>                index += fs_info->csum_size;
>>>        }
>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>>>
>>>        ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>>>        ASSERT(bbio->async_csum == true);
>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>>>        complete(&bbio->csum_done);
>>>    }
>>>
>>>    /*
>>>     * Calculate checksums of the data contained inside a bio.
>>>     */
>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
>>>    {
>>>        struct btrfs_ordered_extent *ordered = bbio->ordered;
>>>        struct btrfs_inode *inode = bbio->inode;
>>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>> -     struct bio *bio = &bbio->bio;
>>>        struct btrfs_ordered_sum *sums;
>>>        unsigned nofs_flag;
>>>
>>> @@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>>>        btrfs_add_ordered_sum(ordered, sums);
>>>
>>>        if (!async) {
>>> -             csum_one_bio(bbio, &bbio->bio.bi_iter);
>>> +             struct bvec_iter iter = bio->bi_iter;
>>> +             csum_one_bio(bbio, bio, &iter);
>>>                return 0;
>>>        }
>>>        init_completion(&bbio->csum_done);
>>>        bbio->async_csum = true;
>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
>>> +     bbio->csum_bio = bio;
>>> +     bbio->csum_saved_iter = bio->bi_iter;
>>>        INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>>>        schedule_work(&bbio->csum_work);
>>>        return 0;
>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>> index 5645c5e3abdb..d16fd2144552 100644
>>> --- a/fs/btrfs/file-item.h
>>> +++ b/fs/btrfs/file-item.h
>>> @@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>>>    int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>>>                           struct btrfs_root *root,
>>>                           struct btrfs_ordered_sum *sums);
>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
>>>    int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
>>>    int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>>>                             struct list_head *list, int search_commit,
>>

Re: [PATCH v6 8/8] btrfs: set the appropriate free space settings in reconfigure

[David Sterba]

On Thu, Nov 13, 2025 at 12:24:33PM +0100, Daniel Vacek wrote:
> On Thu, 13 Nov 2025 at 11:32, David Sterba <dsterba@suse.cz> wrote:
> > On Wed, Nov 12, 2025 at 08:36:08PM +0100, Daniel Vacek wrote:
> > > From: Josef Bacik <josef@toxicpanda.com>
> > >
> > > btrfs/330 uncovered a problem where we were accidentally turning off the
> > > free space tree when we do the transition from ro->rw.  This happens
> > > because we don't update
> >
> > Missing text.
> 
> Hmm, this patch is new to v5. It doesn't even look encryption related.
> I have no idea what Josef really means here.
> 
> The whole idea seems to be to call
> btrfs_set_free_space_cache_settings() from btrfs_reconfigure() and to
> update the ctx->mount_opt instead of fs_info->mount_opt while
> remounting.
> 
> And btrfs/330 is not failing even with the full patchset applied
> without this patch. I'm wondering if it is still needed after those
> years?

Maybe it it's not, as all the patches are somehow independent you can
drop it for now. We can add it later if need be.

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Daniel Vacek]

On Thu, 13 Nov 2025 at 21:16, Qu Wenruo <wqu@suse.com> wrote:
> 在 2025/11/14 05:37, Daniel Vacek 写道:
> > On Wed, 12 Nov 2025 at 22:02, Qu Wenruo <wqu@suse.com> wrote:
> >> 在 2025/11/13 06:06, Daniel Vacek 写道:
> >>> From: Josef Bacik <josef@toxicpanda.com>
> >>>
> >>> We only ever needed the bbio in btrfs_csum_one_bio, since that has the
> >>> bio embedded in it.  However with encryption we'll have a different bio
> >>> with the encrypted data in it, and the original bbio.  Update
> >>> btrfs_csum_one_bio to take the bio we're going to csum as an argument,
> >>> which will allow us to csum the encrypted bio and stuff the csums into
> >>> the corresponding bbio to be used later when the IO completes.
> >>
> >> I'm wondering why we can not do it in a layered bio way.
> >>
> >> E.g. on device-mapper based solutions, the upper layer send out the bio
> >> containing the plaintext data.
> >> Then the dm-crypto send out a new bio, containing the encrypted data.
> >
> > This is similar but fscrypt works on FS level. It gets the original
> > plaintext bio (the btrfs_bio::bio one) and gives us a callback with
> > the encrypted bio to calculate the checksum of the encrypted data. No
> > plaintext bio goes to the storage layer.
>
> Then why put the original bio pointer into the super generic btrfs_bio?

When encryption is enabled, it's not going to be the original bio but
rather the encrypted one.

But giving it another thought and checking the related fscrypt code,
the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
and freed in blk_crypto_fallback_encrypt_endio() before calling
bio_endio() on our original plaintext bio.
This means we have no control over the bounce bio lifetime and we
cannot store the pointer and use it asynchronously. We'll need to
always compute the checksum synchronously for encrypted bios. In that
case we don't need to store it in btrfs_bio::csum_bio at all. For the
regular (unencrypted) case we can keep using the &bbio->bio.

I'll drop the csum_bio for there is no use really.

--nX

> I thought it's more common to put the original plaintext into the
> encryption specific structure, like what we did for compression.
>
> Thanks,
> Qu
>
> >
> > --nX
> >
> >> The storage layer doesn't need to bother the plaintext bio at all, they
> >> just write the encrypted one to disk.
> >>
> >> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
> >>
> >>
> >> So why we can not just create a new bio for the final csum caculation,
> >> just like compression?
> >>
> >> Thanks,
> >> Qu
> >>
> >>>
> >>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> >>> Signed-off-by: Daniel Vacek <neelx@suse.com>
> >>> ---
> >>> Compared to v5 this needed to adapt to recent async csum changes.
> >>> ---
> >>>    fs/btrfs/bio.c       |  4 ++--
> >>>    fs/btrfs/bio.h       |  1 +
> >>>    fs/btrfs/file-item.c | 17 ++++++++---------
> >>>    fs/btrfs/file-item.h |  2 +-
> >>>    4 files changed, 12 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> >>> index a73652b8724a..a69174b2b6b6 100644
> >>> --- a/fs/btrfs/bio.c
> >>> +++ b/fs/btrfs/bio.c
> >>> @@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
> >>>        if (bbio->bio.bi_opf & REQ_META)
> >>>                return btree_csum_one_bio(bbio);
> >>>    #ifdef CONFIG_BTRFS_EXPERIMENTAL
> >>> -     return btrfs_csum_one_bio(bbio, true);
> >>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
> >>>    #else
> >>> -     return btrfs_csum_one_bio(bbio, false);
> >>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
> >>>    #endif
> >>>    }
> >>>
> >>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> >>> index deaeea3becf4..c5a6c66d51a0 100644
> >>> --- a/fs/btrfs/bio.h
> >>> +++ b/fs/btrfs/bio.h
> >>> @@ -58,6 +58,7 @@ struct btrfs_bio {
> >>>                        struct btrfs_ordered_sum *sums;
> >>>                        struct work_struct csum_work;
> >>>                        struct completion csum_done;
> >>> +                     struct bio *csum_bio;
> >>>                        struct bvec_iter csum_saved_iter;
> >>>                        u64 orig_physical;
> >>>                };
> >>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> >>> index 72be3ede0edf..474949074da8 100644
> >>> --- a/fs/btrfs/file-item.c
> >>> +++ b/fs/btrfs/file-item.c
> >>> @@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
> >>>        return ret;
> >>>    }
> >>>
> >>> -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
> >>> +static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
> >>>    {
> >>>        struct btrfs_inode *inode = bbio->inode;
> >>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>>        SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> >>> -     struct bio *bio = &bbio->bio;
> >>>        struct btrfs_ordered_sum *sums = bbio->sums;
> >>> -     struct bvec_iter iter = *src;
> >>>        phys_addr_t paddr;
> >>>        const u32 blocksize = fs_info->sectorsize;
> >>>        int index = 0;
> >>>
> >>>        shash->tfm = fs_info->csum_shash;
> >>>
> >>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> >>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
> >>>                btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
> >>>                index += fs_info->csum_size;
> >>>        }
> >>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
> >>>
> >>>        ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
> >>>        ASSERT(bbio->async_csum == true);
> >>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
> >>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
> >>>        complete(&bbio->csum_done);
> >>>    }
> >>>
> >>>    /*
> >>>     * Calculate checksums of the data contained inside a bio.
> >>>     */
> >>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
> >>>    {
> >>>        struct btrfs_ordered_extent *ordered = bbio->ordered;
> >>>        struct btrfs_inode *inode = bbio->inode;
> >>>        struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>> -     struct bio *bio = &bbio->bio;
> >>>        struct btrfs_ordered_sum *sums;
> >>>        unsigned nofs_flag;
> >>>
> >>> @@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >>>        btrfs_add_ordered_sum(ordered, sums);
> >>>
> >>>        if (!async) {
> >>> -             csum_one_bio(bbio, &bbio->bio.bi_iter);
> >>> +             struct bvec_iter iter = bio->bi_iter;
> >>> +             csum_one_bio(bbio, bio, &iter);
> >>>                return 0;
> >>>        }
> >>>        init_completion(&bbio->csum_done);
> >>>        bbio->async_csum = true;
> >>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
> >>> +     bbio->csum_bio = bio;
> >>> +     bbio->csum_saved_iter = bio->bi_iter;
> >>>        INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> >>>        schedule_work(&bbio->csum_work);
> >>>        return 0;
> >>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> >>> index 5645c5e3abdb..d16fd2144552 100644
> >>> --- a/fs/btrfs/file-item.h
> >>> +++ b/fs/btrfs/file-item.h
> >>> @@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
> >>>    int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
> >>>                           struct btrfs_root *root,
> >>>                           struct btrfs_ordered_sum *sums);
> >>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
> >>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
> >>>    int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
> >>>    int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
> >>>                             struct list_head *list, int search_commit,
> >>
>

Re: [PATCH v6 0/8] btrfs: add fscrypt support, PART 1

[David Sterba]

On Wed, Nov 12, 2025 at 08:36:00PM +0100, Daniel Vacek wrote:
> This is a revive of former work [1] of Omar, Sweet Tea and Josef to bring
> native encryption support to btrfs.
> 
> It will come in more parts. The first part this time is splitting the simple
> and isolated stuff out first to reduce the size of the final patchset.
> 
> Changes since v5 [1] are mostly rebase to the latest for-next and cleaning
> up the conflicts.
> 
> The remaining part needs further cleanup and a bit of redesign and it will
> follow later.
> 
> [1] https://lore.kernel.org/linux-btrfs/cover.1706116485.git.josef@toxicpanda.com/
> 
> Josef Bacik (6):
>   btrfs: add a bio argument to btrfs_csum_one_bio
>   btrfs: add orig_logical to btrfs_bio
>   btrfs: don't rewrite ret from inode_permission
>   btrfs: move inode_to_path higher in backref.c
>   btrfs: don't search back for dir inode item in INO_LOOKUP_USER
>   btrfs: set the appropriate free space settings in reconfigure
> 
> Omar Sandoval (1):
>   btrfs: disable various operations on encrypted inodes
> 
> Sweet Tea Dorminy (1):
>   btrfs: disable verity on encrypted inodes

Please resend the series what is OK for merge right now, I'm counting
two dropped patches. For the signed-off we might need to add an
explanation why there are so many or only keep the first one as the
patch author, the others usually mean only the pass through and not
really doing any contribution. Eventually there could be Co-developed-by
but this would need more investigation in the previous patch iterations.
This can be done once the patches are in for-next.

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Christoph Hellwig]

On Tue, Nov 18, 2025 at 03:05:25PM +0100, Daniel Vacek wrote:
> But giving it another thought and checking the related fscrypt code,
> the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> and freed in blk_crypto_fallback_encrypt_endio() before calling
> bio_endio() on our original plaintext bio.

That code is getting major refactoring right now, and allowing the
file system to hook into the submission is a possibility.

The problem is that I have no idea what you're trying to do as the
context is missing.

In general prep serious should be self contained and at least borderline
useful by themselves.  Adding random dead code checks or weird arguments
as done here are not useful in a prep series without context, they
should be close to the code making use of them to be understandable.

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Daniel Vacek]

On Tue, 18 Nov 2025 at 16:08, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Nov 18, 2025 at 03:05:25PM +0100, Daniel Vacek wrote:
> > But giving it another thought and checking the related fscrypt code,
> > the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> > and freed in blk_crypto_fallback_encrypt_endio() before calling
> > bio_endio() on our original plaintext bio.
>
> That code is getting major refactoring right now, and allowing the
> file system to hook into the submission is a possibility.
>
> The problem is that I have no idea what you're trying to do as the
> context is missing.
>
> In general prep series should be self contained and at least borderline
> useful by themselves.  Adding random dead code checks or weird arguments
> as done here are not useful in a prep series without context, they
> should be close to the code making use of them to be understandable.

Yeah, agreed. It would be better to send this one together with the
fscrypt changes later. I was suggested by Dave to pick this one ahead
and I did not argue while I probably should have.

--nX

On Tue, 18 Nov 2025 at 16:08, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Nov 18, 2025 at 03:05:25PM +0100, Daniel Vacek wrote:
> > But giving it another thought and checking the related fscrypt code,
> > the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> > and freed in blk_crypto_fallback_encrypt_endio() before calling
> > bio_endio() on our original plaintext bio.
>
> That code is getting major refactoring right now, and allowing the
> file system to hook into the submission is a possibility.
>
> The problem is that I have no idea what you're trying to do as the
> context is missing.
>
> In general prep serious should be self contained and at least borderline
> useful by themselves.  Adding random dead code checks or weird arguments
> as done here are not useful in a prep series without context, they
> should be close to the code making use of them to be understandable.

Re: [PATCH v6 0/8] btrfs: add fscrypt support, PART 1

[Daniel Vacek]

On Tue, 18 Nov 2025 at 16:04, David Sterba <dsterba@suse.cz> wrote:
> On Wed, Nov 12, 2025 at 08:36:00PM +0100, Daniel Vacek wrote:
> > This is a revive of former work [1] of Omar, Sweet Tea and Josef to bring
> > native encryption support to btrfs.
> >
> > It will come in more parts. The first part this time is splitting the simple
> > and isolated stuff out first to reduce the size of the final patchset.
> >
> > Changes since v5 [1] are mostly rebase to the latest for-next and cleaning
> > up the conflicts.
> >
> > The remaining part needs further cleanup and a bit of redesign and it will
> > follow later.
> >
> > [1] https://lore.kernel.org/linux-btrfs/cover.1706116485.git.josef@toxicpanda.com/
> >
> > Josef Bacik (6):
> >   btrfs: add a bio argument to btrfs_csum_one_bio
> >   btrfs: add orig_logical to btrfs_bio
> >   btrfs: don't rewrite ret from inode_permission
> >   btrfs: move inode_to_path higher in backref.c
> >   btrfs: don't search back for dir inode item in INO_LOOKUP_USER
> >   btrfs: set the appropriate free space settings in reconfigure
> >
> > Omar Sandoval (1):
> >   btrfs: disable various operations on encrypted inodes
> >
> > Sweet Tea Dorminy (1):
> >   btrfs: disable verity on encrypted inodes
>
> Please resend the series what is OK for merge right now, I'm counting
> two dropped patches. For the signed-off we might need to add an
> explanation why there are so many or only keep the first one as the
> patch author, the others usually mean only the pass through and not
> really doing any contribution. Eventually there could be Co-developed-by
> but this would need more investigation in the previous patch iterations.
> This can be done once the patches are in for-next.

I just did. The SoBs I just kept as they were in v5 from Josef so I
understand that's what it was agreed on before.

--nX

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Qu Wenruo]

在 2025/11/19 00:35, Daniel Vacek 写道:
> On Thu, 13 Nov 2025 at 21:16, Qu Wenruo <wqu@suse.com> wrote:
[...]
>> Then why put the original bio pointer into the super generic btrfs_bio?
> 
> When encryption is enabled, it's not going to be the original bio but
> rather the encrypted one.
> 
> But giving it another thought and checking the related fscrypt code,
> the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> and freed in blk_crypto_fallback_encrypt_endio() before calling
> bio_endio() on our original plaintext bio.
> This means we have no control over the bounce bio lifetime and we
> cannot store the pointer and use it asynchronously.

Sorry I didn't get the point why we can not calculate the csum async.

Higher layer just submit a btrfs_bio, its content is the encrypted contents.

As long as it's still a btrfs_bio, we have all the needed structures to 
do async csum.
We still need to submit the bio for writes, and that means we have 
enough time to calculate the csum async, and before the endio function 
called, we're able to do whatever we need, the bio won't be suddenly 
gone during the submission.

Unless you mean the encrypted bio is not encapsulated by btrfs_bio, but 
a vanilla bio.

In that case you can not even submit it through btrfs_submit_bbio().

Thanks,
Qu

> We'll need to
> always compute the checksum synchronously for encrypted bios. In that
> case we don't need to store it in btrfs_bio::csum_bio at all. For the
> regular (unencrypted) case we can keep using the &bbio->bio.
> 
> I'll drop the csum_bio for there is no use really.
> 
> --nX
> 
>> I thought it's more common to put the original plaintext into the
>> encryption specific structure, like what we did for compression.
>>
>> Thanks,
>> Qu
>>
>>>
>>> --nX
>>>
>>>> The storage layer doesn't need to bother the plaintext bio at all, they
>>>> just write the encrypted one to disk.
>>>>
>>>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>>>>
>>>>
>>>> So why we can not just create a new bio for the final csum caculation,
>>>> just like compression?
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>>
>>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
>>>>> ---
>>>>> Compared to v5 this needed to adapt to recent async csum changes.
>>>>> ---
>>>>>     fs/btrfs/bio.c       |  4 ++--
>>>>>     fs/btrfs/bio.h       |  1 +
>>>>>     fs/btrfs/file-item.c | 17 ++++++++---------
>>>>>     fs/btrfs/file-item.h |  2 +-
>>>>>     4 files changed, 12 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>>>> index a73652b8724a..a69174b2b6b6 100644
>>>>> --- a/fs/btrfs/bio.c
>>>>> +++ b/fs/btrfs/bio.c
>>>>> @@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>>>>>         if (bbio->bio.bi_opf & REQ_META)
>>>>>                 return btree_csum_one_bio(bbio);
>>>>>     #ifdef CONFIG_BTRFS_EXPERIMENTAL
>>>>> -     return btrfs_csum_one_bio(bbio, true);
>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>>>>>     #else
>>>>> -     return btrfs_csum_one_bio(bbio, false);
>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>>>>>     #endif
>>>>>     }
>>>>>
>>>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
>>>>> index deaeea3becf4..c5a6c66d51a0 100644
>>>>> --- a/fs/btrfs/bio.h
>>>>> +++ b/fs/btrfs/bio.h
>>>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
>>>>>                         struct btrfs_ordered_sum *sums;
>>>>>                         struct work_struct csum_work;
>>>>>                         struct completion csum_done;
>>>>> +                     struct bio *csum_bio;
>>>>>                         struct bvec_iter csum_saved_iter;
>>>>>                         u64 orig_physical;
>>>>>                 };
>>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>>>> index 72be3ede0edf..474949074da8 100644
>>>>> --- a/fs/btrfs/file-item.c
>>>>> +++ b/fs/btrfs/file-item.c
>>>>> @@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
>>>>>         return ret;
>>>>>     }
>>>>>
>>>>> -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
>>>>> +static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
>>>>>     {
>>>>>         struct btrfs_inode *inode = bbio->inode;
>>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>>         SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>>>>> -     struct bio *bio = &bbio->bio;
>>>>>         struct btrfs_ordered_sum *sums = bbio->sums;
>>>>> -     struct bvec_iter iter = *src;
>>>>>         phys_addr_t paddr;
>>>>>         const u32 blocksize = fs_info->sectorsize;
>>>>>         int index = 0;
>>>>>
>>>>>         shash->tfm = fs_info->csum_shash;
>>>>>
>>>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
>>>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>>>>>                 btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>>>>>                 index += fs_info->csum_size;
>>>>>         }
>>>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>>>>>
>>>>>         ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>>>>>         ASSERT(bbio->async_csum == true);
>>>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
>>>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>>>>>         complete(&bbio->csum_done);
>>>>>     }
>>>>>
>>>>>     /*
>>>>>      * Calculate checksums of the data contained inside a bio.
>>>>>      */
>>>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>>>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
>>>>>     {
>>>>>         struct btrfs_ordered_extent *ordered = bbio->ordered;
>>>>>         struct btrfs_inode *inode = bbio->inode;
>>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>> -     struct bio *bio = &bbio->bio;
>>>>>         struct btrfs_ordered_sum *sums;
>>>>>         unsigned nofs_flag;
>>>>>
>>>>> @@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>>>>>         btrfs_add_ordered_sum(ordered, sums);
>>>>>
>>>>>         if (!async) {
>>>>> -             csum_one_bio(bbio, &bbio->bio.bi_iter);
>>>>> +             struct bvec_iter iter = bio->bi_iter;
>>>>> +             csum_one_bio(bbio, bio, &iter);
>>>>>                 return 0;
>>>>>         }
>>>>>         init_completion(&bbio->csum_done);
>>>>>         bbio->async_csum = true;
>>>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
>>>>> +     bbio->csum_bio = bio;
>>>>> +     bbio->csum_saved_iter = bio->bi_iter;
>>>>>         INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>>>>>         schedule_work(&bbio->csum_work);
>>>>>         return 0;
>>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>>>> index 5645c5e3abdb..d16fd2144552 100644
>>>>> --- a/fs/btrfs/file-item.h
>>>>> +++ b/fs/btrfs/file-item.h
>>>>> @@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>>>>>     int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>>>>>                            struct btrfs_root *root,
>>>>>                            struct btrfs_ordered_sum *sums);
>>>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
>>>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
>>>>>     int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
>>>>>     int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>>>>>                              struct list_head *list, int search_commit,
>>>>
>>
> 

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Daniel Vacek]

On Tue, 18 Nov 2025 at 22:05, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 在 2025/11/19 00:35, Daniel Vacek 写道:
> > On Thu, 13 Nov 2025 at 21:16, Qu Wenruo <wqu@suse.com> wrote:
> [...]
> >> Then why put the original bio pointer into the super generic btrfs_bio?
> >
> > When encryption is enabled, it's not going to be the original bio but
> > rather the encrypted one.
> >
> > But giving it another thought and checking the related fscrypt code,
> > the encrypted bio is allocated in  blk_crypto_fallback_encrypt_bio()
> > and freed in blk_crypto_fallback_encrypt_endio() before calling
> > bio_endio() on our original plaintext bio.
> > This means we have no control over the bounce bio lifetime and we
> > cannot store the pointer and use it asynchronously.
>
> Sorry I didn't get the point why we can not calculate the csum async.
>
> Higher layer just submit a btrfs_bio, its content is the encrypted contents.
>
> As long as it's still a btrfs_bio, we have all the needed structures to
> do async csum.
> We still need to submit the bio for writes, and that means we have
> enough time to calculate the csum async, and before the endio function
> called, we're able to do whatever we need, the bio won't be suddenly
> gone during the submission.
>
> Unless you mean the encrypted bio is not encapsulated by btrfs_bio, but
> a vanilla bio.

That's the case. The bounce bio is created when you submit the
original one. The data is encrypted by fscrypt, then the csum hook is
called and the new bio submitted instead of the original one. Later
the endio frees the new one and calls endio on the original bio. This
means we don't have control over the bounce bio and cannot use it
asynchronously at the moment. The csum needs to be finished directly
in the hook.

Anyways, the hook changes are not upstreamed yet, so you can only see
it on the mailing list. And that's also why this patch makes more
sense to be sent later together with those changes.

--nX

> In that case you can not even submit it through btrfs_submit_bbio().
>
> Thanks,
> Qu
>
> > We'll need to
> > always compute the checksum synchronously for encrypted bios. In that
> > case we don't need to store it in btrfs_bio::csum_bio at all. For the
> > regular (unencrypted) case we can keep using the &bbio->bio.
> >
> > I'll drop the csum_bio for there is no use really.
> >
> > --nX
> >
> >> I thought it's more common to put the original plaintext into the
> >> encryption specific structure, like what we did for compression.
> >>
> >> Thanks,
> >> Qu
> >>
> >>>
> >>> --nX
> >>>
> >>>> The storage layer doesn't need to bother the plaintext bio at all, they
> >>>> just write the encrypted one to disk.
> >>>>
> >>>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
> >>>>
> >>>>
> >>>> So why we can not just create a new bio for the final csum caculation,
> >>>> just like compression?
> >>>>
> >>>> Thanks,
> >>>> Qu
> >>>>
> >>>>>
> >>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> >>>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
> >>>>> ---
> >>>>> Compared to v5 this needed to adapt to recent async csum changes.
> >>>>> ---
> >>>>>     fs/btrfs/bio.c       |  4 ++--
> >>>>>     fs/btrfs/bio.h       |  1 +
> >>>>>     fs/btrfs/file-item.c | 17 ++++++++---------
> >>>>>     fs/btrfs/file-item.h |  2 +-
> >>>>>     4 files changed, 12 insertions(+), 12 deletions(-)
> >>>>>
> >>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> >>>>> index a73652b8724a..a69174b2b6b6 100644
> >>>>> --- a/fs/btrfs/bio.c
> >>>>> +++ b/fs/btrfs/bio.c
> >>>>> @@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
> >>>>>         if (bbio->bio.bi_opf & REQ_META)
> >>>>>                 return btree_csum_one_bio(bbio);
> >>>>>     #ifdef CONFIG_BTRFS_EXPERIMENTAL
> >>>>> -     return btrfs_csum_one_bio(bbio, true);
> >>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
> >>>>>     #else
> >>>>> -     return btrfs_csum_one_bio(bbio, false);
> >>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
> >>>>>     #endif
> >>>>>     }
> >>>>>
> >>>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> >>>>> index deaeea3becf4..c5a6c66d51a0 100644
> >>>>> --- a/fs/btrfs/bio.h
> >>>>> +++ b/fs/btrfs/bio.h
> >>>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
> >>>>>                         struct btrfs_ordered_sum *sums;
> >>>>>                         struct work_struct csum_work;
> >>>>>                         struct completion csum_done;
> >>>>> +                     struct bio *csum_bio;
> >>>>>                         struct bvec_iter csum_saved_iter;
> >>>>>                         u64 orig_physical;
> >>>>>                 };
> >>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> >>>>> index 72be3ede0edf..474949074da8 100644
> >>>>> --- a/fs/btrfs/file-item.c
> >>>>> +++ b/fs/btrfs/file-item.c
> >>>>> @@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
> >>>>>         return ret;
> >>>>>     }
> >>>>>
> >>>>> -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
> >>>>> +static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
> >>>>>     {
> >>>>>         struct btrfs_inode *inode = bbio->inode;
> >>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>>>>         SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> >>>>> -     struct bio *bio = &bbio->bio;
> >>>>>         struct btrfs_ordered_sum *sums = bbio->sums;
> >>>>> -     struct bvec_iter iter = *src;
> >>>>>         phys_addr_t paddr;
> >>>>>         const u32 blocksize = fs_info->sectorsize;
> >>>>>         int index = 0;
> >>>>>
> >>>>>         shash->tfm = fs_info->csum_shash;
> >>>>>
> >>>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
> >>>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
> >>>>>                 btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
> >>>>>                 index += fs_info->csum_size;
> >>>>>         }
> >>>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
> >>>>>
> >>>>>         ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
> >>>>>         ASSERT(bbio->async_csum == true);
> >>>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
> >>>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
> >>>>>         complete(&bbio->csum_done);
> >>>>>     }
> >>>>>
> >>>>>     /*
> >>>>>      * Calculate checksums of the data contained inside a bio.
> >>>>>      */
> >>>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >>>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
> >>>>>     {
> >>>>>         struct btrfs_ordered_extent *ordered = bbio->ordered;
> >>>>>         struct btrfs_inode *inode = bbio->inode;
> >>>>>         struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >>>>> -     struct bio *bio = &bbio->bio;
> >>>>>         struct btrfs_ordered_sum *sums;
> >>>>>         unsigned nofs_flag;
> >>>>>
> >>>>> @@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >>>>>         btrfs_add_ordered_sum(ordered, sums);
> >>>>>
> >>>>>         if (!async) {
> >>>>> -             csum_one_bio(bbio, &bbio->bio.bi_iter);
> >>>>> +             struct bvec_iter iter = bio->bi_iter;
> >>>>> +             csum_one_bio(bbio, bio, &iter);
> >>>>>                 return 0;
> >>>>>         }
> >>>>>         init_completion(&bbio->csum_done);
> >>>>>         bbio->async_csum = true;
> >>>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
> >>>>> +     bbio->csum_bio = bio;
> >>>>> +     bbio->csum_saved_iter = bio->bi_iter;
> >>>>>         INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> >>>>>         schedule_work(&bbio->csum_work);
> >>>>>         return 0;
> >>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
> >>>>> index 5645c5e3abdb..d16fd2144552 100644
> >>>>> --- a/fs/btrfs/file-item.h
> >>>>> +++ b/fs/btrfs/file-item.h
> >>>>> @@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
> >>>>>     int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
> >>>>>                            struct btrfs_root *root,
> >>>>>                            struct btrfs_ordered_sum *sums);
> >>>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
> >>>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
> >>>>>     int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
> >>>>>     int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
> >>>>>                              struct list_head *list, int search_commit,
> >>>>
> >>
> >
>

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Qu Wenruo]

在 2025/11/19 18:04, Daniel Vacek 写道:
[...]
>> Unless you mean the encrypted bio is not encapsulated by btrfs_bio, but
>> a vanilla bio.
> 

Now it's completely unrelated to the patchset, but I'm still very 
interested in the workflow of fscrypt.

Is there high level docs about it?

> That's the case. The bounce bio is created when you submit the
> original one. The data is encrypted by fscrypt, then the csum hook is
> called and the new bio submitted instead of the original one.

I guess by "submit" you didn't mean btrfs_submit_bio(), but something 
else right?
Or the plaintext bio will be submitted to the disks.

> Later
> the endio frees the new one and calls endio on the original bio.

But the encrypted bio still needs to be handled by btrfs_submit_bio(), 
as we still have a lot of extra works like splitting due to stripe 
boundary and duplicating to different mirrors.

The end io function of the encrypted bio won't be called until all of 
our btrfs specific work/IO is done.

> This
> means we don't have control over the bounce bio and cannot use it
> asynchronously at the moment. The csum needs to be finished directly
> in the hook.

Any code I can look into the "hook"?

I checked the ext4's code (althought it doesn't support csum), they just 
call fscrypt_encrypt_pagecache_blocks() on the plaintext folios, and 
submit the encrypted folios as the bio payload.

It's completely fine to just add those encrypted folios into a 
btrfs_bio, and submit as usual.

I believe we can handle it inside the endio function, end_bbio_data_write().

In fact, currently for write bios, we use an empty btrfs_bio::private, 
we can definitely allocate some extra structure for encrypted writes to 
track the plain text pages (mostly just the start/end file pos).


So my uneducated guess of the encyrpted write would be something like 
the following, mostly happenin in the function submit_extent_folio():

submit_extent_folio()
{
	/* setup bio_ctrl for encrypted, involving the
          * btrfs_bio::private.
	 * So that alloc_new_bio() will allocate a new
	 * private for encrypted write.
          */
	do {
		if (is_encrypted) {
			data_folio = fscrypt_encrypt();
			ret = bio_add_folio();
		} else {
			ret = bio_add_folio();
		}
		/* the remaining as usual */
	}
}

Then in the endio do the special handling:

end_bbio_data_write()
{
	if (!bbio->private) {
		/* The existing one. */
		end_bbio_data_write_plaintext();
	} else {
		/*
		 * Using bbio->private to grab the page cache folios
		 * and free the encrypted folios.
		 */
	}
	bio_put(&bbio->bio);
}

Thus that's why I didn't see the point to trace the original bbio inside 
btrfs_bio, and also didn't see why we have any problem doing async csum.

BTW, the same can be applied to data read, as we didn't utilize 
btrfs_bio::private either.

Or did I miss something?

Thanks,
Qu

> 
> Anyways, the hook changes are not upstreamed yet, so you can only see
> it on the mailing list. And that's also why this patch makes more
> sense to be sent later together with those changes.
> 
> --nX
> 
>> In that case you can not even submit it through btrfs_submit_bbio().
>>
>> Thanks,
>> Qu
>>
>>> We'll need to
>>> always compute the checksum synchronously for encrypted bios. In that
>>> case we don't need to store it in btrfs_bio::csum_bio at all. For the
>>> regular (unencrypted) case we can keep using the &bbio->bio.
>>>
>>> I'll drop the csum_bio for there is no use really.
>>>
>>> --nX
>>>
>>>> I thought it's more common to put the original plaintext into the
>>>> encryption specific structure, like what we did for compression.
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>>
>>>>> --nX
>>>>>
>>>>>> The storage layer doesn't need to bother the plaintext bio at all, they
>>>>>> just write the encrypted one to disk.
>>>>>>
>>>>>> And it's the dm-crypto tracking the plaintext bio <-> encrypted bio mapping.
>>>>>>
>>>>>>
>>>>>> So why we can not just create a new bio for the final csum caculation,
>>>>>> just like compression?
>>>>>>
>>>>>> Thanks,
>>>>>> Qu
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>>>>>> Signed-off-by: Daniel Vacek <neelx@suse.com>
>>>>>>> ---
>>>>>>> Compared to v5 this needed to adapt to recent async csum changes.
>>>>>>> ---
>>>>>>>      fs/btrfs/bio.c       |  4 ++--
>>>>>>>      fs/btrfs/bio.h       |  1 +
>>>>>>>      fs/btrfs/file-item.c | 17 ++++++++---------
>>>>>>>      fs/btrfs/file-item.h |  2 +-
>>>>>>>      4 files changed, 12 insertions(+), 12 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
>>>>>>> index a73652b8724a..a69174b2b6b6 100644
>>>>>>> --- a/fs/btrfs/bio.c
>>>>>>> +++ b/fs/btrfs/bio.c
>>>>>>> @@ -542,9 +542,9 @@ static int btrfs_bio_csum(struct btrfs_bio *bbio)
>>>>>>>          if (bbio->bio.bi_opf & REQ_META)
>>>>>>>                  return btree_csum_one_bio(bbio);
>>>>>>>      #ifdef CONFIG_BTRFS_EXPERIMENTAL
>>>>>>> -     return btrfs_csum_one_bio(bbio, true);
>>>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, true);
>>>>>>>      #else
>>>>>>> -     return btrfs_csum_one_bio(bbio, false);
>>>>>>> +     return btrfs_csum_one_bio(bbio, &bbio->bio, false);
>>>>>>>      #endif
>>>>>>>      }
>>>>>>>
>>>>>>> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
>>>>>>> index deaeea3becf4..c5a6c66d51a0 100644
>>>>>>> --- a/fs/btrfs/bio.h
>>>>>>> +++ b/fs/btrfs/bio.h
>>>>>>> @@ -58,6 +58,7 @@ struct btrfs_bio {
>>>>>>>                          struct btrfs_ordered_sum *sums;
>>>>>>>                          struct work_struct csum_work;
>>>>>>>                          struct completion csum_done;
>>>>>>> +                     struct bio *csum_bio;
>>>>>>>                          struct bvec_iter csum_saved_iter;
>>>>>>>                          u64 orig_physical;
>>>>>>>                  };
>>>>>>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>>>>>>> index 72be3ede0edf..474949074da8 100644
>>>>>>> --- a/fs/btrfs/file-item.c
>>>>>>> +++ b/fs/btrfs/file-item.c
>>>>>>> @@ -765,21 +765,19 @@ int btrfs_lookup_csums_bitmap(struct btrfs_root *root, struct btrfs_path *path,
>>>>>>>          return ret;
>>>>>>>      }
>>>>>>>
>>>>>>> -static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src)
>>>>>>> +static void csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, struct bvec_iter *iter)
>>>>>>>      {
>>>>>>>          struct btrfs_inode *inode = bbio->inode;
>>>>>>>          struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>>>>          SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
>>>>>>> -     struct bio *bio = &bbio->bio;
>>>>>>>          struct btrfs_ordered_sum *sums = bbio->sums;
>>>>>>> -     struct bvec_iter iter = *src;
>>>>>>>          phys_addr_t paddr;
>>>>>>>          const u32 blocksize = fs_info->sectorsize;
>>>>>>>          int index = 0;
>>>>>>>
>>>>>>>          shash->tfm = fs_info->csum_shash;
>>>>>>>
>>>>>>> -     btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
>>>>>>> +     btrfs_bio_for_each_block(paddr, bio, iter, blocksize) {
>>>>>>>                  btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
>>>>>>>                  index += fs_info->csum_size;
>>>>>>>          }
>>>>>>> @@ -791,19 +789,18 @@ static void csum_one_bio_work(struct work_struct *work)
>>>>>>>
>>>>>>>          ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>>>>>>>          ASSERT(bbio->async_csum == true);
>>>>>>> -     csum_one_bio(bbio, &bbio->csum_saved_iter);
>>>>>>> +     csum_one_bio(bbio, bbio->csum_bio, &bbio->csum_saved_iter);
>>>>>>>          complete(&bbio->csum_done);
>>>>>>>      }
>>>>>>>
>>>>>>>      /*
>>>>>>>       * Calculate checksums of the data contained inside a bio.
>>>>>>>       */
>>>>>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>>>>>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async)
>>>>>>>      {
>>>>>>>          struct btrfs_ordered_extent *ordered = bbio->ordered;
>>>>>>>          struct btrfs_inode *inode = bbio->inode;
>>>>>>>          struct btrfs_fs_info *fs_info = inode->root->fs_info;
>>>>>>> -     struct bio *bio = &bbio->bio;
>>>>>>>          struct btrfs_ordered_sum *sums;
>>>>>>>          unsigned nofs_flag;
>>>>>>>
>>>>>>> @@ -822,12 +819,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>>>>>>>          btrfs_add_ordered_sum(ordered, sums);
>>>>>>>
>>>>>>>          if (!async) {
>>>>>>> -             csum_one_bio(bbio, &bbio->bio.bi_iter);
>>>>>>> +             struct bvec_iter iter = bio->bi_iter;
>>>>>>> +             csum_one_bio(bbio, bio, &iter);
>>>>>>>                  return 0;
>>>>>>>          }
>>>>>>>          init_completion(&bbio->csum_done);
>>>>>>>          bbio->async_csum = true;
>>>>>>> -     bbio->csum_saved_iter = bbio->bio.bi_iter;
>>>>>>> +     bbio->csum_bio = bio;
>>>>>>> +     bbio->csum_saved_iter = bio->bi_iter;
>>>>>>>          INIT_WORK(&bbio->csum_work, csum_one_bio_work);
>>>>>>>          schedule_work(&bbio->csum_work);
>>>>>>>          return 0;
>>>>>>> diff --git a/fs/btrfs/file-item.h b/fs/btrfs/file-item.h
>>>>>>> index 5645c5e3abdb..d16fd2144552 100644
>>>>>>> --- a/fs/btrfs/file-item.h
>>>>>>> +++ b/fs/btrfs/file-item.h
>>>>>>> @@ -64,7 +64,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>>>>>>>      int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>>>>>>>                             struct btrfs_root *root,
>>>>>>>                             struct btrfs_ordered_sum *sums);
>>>>>>> -int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async);
>>>>>>> +int btrfs_csum_one_bio(struct btrfs_bio *bbio, struct bio *bio, bool async);
>>>>>>>      int btrfs_alloc_dummy_sum(struct btrfs_bio *bbio);
>>>>>>>      int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>>>>>>>                               struct list_head *list, int search_commit,
>>>>>>
>>>>
>>>
>>

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Christoph Hellwig]

On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> That's the case. The bounce bio is created when you submit the
> original one. The data is encrypted by fscrypt, then the csum hook is
> called and the new bio submitted instead of the original one. Later
> the endio frees the new one and calls endio on the original bio. This
> means we don't have control over the bounce bio and cannot use it
> asynchronously at the moment. The csum needs to be finished directly
> in the hook.

And as I told you that can be changed.  Please get your entire series
out of review to allow people to try to review what you're trying to
do.

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Daniel Vacek]

On Wed, 19 Nov 2025 at 09:22, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> > That's the case. The bounce bio is created when you submit the
> > original one. The data is encrypted by fscrypt, then the csum hook is
> > called and the new bio submitted instead of the original one. Later
> > the endio frees the new one and calls endio on the original bio. This
> > means we don't have control over the bounce bio and cannot use it
> > asynchronously at the moment. The csum needs to be finished directly
> > in the hook.
>
> And as I told you that can be changed.  Please get your entire series
> out of review to allow people to try to review what you're trying to
> do.

It's coming. Stay tuned! I'm just finishing a bit of re-design to
btrfs crypt context metadata storing which was suggested in code
review of matching changes in btrfs-progs. The fscrypt part is mostly
without any changes to the old v5 series from Josef.

--nX

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Christoph Hellwig]

On Wed, Nov 19, 2025 at 10:28:55AM +0100, Daniel Vacek wrote:
> On Wed, 19 Nov 2025 at 09:22, Christoph Hellwig <hch@infradead.org> wrote:
> > On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> > > That's the case. The bounce bio is created when you submit the
> > > original one. The data is encrypted by fscrypt, then the csum hook is
> > > called and the new bio submitted instead of the original one. Later
> > > the endio frees the new one and calls endio on the original bio. This
> > > means we don't have control over the bounce bio and cannot use it
> > > asynchronously at the moment. The csum needs to be finished directly
> > > in the hook.
> >
> > And as I told you that can be changed.  Please get your entire series
> > out of review to allow people to try to review what you're trying to
> > do.
> 
> It's coming. Stay tuned! I'm just finishing a bit of re-design to
> btrfs crypt context metadata storing which was suggested in code
> review of matching changes in btrfs-progs. The fscrypt part is mostly
> without any changes to the old v5 series from Josef.

The point is that anything directly related should be presented
together.  Patches 1-3 don't make sense without the rest.  And
especially for patch 3 I'm really doubtful it is a good idea to
start with, but that can only be argued when the reset is shown.

Re: [PATCH v6 3/8] btrfs: add a bio argument to btrfs_csum_one_bio

[Daniel Vacek]

On Wed, 19 Nov 2025 at 10:32, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Nov 19, 2025 at 10:28:55AM +0100, Daniel Vacek wrote:
> > On Wed, 19 Nov 2025 at 09:22, Christoph Hellwig <hch@infradead.org> wrote:
> > > On Wed, Nov 19, 2025 at 08:34:13AM +0100, Daniel Vacek wrote:
> > > > That's the case. The bounce bio is created when you submit the
> > > > original one. The data is encrypted by fscrypt, then the csum hook is
> > > > called and the new bio submitted instead of the original one. Later
> > > > the endio frees the new one and calls endio on the original bio. This
> > > > means we don't have control over the bounce bio and cannot use it
> > > > asynchronously at the moment. The csum needs to be finished directly
> > > > in the hook.
> > >
> > > And as I told you that can be changed.  Please get your entire series
> > > out of review to allow people to try to review what you're trying to
> > > do.
> >
> > It's coming. Stay tuned! I'm just finishing a bit of re-design to
> > btrfs crypt context metadata storing which was suggested in code
> > review of matching changes in btrfs-progs. The fscrypt part is mostly
> > without any changes to the old v5 series from Josef.
>
> The point is that anything directly related should be presented
> together.  Patches 1-3 don't make sense without the rest.  And
> especially for patch 3 I'm really doubtful it is a good idea to
> start with, but that can only be argued when the reset is shown.

Oh, sorry I didn't say that out loud. Maybe you missed it, I already
dropped this patch (and another one) from v7 yesterday.

The patch itself is still the same as it was in v4 or v5 where you did
not object. I'd say let's discuss it again when I'll send out the rest
of the series so that we have the full picture. Maybe the refactoring
you mentioned could provide us with a better way for checksumming.

About patches 1 and 2, it's really up to Dave. He picked them and
asked me to send them ahead. I'm perfectly fine either way.

--nX