From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52490) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W5azX-0008Eh-3S for qemu-devel@nongnu.org; Tue, 21 Jan 2014 08:03:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W5azR-000178-2N for qemu-devel@nongnu.org; Tue, 21 Jan 2014 08:03:43 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:47934) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W5azQ-00016u-EP for qemu-devel@nongnu.org; Tue, 21 Jan 2014 08:03:36 -0500 Date: Tue, 21 Jan 2014 14:03:35 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140121130335.GC9834@irqsave.net> References: <1389968119-24771-1-git-send-email-kwolf@redhat.com> <1389968119-24771-9-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1389968119-24771-9-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 08/29] raw: Probe required direct I/O alignment List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pl@kamp.de, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, xiawenc@linux.vnet.ibm.com Le Friday 17 Jan 2014 =E0 15:14:58 (+0100), Kevin Wolf a =E9crit : > From: Paolo Bonzini >=20 > Add a bs->request_alignment field that contains the required > offset/length alignment for I/O requests and fill it in the raw block > drivers. Use ioctls if possible, else see what alignment it takes for > O_DIRECT to succeed. >=20 > While at it, also expose the memory alignment requirements, which may b= e > (and in practice are) different from the disk alignment requirements. >=20 > Signed-off-by: Paolo Bonzini > Signed-off-by: Kevin Wolf > Reviewed-by: Max Reitz > --- > block.c | 3 ++ > block/raw-posix.c | 102 ++++++++++++++++++++++++++++++++++++++= -------- > block/raw-win32.c | 41 +++++++++++++++++++ > include/block/block_int.h | 3 ++ > 4 files changed, 132 insertions(+), 17 deletions(-) >=20 > diff --git a/block.c b/block.c > index b738abe..25ae896 100644 > --- a/block.c > +++ b/block.c > @@ -813,6 +813,7 @@ static int bdrv_open_common(BlockDriverState *bs, B= lockDriverState *file, > =20 > bs->open_flags =3D flags; > bs->guest_block_size =3D 512; > + bs->request_alignment =3D 512; > bs->zero_beyond_eof =3D true; > open_flags =3D bdrv_open_flags(bs, flags); > bs->read_only =3D !(open_flags & BDRV_O_RDWR); > @@ -881,6 +882,8 @@ static int bdrv_open_common(BlockDriverState *bs, B= lockDriverState *file, > } > =20 > bdrv_refresh_limits(bs); > + assert(bdrv_opt_mem_align(bs) !=3D 0); > + assert(bs->request_alignment !=3D 0); > =20 > #ifndef _WIN32 > if (bs->is_temporary) { > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 0676037..126a634 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -127,6 +127,8 @@ typedef struct BDRVRawState { > int fd; > int type; > int open_flags; > + size_t buf_align; > + > #if defined(__linux__) > /* linux floppy specific */ > int64_t fd_open_time; > @@ -213,6 +215,76 @@ static int raw_normalize_devicepath(const char **f= ilename) > } > #endif > =20 > +static void raw_probe_alignment(BlockDriverState *bs) > +{ > + BDRVRawState *s =3D bs->opaque; > + char *buf; > + unsigned int sector_size; > + > + /* For /dev/sg devices the alignment is not really used. > + With buffered I/O, we don't have any restrictions. */ > + if (bs->sg || !(s->open_flags & O_DIRECT)) { > + bs->request_alignment =3D 1; > + s->buf_align =3D 1; > + return; > + } > + > + /* Try a few ioctls to get the right size */ > + bs->request_alignment =3D 0; > + s->buf_align =3D 0; > + > +#ifdef BLKSSZGET > + if (ioctl(s->fd, BLKSSZGET, §or_size) >=3D 0) { > + bs->request_alignment =3D sector_size; > + } > +#endif > +#ifdef DKIOCGETBLOCKSIZE > + if (ioctl(s->fd, DKIOCGETBLOCKSIZE, §or_size) >=3D 0) { > + bs->request_alignment =3D sector_size; > + } > +#endif > +#ifdef DIOCGSECTORSIZE > + if (ioctl(s->fd, DIOCGSECTORSIZE, §or_size) >=3D 0) { > + bs->request_alignment =3D sector_size; > + } > +#endif > +#ifdef CONFIG_XFS > + if (s->is_xfs) { > + struct dioattr da; > + if (xfsctl(NULL, s->fd, XFS_IOC_DIOINFO, &da) >=3D 0) { > + bs->request_alignment =3D da.d_miniosz; > + /* The kernel returns wrong information for d_mem */ > + /* s->buf_align =3D da.d_mem; */ > + } > + } > +#endif > + > + /* If we could not get the sizes so far, we can only guess them */ > + if (!s->buf_align) { > + size_t align; > + buf =3D qemu_memalign(MAX_BLOCKSIZE, 2 * MAX_BLOCKSIZE); > + for (align =3D 512; align <=3D MAX_BLOCKSIZE; align <<=3D 1) { > + if (pread(s->fd, buf + align, MAX_BLOCKSIZE, 0) >=3D 0) { > + s->buf_align =3D align; > + break; > + } > + } > + qemu_vfree(buf); > + } > + > + if (!bs->request_alignment) { > + size_t align; > + buf =3D qemu_memalign(s->buf_align, MAX_BLOCKSIZE); > + for (align =3D 512; align <=3D MAX_BLOCKSIZE; align <<=3D 1) { > + if (pread(s->fd, buf, align, 0) >=3D 0) { > + bs->request_alignment =3D align; > + break; > + } > + } > + qemu_vfree(buf); > + } > +} > + > static void raw_parse_flags(int bdrv_flags, int *open_flags) > { > assert(open_flags !=3D NULL); > @@ -463,7 +535,6 @@ static int raw_reopen_prepare(BDRVReopenState *stat= e, > return ret; > } > =20 > - > static void raw_reopen_commit(BDRVReopenState *state) > { > BDRVRawReopenState *raw_s =3D state->opaque; > @@ -499,23 +570,15 @@ static void raw_reopen_abort(BDRVReopenState *sta= te) > state->opaque =3D NULL; > } > =20 > +static int raw_refresh_limits(BlockDriverState *bs) > +{ > + BDRVRawState *s =3D bs->opaque; > =20 > -/* XXX: use host sector size if necessary with: > -#ifdef DIOCGSECTORSIZE > - { > - unsigned int sectorsize =3D 512; > - if (!ioctl(fd, DIOCGSECTORSIZE, §orsize) && > - sectorsize > bufsize) > - bufsize =3D sectorsize; > - } > -#endif > -#ifdef CONFIG_COCOA > - uint32_t blockSize =3D 512; > - if ( !ioctl( fd, DKIOCGETBLOCKSIZE, &blockSize ) && blockSize = > bufsize) { > - bufsize =3D blockSize; > - } > -#endif > -*/ > + raw_probe_alignment(bs); > + bs->bl.opt_mem_alignment =3D s->buf_align; > + > + return 0; > +} > =20 > static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb) > { > @@ -1363,6 +1426,7 @@ static BlockDriver bdrv_file =3D { > .bdrv_aio_writev =3D raw_aio_writev, > .bdrv_aio_flush =3D raw_aio_flush, > .bdrv_aio_discard =3D raw_aio_discard, > + .bdrv_refresh_limits =3D raw_refresh_limits, > =20 > .bdrv_truncate =3D raw_truncate, > .bdrv_getlength =3D raw_getlength, > @@ -1740,6 +1804,7 @@ static BlockDriver bdrv_host_device =3D { > .bdrv_aio_writev =3D raw_aio_writev, > .bdrv_aio_flush =3D raw_aio_flush, > .bdrv_aio_discard =3D hdev_aio_discard, > + .bdrv_refresh_limits =3D raw_refresh_limits, > =20 > .bdrv_truncate =3D raw_truncate, > .bdrv_getlength =3D raw_getlength, > @@ -1871,6 +1936,7 @@ static BlockDriver bdrv_host_floppy =3D { > .bdrv_aio_readv =3D raw_aio_readv, > .bdrv_aio_writev =3D raw_aio_writev, > .bdrv_aio_flush =3D raw_aio_flush, > + .bdrv_refresh_limits =3D raw_refresh_limits, > =20 > .bdrv_truncate =3D raw_truncate, > .bdrv_getlength =3D raw_getlength, > @@ -1981,6 +2047,7 @@ static BlockDriver bdrv_host_cdrom =3D { > .bdrv_aio_readv =3D raw_aio_readv, > .bdrv_aio_writev =3D raw_aio_writev, > .bdrv_aio_flush =3D raw_aio_flush, > + .bdrv_refresh_limits =3D raw_refresh_limits, > =20 > .bdrv_truncate =3D raw_truncate, > .bdrv_getlength =3D raw_getlength, > @@ -2110,6 +2177,7 @@ static BlockDriver bdrv_host_cdrom =3D { > .bdrv_aio_readv =3D raw_aio_readv, > .bdrv_aio_writev =3D raw_aio_writev, > .bdrv_aio_flush =3D raw_aio_flush, > + .bdrv_refresh_limits =3D raw_refresh_limits, > =20 > .bdrv_truncate =3D raw_truncate, > .bdrv_getlength =3D raw_getlength, > diff --git a/block/raw-win32.c b/block/raw-win32.c > index ce314fd..beb7f23 100644 > --- a/block/raw-win32.c > +++ b/block/raw-win32.c > @@ -202,6 +202,35 @@ static int set_sparse(int fd) > NULL, 0, NULL, 0, &returned, NULL); > } > =20 > +static void raw_probe_alignment(BlockDriverState *bs) > +{ > + BDRVRawState *s =3D bs->opaque; > + DWORD sectorsPerCluster, freeClusters, totalClusters, count; > + DISK_GEOMETRY_EX dg; > + BOOL status; > + > + if (s->type =3D=3D FTYPE_CD) { > + bs->request_alignment =3D 2048; > + return; > + } > + if (s->type =3D=3D FTYPE_HARDDISK) { > + status =3D DeviceIoControl(s->hfile, IOCTL_DISK_GET_DRIVE_GEOM= ETRY_EX, > + NULL, 0, &dg, sizeof(dg), &count, NUL= L); > + if (status !=3D 0) { > + bs->request_alignment =3D dg.Geometry.BytesPerSector; > + return; > + } > + /* try GetDiskFreeSpace too */ > + } > + > + if (s->drive_path[0]) { > + GetDiskFreeSpace(s->drive_path, §orsPerCluster, > + &dg.Geometry.BytesPerSector, > + &freeClusters, &totalClusters); > + bs->request_alignment =3D dg.Geometry.BytesPerSector; > + } > +} > + > static void raw_parse_flags(int flags, int *access_flags, DWORD *overl= apped) > { > assert(access_flags !=3D NULL); > @@ -269,6 +298,17 @@ static int raw_open(BlockDriverState *bs, QDict *o= ptions, int flags, > } > } > =20 > + if (filename[0] && filename[1] =3D=3D ':') { > + snprintf(s->drive_path, sizeof(s->drive_path), "%c:\\", filena= me[0]); > + } else if (filename[0] =3D=3D '\\' && filename[1] =3D=3D '\\') { > + s->drive_path[0] =3D 0; > + } else { > + /* Relative path. */ > + char buf[MAX_PATH]; > + GetCurrentDirectory(MAX_PATH, buf); > + snprintf(s->drive_path, sizeof(s->drive_path), "%c:\\", buf[0]= ); > + } > + This is not really alignment related. Does this really belong to this patch ? Best regards Beno=EEt > s->hfile =3D CreateFile(filename, access_flags, > FILE_SHARE_READ, NULL, > OPEN_EXISTING, overlapped, NULL); > @@ -293,6 +333,7 @@ static int raw_open(BlockDriverState *bs, QDict *op= tions, int flags, > s->aio =3D aio; > } > =20 > + raw_probe_alignment(bs); > ret =3D 0; > fail: > qemu_opts_del(opts); > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 36c068f..fb1d355 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -319,6 +319,9 @@ struct BlockDriverState { > /* Whether produces zeros when read beyond eof */ > bool zero_beyond_eof; > =20 > + /* Alignment requirement for offset/length of I/O requests */ > + unsigned int request_alignment; > + > /* the block size for which the guest device expects atomicity */ > int guest_block_size; > =20 > --=20 > 1.8.1.4 >=20 >=20