Daniel Vacek <[email protected]> wrote: > From: Josef Bacik <[email protected]> > > For extent encryption we have to use a logical block nr as input for the > IV. For btrfs we're using the offset into the extent we're operating > on. For most ordered extents this is the same as the file_offset, > however for prealloc and NOCOW we have to use the original offset. > > Add this as an argument and plumb it through everywhere, this will be > used when setting up the bio. >
Hi everyone, I'm running the btrfs for-next branch, along with the encryption patch set through my AI review prompts: https://github.com/masoncl/review-prompts I'm working on some btrfs specific instructions, but hopefully these reviews are already useful. I've kept the headers to make it easier to double check that I'm actually replying to the right email, but please let me know if any parts of this are incorrect: commit 0e7f6a617a2642bbdf97bcd639fb887fa88189ad Author: Josef Bacik <[email protected]> btrfs: populate ordered_extent with the orig offset This commit adds orig_offset tracking to btrfs_ordered_extent and btrfs_file_extent structures for extent encryption support, where the offset into the extent is needed for IV generation. > diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c > index d3789109ca851..2d89ac05b1b38 100644 > --- a/fs/btrfs/direct-io.c > +++ b/fs/btrfs/direct-io.c > @@ -155,6 +155,7 @@ static struct extent_map *btrfs_create_dio_extent(struct > btrfs_inode *inode, > file_extent->fscrypt_info = em->fscrypt_info; > } > > + file_extent->orig_offset = start - file_extent->offset; > ordered = btrfs_alloc_ordered_extent(inode, start, file_extent, > (1U << type) | > (1U << BTRFS_ORDERED_DIRECT)); [ ... ] > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index fbb968d4010b7..8078049f690e1 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c [ ... ] > @@ -2296,6 +2298,8 @@ static noinline int run_delalloc_nocow(struct > btrfs_inode *inode, > cow_start = (u64)-1; > } > > + nocow_args.file_extent.orig_offset = > + found_key.offset - nocow_args.file_extent.offset; > ret = nocow_one_range(inode, locked_folio, &cached_state, > &nocow_args, cur_offset, > extent_type == > BTRFS_FILE_EXTENT_PREALLOC); Should this calculation use cur_offset instead of found_key.offset? Looking at the DIO path in btrfs_create_dio_extent() (direct-io.c line 158), the orig_offset is calculated as: file_extent->orig_offset = start - file_extent->offset; where 'start' is the current write position. In run_delalloc_nocow(), found_key.offset represents where the extent item is stored, while cur_offset represents the current write position (analogous to 'start' in the DIO path). When writing to a position within an existing extent (not at its start), using found_key.offset - file_extent.offset could produce incorrect values. For consistency with the DIO path, would the formula be: nocow_args.file_extent.orig_offset = cur_offset - nocow_args.file_extent.offset; This would ensure that NOCOW writes calculate orig_offset the same way as DIO writes, which matters for encryption IV generation where an incorrect offset would cause data corruption on reads.
