Re: [PATCH] block: use the request length for iov alignment

[Damien Le Moal]

On 2022/09/13 15:51, Keith Busch wrote:
> On Tue, Sep 13, 2022 at 03:20:23PM +0100, Damien Le Moal wrote:
>> On 2022/09/13 15:12, Keith Busch wrote:
>>> On Thu, Sep 08, 2022 at 09:45:26AM -0700, Keith Busch wrote:
>>>> From: Keith Busch <kbusch@kernel.org>
>>>>
>>>> An iov length needs to be aligned to the logical block size, which may
>>>> be larger than the memory alignment.
>>>
>>> [cc'ing some other interested folks]
>>>
>>> Any thoughts on this patch? It is fixing an observed IO error  when running
>>> virtio-blk with the default 512b logical block size backed by a drive 
>>> formatted
>>> with 4k logical block.
>>
>> The patch look OK to me, but having virtio expose a 512B LBA size for a 
>> backing
>> device that has 4K LBAs will break all IOs if caching is turned off (direct 
>> IOs
>> case), even if this patch is applied. No ?
> 
> Oh, as to why that type of setup "works" with O_DIRECT, when the check below
> returns 'false', qemu allocates a bounce buffer. We want that to happen if the
> guest's virtio driver tries to read/write 512b. The lengths just need to be
> checked against the backing store's block size instead of the memory address
> alignment.

Ah ! Got it. Thanks !

> 
>>>> @@ -3243,13 +3243,14 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
>>>> QEMUIOVector *qiov)
>>>>  {
>>>>      int i;
>>>>      size_t alignment = bdrv_min_mem_align(bs);
>>>> +    size_t len = bs->bl.request_alignment;
>>>>      IO_CODE();
>>>>  
>>>>      for (i = 0; i < qiov->niov; i++) {
>>>>          if ((uintptr_t) qiov->iov[i].iov_base % alignment) {
>>>>              return false;
>>>>          }
>>>> -        if (qiov->iov[i].iov_len % alignment) {
>>>> +        if (qiov->iov[i].iov_len % len) {
>>>>              return false;
>>>>          }
>>>>      }

-- 
Damien Le Moal
Western Digital Research