From: Eric Biggers <ebiggers@kernel.org>
To: Carlos Llamas <cmllamas@google.com>
Cc: Keith Busch <kbusch@kernel.org>, Keith Busch <kbusch@meta.com>,
linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org,
hch@lst.de, axboe@kernel.dk, Hannes Reinecke <hare@suse.de>,
"Martin K. Petersen" <martin.petersen@oracle.com>
Subject: Re: [PATCHv4 5/8] iomap: simplify direct io validity check
Date: Tue, 28 Oct 2025 23:03:50 +0000 [thread overview]
Message-ID: <20251028230350.GB1639650@google.com> (raw)
In-Reply-To: <20251028225648.GA1639650@google.com>
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
next prev parent reply other threads:[~2025-10-28 23:03 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-27 14:12 [PATCHv4 0/8] Keith Busch
2025-08-27 14:12 ` [PATCHv4 1/8] block: check for valid bio while splitting Keith Busch
2025-08-31 0:40 ` Martin K. Petersen
2025-08-27 14:12 ` [PATCHv4 2/8] block: add size alignment to bio_iov_iter_get_pages Keith Busch
2025-08-31 0:40 ` Martin K. Petersen
2025-08-27 14:12 ` [PATCHv4 3/8] block: align the bio after building it Keith Busch
2025-08-31 0:41 ` Martin K. Petersen
2025-09-02 5:23 ` Christoph Hellwig
2025-08-27 14:12 ` [PATCHv4 4/8] block: simplify direct io validity check Keith Busch
2025-08-27 14:12 ` [PATCHv4 5/8] iomap: " Keith Busch
2025-10-27 16:25 ` Carlos Llamas
2025-10-27 16:42 ` Keith Busch
2025-10-27 17:12 ` Carlos Llamas
2025-10-28 22:47 ` Carlos Llamas
2025-10-28 22:56 ` Eric Biggers
2025-10-28 23:03 ` Eric Biggers [this message]
2025-10-29 7:06 ` Christoph Hellwig
2025-10-30 17:40 ` Eric Biggers
2025-10-31 9:18 ` Christoph Hellwig
2025-11-03 18:10 ` Eric Biggers
2025-11-03 18:26 ` Keith Busch
2025-11-04 11:35 ` Christoph Hellwig
2025-10-30 4:54 ` Carlos Llamas
2025-08-27 14:12 ` [PATCHv4 6/8] block: remove bdev_iter_is_aligned Keith Busch
2025-08-27 14:12 ` [PATCHv4 7/8] blk-integrity: use simpler alignment check Keith Busch
2025-08-27 14:12 ` [PATCHv4 8/8] iov_iter: remove iov_iter_is_aligned Keith Busch
2025-09-09 16:27 ` [PATCHv4 0/8] Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251028230350.GB1639650@google.com \
--to=ebiggers@kernel.org \
--cc=axboe@kernel.dk \
--cc=cmllamas@google.com \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=kbusch@kernel.org \
--cc=kbusch@meta.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=martin.petersen@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox