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

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 5/20/22 02:19, Eric Biggers wrote:
> On Thu, May 19, 2022 at 08:45:55AM +0200, Damien Le Moal wrote:
>> On 2022/05/19 6:56, Keith Busch wrote:
>>> On Wed, May 18, 2022 at 08:27:31PM -0700, Eric Biggers wrote:
>>>>
>>>> So the bio ends up with a total length that is a multiple of the logical block
>>>> size, but the lengths of the individual bvecs in the bio are *not* necessarily
>>>> multiples of the logical block size.  That's the problem.
>>>
>>> I'm surely missing something here. I know the bvecs are not necessarily lbs
>>> aligned, but why does that matter? Is there some driver that can only take
>>> exactly 1 bvec, but allows it to be unaligned? If so, we could take the segment
>>> queue limit into account, but I am not sure that we need to.
>>
>> For direct IO, the first bvec will always be aligned to a logical block size.
>> See __blkdev_direct_IO() and __blkdev_direct_IO_simple():
>>
>>         if ((pos | iov_iter_alignment(iter)) &
>>             (bdev_logical_block_size(bdev) - 1))
>>                 return -EINVAL;
>>
>> And given that, all bvecs should also be LBA aligned since the LBA size is
>> always a divisor of the page size. Since splitting is always done on an LBA
>> boundary, I do not see how we can ever get bvecs that are not LBA aligned.
>> Or I am missing something too...
>>
> 
> You're looking at the current upstream code.  This patch changes that to:
> 
> 	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;
> 
> So, if this patch is accepted, then the file position and total I/O length will
> need to be logical block aligned (as before), but the memory address and length
> of each individual iovec will no longer need to be logical block aligned.  How
> the iovecs get turned into bios (and thus bvecs) is a little complicated, but
> the result is that logical blocks will be able to span bvecs.

Indeed. I missed that change in patch 3. Got it.

> 
> - Eric


-- 
Damien Le Moal
Western Digital Research