On Sun, 8 Feb 2026 at 16:14, Chris Mason <[email protected]> wrote: > 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?
IIUC, no. We need to keep the offset from the original extent precisely to keep the data consistent. That's the whole point. --nX > 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. >
