From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb76e-00023q-Er for qemu-devel@nongnu.org; Wed, 21 Nov 2012 05:00:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tb76Y-0004uW-GC for qemu-devel@nongnu.org; Wed, 21 Nov 2012 05:00:32 -0500 Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:53660) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb76Y-0004tE-5s for qemu-devel@nongnu.org; Wed, 21 Nov 2012 05:00:26 -0500 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 21 Nov 2012 10:00:23 -0000 Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qALA0E4K60031160 for ; Wed, 21 Nov 2012 10:00:14 GMT Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qALA0LmZ011653 for ; Wed, 21 Nov 2012 03:00:21 -0700 Message-ID: <50ACA634.8040002@de.ibm.com> Date: Wed, 21 Nov 2012 11:00:20 +0100 From: Christian Borntraeger MIME-Version: 1.0 References: <1353488287-47077-1-git-send-email-borntraeger@de.ibm.com> <50AC9B96.9070908@redhat.com> In-Reply-To: <50AC9B96.9070908@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH/RFC] block: Ensure that block size constraints are considered List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , Heinz Graalfs Cc: jfrei@linux.vnet.ibm.com, qemu-devel@nongnu.org, Stefan Hajnoczi , agraf@suse.de On 21/11/12 10:15, Kevin Wolf wrote: > Am 21.11.2012 09:58, schrieb Christian Borntraeger: >> From: Heinz Graalfs >> >> While testing IPL code (booting) for s390x we faced some problems >> with cache=none on dasds (4k block size) on bdrv_preads with length >> values != block size. >> >> This patch makes sure that bdrv_pread and friends work fine with >> unaligned access even with cache=none >> - propagate alignment value also into bs->file struct >> - modify the size in case of no cache to avoid EINVAL on >> pread() etc. (file was opened with O_DIRECT). >> >> This patch seems to cure the problems. >> >> CC: Kevin Wolf >> CC: Stefan Hajnoczi >> Signed-off-by: Heinz Graalfs >> Signed-off-by: Christian Borntraeger >> --- >> block.c | 3 +++ >> block/raw-posix.c | 6 ++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/block.c b/block.c >> index 854ebd6..f23c562 100644 >> --- a/block.c >> +++ b/block.c >> @@ -4242,6 +4242,9 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, >> void bdrv_set_buffer_alignment(BlockDriverState *bs, int align) >> { >> bs->buffer_alignment = align; >> + if ((bs->open_flags & BDRV_O_NOCACHE)) { >> + bs->file->buffer_alignment = align; >> + } > > Any reason to restrict this to BDRV_O_NOCACHE? > > There have been patches to change the BDRV_O_NOCACHE flag from the > monitor, in which case bdrv_set_buffer_alignment() wouldn't be called > anew and O_DIRECT requests start to fail again. Right, should be ok to remove the check. > >> } >> >> void *qemu_blockalign(BlockDriverState *bs, size_t size) >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index f2f0404..baebf1d 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -700,6 +700,12 @@ static BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd, >> acb->aio_nbytes = nb_sectors * 512; >> acb->aio_offset = sector_num * 512; >> >> + /* O_DIRECT also requires an aligned length */ >> + if (bs->open_flags & BDRV_O_NOCACHE) { >> + acb->aio_nbytes += acb->bs->buffer_alignment - 1; >> + acb->aio_nbytes &= ~(acb->bs->buffer_alignment - 1); >> + } > > Modifying aio_nbytes, but not the iov looks wrong to me. This may work > in the handle_aiocb_rw_linear() code path, but not with actual vectored I/O. I think it seemed to work because the vectored I/O cases that we were testing were properly aligned or were in the QEMU_AIO_MISALIGNED case which does bounce buffering anyway. But I am not sure... Heinz can you have a look at this and identify the exact place were it was failing and why this patch helps? (I just know it does). That might help to understand if we also need to touch the iovs. Christian