Re: [PATCH v11 03/14] btrfs: don't advance offset for compressed bios in btrfs_csum_one_bio()

[Omar Sandoval <osandov@osandov.com>]

On Thu, Oct 14, 2021 at 03:05:54PM +0300, Nikolay Borisov wrote:
> 
> 
> On 1.09.21 г. 20:00, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > btrfs_csum_one_bio() loops over each filesystem block in the bio while
> > keeping a cursor of its current logical position in the file in order to
> > look up the ordered extent to add the checksums to. However, this
> > doesn't make much sense for compressed extents, as a sector on disk does
> > not correspond to a sector of decompressed file data. It happens to work
> > because 1) the compressed bio always covers one ordered extent and 2)
> > the size of the bio is always less than the size of the ordered extent.
> > However, the second point will not always be true for encoded writes.
> > 
> > Let's add a boolean parameter to btrfs_csum_one_bio() to indicate that
> > it can assume that the bio only covers one ordered extent. Since we're
> > already changing the signature, let's get rid of the contig parameter
> > and make it implied by the offset parameter, similar to the change we
> > recently made to btrfs_lookup_bio_sums(). Additionally, let's rename
> > nr_sectors to blockcount to make it clear that it's the number of
> > filesystem blocks, not the number of 512-byte sectors.
> > 
> > Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> 
> Code-wise this looks, though I don't know why we are guaranteed that a
> compressed extent will only cover a single OE.

See submit_compressed_extents(): we always add an ordered extent that
covers the whole range that we're submitting, and then
btrfs_submit_compressed_write() creates one or more bios that are
subsets of that range.

> But I trust you so:
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>

Thanks!