Re: [PATCH v6 28/43] btrfs: populate ordered_extent with the orig offset

[Daniel Vacek]

On Sun, 8 Feb 2026 at 16:14, Chris Mason <clm@xxxxxxxx> wrote:
> Daniel Vacek <neelx@xxxxxxxx> wrote:
> > From: Josef Bacik <josef@xxxxxxxxxxxxxx>
> >
> > 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 <josef@xxxxxxxxxxxxxx>
>
> 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.
>