[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] block: use the request length for iov alignment
From: |
Kevin Wolf |
Subject: |
Re: [PATCH] block: use the request length for iov alignment |
Date: |
Wed, 14 Sep 2022 11:36:14 +0100 |
Am 13.09.2022 um 15:12 hat Keith Busch geschrieben:
> 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.
I need to take a real look after KVM Forum, but my first thought was
that we might be overloading request_alignment with multiple meanings
now (file offset alignment and memory address alignment), and the values
just happen to be the same for files on Linux.
Did you consider a separate iov_alignment or similar and intentionally
decided against it, or is it something you just didn't think about?
Kevin
> > ---
> > block/io.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/io.c b/block/io.c
> > index 0a8cbefe86..296d4b49a7 100644
> > --- a/block/io.c
> > +++ b/block/io.c
> > @@ -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;
> > }
> > }
> > --
> > 2.30.2