Re: [PATCHv2 3/3] block: relax direct io memory alignment

[Eric Biggers <ebiggers@kernel.org>]

On Wed, May 18, 2022 at 07:00:39PM -0600, Keith Busch wrote:
> On Wed, May 18, 2022 at 05:14:49PM -0700, Eric Biggers wrote:
> > On Wed, May 18, 2022 at 10:11:31AM -0700, Keith Busch wrote:
> > > diff --git a/block/fops.c b/block/fops.c
> > > index b9b83030e0df..d8537c29602f 100644
> > > --- a/block/fops.c
> > > +++ b/block/fops.c
> > > @@ -54,8 +54,9 @@ static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
> > >  	struct bio bio;
> > >  	ssize_t ret;
> > >  
> > > -	if ((pos | iov_iter_alignment(iter)) &
> > > -	    (bdev_logical_block_size(bdev) - 1))
> > > +	if ((pos | iov_iter_count(iter)) & (bdev_logical_block_size(bdev) - 1))
> > > +		return -EINVAL;
> > > +	if (iov_iter_alignment(iter) & bdev_dma_alignment(bdev))
> > >  		return -EINVAL;
> > 
> > The block layer makes a lot of assumptions that bios can be split at any bvec
> > boundary.  With this patch, bios whose length isn't a multiple of the logical
> > block size can be generated by splitting, which isn't valid.
> 
> How? This patch ensures every segment is block size aligned.

No, it doesn't.  It ensures that the *total* length of each bio is logical block
size aligned.  It doesn't ensure that for the individual bvecs.  By decreasing
the required memory alignment to below the logical block size, you're allowing
logical blocks to span a page boundary.  Whenever the two pages involved aren't
physically contiguous, the data of the block will be split across two bvecs.

> > Also some devices aren't compatible with logical blocks spanning bdevs at all.
> > dm-crypt errors out in this case, for example.
> 
> I'm sorry, but I am not understanding this.

I meant to write bvecs, not bdevs.

- Eric