Re: [PATCHv4 5/8] iomap: simplify direct io validity check

Eric Biggers
2025-10-28T23:03:52Z

On Tue, Oct 28, 2025 at 10:56:48PM +0000, Eric Biggers wrote:
> On Tue, Oct 28, 2025 at 10:47:53PM +0000, Carlos Llamas wrote:
> > Ok, I did a bit more digging. I'm using f2fs but the problem in this
> > case is the blk_crypto layer. The OP_READ request goes through
> > submit_bio() which then calls blk_crypto_bio_prep() and if the bio has
> > crypto context then it checks for bio_crypt_check_alignment().
> >
> > This is where the LTP tests fails the alignment. However, the propagated
> > error goes through "bio->bi_status = BLK_STS_IOERR" which in bio_endio()
> > get translates to EIO due to blk_status_to_errno().
> >
> > I've verified this restores the original behavior matching the LTP test,
> > so I'll write up a patch and send it a bit later.
> >
> > diff --git a/block/blk-crypto.c b/block/blk-crypto.c
> > index 1336cbf5e3bd..a417843e7e4a 100644
> > --- a/block/blk-crypto.c
> > +++ b/block/blk-crypto.c
> > @@ -293,7 +293,7 @@ bool __blk_crypto_bio_prep(struct bio **bio_ptr)
> > }
> >
> > if (!bio_crypt_check_alignment(bio)) {
> > - bio->bi_status = BLK_STS_IOERR;
> > + bio->bi_status = BLK_STS_INVAL;
> > goto fail;
> > }
>
> That change looks fine, but I'm wondering how this case was reached in
> the first place. Upper layers aren't supposed to be submitting
> misaligned bios like this. For example, ext4 and f2fs require
> filesystem logical block size alignment for direct I/O on encrypted
> files. They check for this early, before getting to the point of
> submitting a bio, and fall back to buffered I/O if needed.

I suppose it's this code in f2fs_should_use_dio():

/*
* Direct I/O not aligned to the disk's logical_block_size will be
* attempted, but will fail with -EINVAL.
*
* f2fs additionally requires that direct I/O be aligned to the
* filesystem block size, which is often a stricter requirement.
* However, f2fs traditionally falls back to buffered I/O on requests
* that are logical_block_size-aligned but not fs-block aligned.
*
* The below logic implements this behavior.
*/
align = iocb->ki_pos | iov_iter_alignment(iter);
if (!IS_ALIGNED(align, i_blocksize(inode)) &&
IS_ALIGNED(align, bdev_logical_block_size(inode->i_sb->s_bdev)))
return false;

So it relies on the alignment check in iomap in the case where the
request is neither logical_block_size nor filesystem_block_size aligned.

f2fs_should_use_dio() probably should just handle that case explicitly.

But making __blk_crypto_bio_prep() use a better error code sounds good
too.

- Eric