Re: [PATCHv3 0/8] direct-io: even more flexible io vectors

Re: [PATCHv3 0/8] direct-io: even more flexible io vectors

[Mike Snitzer]

On Tue, Aug 19, 2025 at 09:49:14AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> Previous version:
> 
>   https://lore.kernel.org/linux-block/20250805141123.332298-1-kbusch@meta.com/
> 
> This series removes the direct io requirement that io vector lengths
> align to the logical block size.
> 
> I tested this on a few raw block device types including nvme,
> virtio-blk, ahci, and loop. NVMe is the only one I tested with 4k
> logical sectors; everything else was 512.
> 
> On each of those, I tested several iomap filesystems: xfs, ext4, and
> btrfs. I found it interesting that each behave a little
> differently with handling invalid vector alignments:
> 
>   - XFS is the most straight forward and reports failures on invalid
>     vector conditions, same as raw blocks devices.
> 
>   - EXT4 falls back to buffered io for writes but not for reads.
> 
>   - BTRFS doesn't even try direct io for any unusual alignments; it
>     chooses buffered io from the start.
> 
> So it has been a little slow going figuring out which results to expect
> from various tests, but I think I've got all the corner cases covered. I
> can submit the tests cases to blktests and fstests for consideration
> separately, too.
> 
> I'm not 100% sure where we're at with the last patch. I think Mike
> initially indicated this was okay to remove, but I could swear I read
> something saying that might not be the case anymore. I just can't find
> the message now. Mike?

Hey,

Yes, I don't have pointers immediately available but I did mention it
and cc'd you.  I have found that my work relative to NFS and NFSD does
still need to use iov_iter_aligned_bvec -- otherwise misaligned DIO
can get issued to the underlying filesystem.

I did try to push all the relevant checking down to NFS/NFSD code that
assembles their respective bvec into an iov_iter, like you suggested,
but came up short after my first attempt.

I don't want to speak for the NFS or NFSD miantainers, but I'm
personally still OK with the broader iov_iter_is_aligned() interface
and even iov_iter_aligned_bvec() going away (and NFS/NFSD carrying
their own until I can circle back to hopefully eliminating the need).

Either that, or we remove all but iov_iter_aligned_bvec() and export
it so that NFS/NFSD can use it, _and_  tweak it so that it offers more
coarse-grained length checking, like so:
https://lore.kernel.org/linux-nfs/20250708160619.64800-5-snitzer@kernel.org/
(this is probably the best intermediate solution actually, though it'd
force my NFS and NFSD changes to be dependent on your series landing
-- which is probably a perfectly appropriate constraint)

Thanks,
Mike


> 
> Changes from v2:
> 
>   Include vector lengths when validating a split. The length check is
>   only valid for r/w commands, and skipped for passthrough
>   DRV_IN/DRV_OUT commands.
> 
>   Introduce a prep patch having bio_iov_iter_get_pages() take the
>   caller's desired length alignment.
> 
>   Additional code comments explaing less obvious error conditions.
> 
>   Added reviews on the patches that haven't changed.
> 
> Keith Busch (8):
>   block: check for valid bio while splitting
>   block: add size alignment to bio_iov_iter_get_pages
>   block: align the bio after building it
>   block: simplify direct io validity check
>   iomap: simplify direct io validity check
>   block: remove bdev_iter_is_aligned
>   blk-integrity: use simpler alignment check
>   iov_iter: remove iov_iter_is_aligned
> 
>  block/bio-integrity.c  |  4 +-
>  block/bio.c            | 64 ++++++++++++++++++----------
>  block/blk-map.c        |  2 +-
>  block/blk-merge.c      | 20 +++++++--
>  block/fops.c           | 13 +++---
>  fs/iomap/direct-io.c   |  6 +--
>  include/linux/bio.h    | 13 ++++--
>  include/linux/blkdev.h | 20 +++++----
>  include/linux/uio.h    |  2 -
>  lib/iov_iter.c         | 95 ------------------------------------------
>  10 files changed, 94 insertions(+), 145 deletions(-)
> 
> -- 
> 2.47.3
>