From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M8rtJ-0008PA-1h for qemu-devel@nongnu.org; Tue, 26 May 2009 04:20:09 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M8rtC-0008Ii-VT for qemu-devel@nongnu.org; Tue, 26 May 2009 04:20:08 -0400 Received: from [199.232.76.173] (port=42788 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M8rtC-0008IN-2e for qemu-devel@nongnu.org; Tue, 26 May 2009 04:20:02 -0400 Received: from mx20.gnu.org ([199.232.41.8]:1710) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M8rtB-0002gg-GO for qemu-devel@nongnu.org; Tue, 26 May 2009 04:20:01 -0400 Received: from verein.lst.de ([213.95.11.210]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1M8rt8-0001q4-Uz for qemu-devel@nongnu.org; Tue, 26 May 2009 04:19:59 -0400 Date: Tue, 26 May 2009 10:19:42 +0200 From: Christoph Hellwig Subject: Re: [Qemu-devel] Do we need CONFIG_AIO? Message-ID: <20090526081942.GA4388@lst.de> References: <20090525082151.GA4107@lst.de> <4A1A85B4.3060200@eu.citrix.com> <4A1BA111.9060801@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A1BA111.9060801@codemonkey.ws> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: "qemu-devel@nongnu.org" , Christoph Hellwig , Stefano Stabellini On Tue, May 26, 2009 at 02:58:09AM -0500, Anthony Liguori wrote: > Threads are not going to be able to remain an optional dependency > forever. I would be willing to merge something to remove CONFIG_AIO > although I expect that there will be some fall out. Some people are > using --disable-aio today for either historic reasons or because there > are weird bugs with AIO enabled. Btw, this is the example patch I had in mind. Also when finishing up the configure bits I noticed DragonflyBSD currently does disable aio (but not pthreads in general), but that's the only platform I found. Index: qemu/block/raw-posix.c =================================================================== --- qemu.orig/block/raw-posix.c 2009-05-26 10:10:43.787976336 +0200 +++ qemu/block/raw-posix.c 2009-05-26 10:12:03.328027500 +0200 @@ -26,9 +26,7 @@ #include "qemu-char.h" #include "block_int.h" #include "module.h" -#ifdef CONFIG_AIO #include "posix-aio-compat.h" -#endif #ifdef CONFIG_COCOA #include @@ -102,7 +100,7 @@ typedef struct BDRVRawState { int fd; int type; - unsigned int lseek_err_cnt; + int flags; int open_flags; #if defined(__linux__) /* linux floppy specific */ @@ -111,7 +109,6 @@ typedef struct BDRVRawState { int fd_got_error; int fd_media_changed; #endif - uint8_t* aligned_buf; } BDRVRawState; static int posix_aio_init(void); @@ -130,8 +127,6 @@ static int raw_open_common(BlockDriverSt posix_aio_init(); - s->lseek_err_cnt = 0; - s->open_flags |= O_BINARY; if ((flags & BDRV_O_ACCESS) == O_RDWR) { s->open_flags |= O_RDWR; @@ -156,15 +151,8 @@ static int raw_open_common(BlockDriverSt return ret; } s->fd = fd; - s->aligned_buf = NULL; - if ((flags & BDRV_O_NOCACHE)) { - s->aligned_buf = qemu_blockalign(bs, ALIGNED_BUFFER_SIZE); - if (s->aligned_buf == NULL) { - ret = -errno; - close(fd); - return ret; - } - } + s->flags = flags; + return 0; } @@ -196,282 +184,6 @@ static int raw_open(BlockDriverState *bs #endif */ -/* - * offset and count are in bytes, but must be multiples of 512 for files - * opened with O_DIRECT. buf must be aligned to 512 bytes then. - * - * This function may be called without alignment if the caller ensures - * that O_DIRECT is not in effect. - */ -static int raw_pread_aligned(BlockDriverState *bs, int64_t offset, - uint8_t *buf, int count) -{ - BDRVRawState *s = bs->opaque; - int ret; - - ret = fd_open(bs); - if (ret < 0) - return ret; - - if (offset >= 0 && lseek(s->fd, offset, SEEK_SET) == (off_t)-1) { - ++(s->lseek_err_cnt); - if(s->lseek_err_cnt <= 10) { - DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64 - "] lseek failed : %d = %s\n", - s->fd, bs->filename, offset, buf, count, - bs->total_sectors, errno, strerror(errno)); - } - return -1; - } - s->lseek_err_cnt=0; - - ret = read(s->fd, buf, count); - if (ret == count) - goto label__raw_read__success; - - DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64 - "] read failed %d : %d = %s\n", - s->fd, bs->filename, offset, buf, count, - bs->total_sectors, ret, errno, strerror(errno)); - - /* Try harder for CDrom. */ - if (bs->type == BDRV_TYPE_CDROM) { - lseek(s->fd, offset, SEEK_SET); - ret = read(s->fd, buf, count); - if (ret == count) - goto label__raw_read__success; - lseek(s->fd, offset, SEEK_SET); - ret = read(s->fd, buf, count); - if (ret == count) - goto label__raw_read__success; - - DEBUG_BLOCK_PRINT("raw_pread(%d:%s, %" PRId64 ", %p, %d) [%" PRId64 - "] retry read failed %d : %d = %s\n", - s->fd, bs->filename, offset, buf, count, - bs->total_sectors, ret, errno, strerror(errno)); - } - -label__raw_read__success: - - return (ret < 0) ? -errno : ret; -} - -/* - * offset and count are in bytes, but must be multiples of 512 for files - * opened with O_DIRECT. buf must be aligned to 512 bytes then. - * - * This function may be called without alignment if the caller ensures - * that O_DIRECT is not in effect. - */ -static int raw_pwrite_aligned(BlockDriverState *bs, int64_t offset, - const uint8_t *buf, int count) -{ - BDRVRawState *s = bs->opaque; - int ret; - - ret = fd_open(bs); - if (ret < 0) - return -errno; - - if (offset >= 0 && lseek(s->fd, offset, SEEK_SET) == (off_t)-1) { - ++(s->lseek_err_cnt); - if(s->lseek_err_cnt) { - DEBUG_BLOCK_PRINT("raw_pwrite(%d:%s, %" PRId64 ", %p, %d) [%" - PRId64 "] lseek failed : %d = %s\n", - s->fd, bs->filename, offset, buf, count, - bs->total_sectors, errno, strerror(errno)); - } - return -EIO; - } - s->lseek_err_cnt = 0; - - ret = write(s->fd, buf, count); - if (ret == count) - goto label__raw_write__success; - - DEBUG_BLOCK_PRINT("raw_pwrite(%d:%s, %" PRId64 ", %p, %d) [%" PRId64 - "] write failed %d : %d = %s\n", - s->fd, bs->filename, offset, buf, count, - bs->total_sectors, ret, errno, strerror(errno)); - -label__raw_write__success: - - return (ret < 0) ? -errno : ret; -} - - -/* - * offset and count are in bytes and possibly not aligned. For files opened - * with O_DIRECT, necessary alignments are ensured before calling - * raw_pread_aligned to do the actual read. - */ -static int raw_pread(BlockDriverState *bs, int64_t offset, - uint8_t *buf, int count) -{ - BDRVRawState *s = bs->opaque; - int size, ret, shift, sum; - - sum = 0; - - if (s->aligned_buf != NULL) { - - if (offset & 0x1ff) { - /* align offset on a 512 bytes boundary */ - - shift = offset & 0x1ff; - size = (shift + count + 0x1ff) & ~0x1ff; - if (size > ALIGNED_BUFFER_SIZE) - size = ALIGNED_BUFFER_SIZE; - ret = raw_pread_aligned(bs, offset - shift, s->aligned_buf, size); - if (ret < 0) - return ret; - - size = 512 - shift; - if (size > count) - size = count; - memcpy(buf, s->aligned_buf + shift, size); - - buf += size; - offset += size; - count -= size; - sum += size; - - if (count == 0) - return sum; - } - if (count & 0x1ff || (uintptr_t) buf & 0x1ff) { - - /* read on aligned buffer */ - - while (count) { - - size = (count + 0x1ff) & ~0x1ff; - if (size > ALIGNED_BUFFER_SIZE) - size = ALIGNED_BUFFER_SIZE; - - ret = raw_pread_aligned(bs, offset, s->aligned_buf, size); - if (ret < 0) - return ret; - - size = ret; - if (size > count) - size = count; - - memcpy(buf, s->aligned_buf, size); - - buf += size; - offset += size; - count -= size; - sum += size; - } - - return sum; - } - } - - return raw_pread_aligned(bs, offset, buf, count) + sum; -} - -static int raw_read(BlockDriverState *bs, int64_t sector_num, - uint8_t *buf, int nb_sectors) -{ - int ret; - - ret = raw_pread(bs, sector_num * 512, buf, nb_sectors * 512); - if (ret == (nb_sectors * 512)) - ret = 0; - return ret; -} - -/* - * offset and count are in bytes and possibly not aligned. For files opened - * with O_DIRECT, necessary alignments are ensured before calling - * raw_pwrite_aligned to do the actual write. - */ -static int raw_pwrite(BlockDriverState *bs, int64_t offset, - const uint8_t *buf, int count) -{ - BDRVRawState *s = bs->opaque; - int size, ret, shift, sum; - - sum = 0; - - if (s->aligned_buf != NULL) { - - if (offset & 0x1ff) { - /* align offset on a 512 bytes boundary */ - shift = offset & 0x1ff; - ret = raw_pread_aligned(bs, offset - shift, s->aligned_buf, 512); - if (ret < 0) - return ret; - - size = 512 - shift; - if (size > count) - size = count; - memcpy(s->aligned_buf + shift, buf, size); - - ret = raw_pwrite_aligned(bs, offset - shift, s->aligned_buf, 512); - if (ret < 0) - return ret; - - buf += size; - offset += size; - count -= size; - sum += size; - - if (count == 0) - return sum; - } - if (count & 0x1ff || (uintptr_t) buf & 0x1ff) { - - while ((size = (count & ~0x1ff)) != 0) { - - if (size > ALIGNED_BUFFER_SIZE) - size = ALIGNED_BUFFER_SIZE; - - memcpy(s->aligned_buf, buf, size); - - ret = raw_pwrite_aligned(bs, offset, s->aligned_buf, size); - if (ret < 0) - return ret; - - buf += ret; - offset += ret; - count -= ret; - sum += ret; - } - /* here, count < 512 because (count & ~0x1ff) == 0 */ - if (count) { - ret = raw_pread_aligned(bs, offset, s->aligned_buf, 512); - if (ret < 0) - return ret; - memcpy(s->aligned_buf, buf, count); - - ret = raw_pwrite_aligned(bs, offset, s->aligned_buf, 512); - if (ret < 0) - return ret; - if (count < ret) - ret = count; - - sum += ret; - } - return sum; - } - } - return raw_pwrite_aligned(bs, offset, buf, count) + sum; -} - -static int raw_write(BlockDriverState *bs, int64_t sector_num, - const uint8_t *buf, int nb_sectors) -{ - int ret; - ret = raw_pwrite(bs, sector_num * 512, buf, nb_sectors * 512); - if (ret == (nb_sectors * 512)) - ret = 0; - return ret; -} - -#ifdef CONFIG_AIO /***********************************************************/ /* Unix AIO using POSIX AIO */ @@ -629,7 +341,7 @@ static RawAIOCB *raw_aio_setup(BlockDriv * boundary. Tell the low level code to ensure that in case it's * not done yet. */ - if (s->aligned_buf) + if (s->flags & BDRV_O_NOCACHE) acb->aiocb.aio_flags |= QEMU_AIO_SECTOR_ALIGNED; acb->next = posix_aio_state->first_aio; @@ -702,13 +414,6 @@ static void raw_aio_cancel(BlockDriverAI raw_aio_remove(acb); } -#else /* CONFIG_AIO */ -static int posix_aio_init(void) -{ - return 0; -} -#endif /* CONFIG_AIO */ - static void raw_close(BlockDriverState *bs) { @@ -716,8 +421,6 @@ static void raw_close(BlockDriverState * if (s->fd >= 0) { close(s->fd); s->fd = -1; - if (s->aligned_buf != NULL) - qemu_free(s->aligned_buf); } } @@ -866,18 +569,14 @@ static BlockDriver bdrv_raw = { .instance_size = sizeof(BDRVRawState), .bdrv_probe = NULL, /* no probe for protocols */ .bdrv_open = raw_open, - .bdrv_read = raw_read, - .bdrv_write = raw_write, .bdrv_close = raw_close, .bdrv_create = raw_create, .bdrv_flush = raw_flush, -#ifdef CONFIG_AIO .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, .bdrv_aio_cancel = raw_aio_cancel, .aiocb_size = sizeof(RawAIOCB), -#endif .bdrv_truncate = raw_truncate, .bdrv_getlength = raw_getlength, @@ -978,7 +677,7 @@ static int hdev_open(BlockDriverState *b #endif s->type = FTYPE_FILE; -#if defined(__linux__) && defined(CONFIG_AIO) +#if defined(__linux__) if (strstart(filename, "/dev/sg", NULL)) { bs->sg = 1; } @@ -1044,7 +743,6 @@ static int hdev_ioctl(BlockDriverState * return ioctl(s->fd, req, buf); } -#ifdef CONFIG_AIO static BlockDriverAIOCB *hdev_aio_ioctl(BlockDriverState *bs, unsigned long int req, void *buf, BlockDriverCompletionFunc *cb, void *opaque) @@ -1075,7 +773,6 @@ static BlockDriverAIOCB *hdev_aio_ioctl( return &acb->common; } -#endif #elif defined(__FreeBSD__) static int fd_open(BlockDriverState *bs) @@ -1134,24 +831,18 @@ static BlockDriver bdrv_host_device = { .bdrv_create = hdev_create, .bdrv_flush = raw_flush, -#ifdef CONFIG_AIO .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, .bdrv_aio_cancel = raw_aio_cancel, .aiocb_size = sizeof(RawAIOCB), -#endif - .bdrv_read = raw_read, - .bdrv_write = raw_write, .bdrv_getlength = raw_getlength, /* generic scsi device */ #ifdef __linux__ .bdrv_ioctl = hdev_ioctl, -#ifdef CONFIG_AIO .bdrv_aio_ioctl = hdev_aio_ioctl, #endif -#endif }; #ifdef __linux__ @@ -1228,15 +919,11 @@ static BlockDriver bdrv_host_floppy = { .bdrv_create = hdev_create, .bdrv_flush = raw_flush, -#ifdef CONFIG_AIO .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, .bdrv_aio_cancel = raw_aio_cancel, .aiocb_size = sizeof(RawAIOCB), -#endif - .bdrv_read = raw_read, - .bdrv_write = raw_write, .bdrv_getlength = raw_getlength, /* removable device support */ @@ -1305,15 +992,11 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_create = hdev_create, .bdrv_flush = raw_flush, -#ifdef CONFIG_AIO .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, .bdrv_aio_cancel = raw_aio_cancel, .aiocb_size = sizeof(RawAIOCB), -#endif - .bdrv_read = raw_read, - .bdrv_write = raw_write, .bdrv_getlength = raw_getlength, /* removable device support */ @@ -1323,9 +1006,7 @@ static BlockDriver bdrv_host_cdrom = { /* generic scsi device */ .bdrv_ioctl = hdev_ioctl, -#ifdef CONFIG_AIO .bdrv_aio_ioctl = hdev_aio_ioctl, -#endif }; #endif /* __linux__ */ @@ -1421,15 +1102,11 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_create = hdev_create, .bdrv_flush = raw_flush, -#ifdef CONFIG_AIO .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, .bdrv_aio_cancel = raw_aio_cancel, .aiocb_size = sizeof(RawAIOCB), -#endif - .bdrv_read = raw_read, - .bdrv_write = raw_write, .bdrv_getlength = raw_getlength, /* removable device support */ Index: qemu/Makefile =================================================================== --- qemu.orig/Makefile 2009-05-26 10:12:10.201963074 +0200 +++ qemu/Makefile 2009-05-26 10:12:25.209855741 +0200 @@ -74,10 +74,7 @@ BLOCK_OBJS+=nbd.o block.o aio.o ifdef CONFIG_WIN32 BLOCK_OBJS += block/raw-win32.o else -ifdef CONFIG_AIO -BLOCK_OBJS += posix-aio-compat.o -endif -BLOCK_OBJS += block/raw-posix.o +BLOCK_OBJS += block/raw-posix.o posix-aio-compat.o endif ifdef CONFIG_CURL Index: qemu/configure =================================================================== --- qemu.orig/configure 2009-05-26 10:13:16.706814350 +0200 +++ qemu/configure 2009-05-26 10:16:16.304814641 +0200 @@ -181,7 +181,6 @@ uname_release="" curses="yes" curl="yes" pthread="yes" -aio="yes" io_thread="no" nptl="yes" mixemu="no" @@ -246,7 +245,6 @@ audio_possible_drivers="oss sdl esd pa" if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then kqemu="yes" fi -aio="no" ;; NetBSD) bsd="yes" @@ -481,10 +479,6 @@ for opt do ;; --enable-mixemu) mixemu="yes" ;; - --disable-pthread) pthread="no" - ;; - --disable-aio) aio="no" - ;; --enable-io-thread) io_thread="yes" ;; --disable-blobs) blobs="no" @@ -620,8 +614,6 @@ echo " --oss-lib path to echo " --enable-uname-release=R Return R for uname -r in usermode emulation" echo " --sparc_cpu=V Build qemu for Sparc architecture v7, v8, v8plus, v8plusa, v9" echo " --disable-vde disable support for vde network" -echo " --disable-pthread disable pthread support" -echo " --disable-aio disable AIO support" echo " --enable-io-thread enable IO thread" echo " --disable-blobs disable installing provided firmware blobs" echo " --kerneldir=PATH look for kernel includes in PATH" @@ -1152,21 +1144,22 @@ fi # pthread probe PTHREADLIBS="" -if test "$pthread" = yes; then - pthread=no +pthread=no cat > $TMPC << EOF #include int main(void) { pthread_mutex_t lock; return 0; } EOF - if $cc $ARCH_CFLAGS -o $TMPE $PTHREADLIBS $TMPC 2> /dev/null > /dev/null ; then - pthread=yes - PTHREADLIBS="-lpthread" - fi +if $cc $ARCH_CFLAGS -o $TMPE $PTHREADLIBS $TMPC 2> /dev/null > /dev/null ; then + pthread=yes + PTHREADLIBS="-lpthread" fi if test "$pthread" = no; then - aio=no - io_thread=no + echo + echo "Error: pthread check failed" + echo "Make sure to have the pthread libs and headers installed." + echo + exit 1 fi ########################################## @@ -1354,7 +1347,6 @@ echo "Documentation $build_docs" echo "uname -r $uname_release" echo "NPTL support $nptl" echo "vde support $vde" -echo "AIO support $aio" echo "IO thread $io_thread" echo "Install blobs $blobs" echo "KVM support $kvm" @@ -1666,10 +1658,6 @@ fi if test "$xen" = "yes" ; then echo "XEN_LIBS=-lxenstore -lxenctrl -lxenguest" >> $config_mak fi -if test "$aio" = "yes" ; then - echo "#define CONFIG_AIO 1" >> $config_h - echo "CONFIG_AIO=yes" >> $config_mak -fi if test "$io_thread" = "yes" ; then echo "CONFIG_IOTHREAD=yes" >> $config_mak echo "#define CONFIG_IOTHREAD 1" >> $config_h