From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38729) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGozn-0000E1-0H for qemu-devel@nongnu.org; Thu, 29 Jan 2015 08:18:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGozj-0002eH-Qx for qemu-devel@nongnu.org; Thu, 29 Jan 2015 08:18:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60497) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGozj-0002do-IQ for qemu-devel@nongnu.org; Thu, 29 Jan 2015 08:18:51 -0500 Date: Thu, 29 Jan 2015 14:18:48 +0100 From: Kevin Wolf Message-ID: <20150129131848.GA3950@noname.redhat.com> References: <1422528659-3121-1-git-send-email-den@openvz.org> <1422528659-3121-2-git-send-email-den@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1422528659-3121-2-git-send-email-den@openvz.org> Subject: Re: [Qemu-devel] [PATCH 1/1] block: enforce minimal 4096 alignment in qemu_blockalign List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Paolo Bonzini , qemu-devel@nongnu.org, Stefan Hajnoczi Am 29.01.2015 um 11:50 hat Denis V. Lunev geschrieben: > The following sequence > int fd = open(argv[1], O_RDWR | O_CREAT | O_DIRECT, 0644); > for (i = 0; i < 100000; i++) > write(fd, buf, 4096); > performs 5% better if buf is aligned to 4096 bytes rather then to > 512 bytes on HDD with 512/4096 logical/physical sector size. > > The difference is quite reliable. > > On the other hand we do not want at the moment to enforce bounce > buffering if guest request is aligned to 512 bytes. This patch > forces page alignment when we really forced to perform memory > allocation. > > Signed-off-by: Denis V. Lunev > CC: Paolo Bonzini > CC: Kevin Wolf > CC: Stefan Hajnoczi > --- > block.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index d45e4dd..38cf73f 100644 > --- a/block.c > +++ b/block.c > @@ -5293,7 +5293,11 @@ void bdrv_set_guest_block_size(BlockDriverState *bs, int align) > > void *qemu_blockalign(BlockDriverState *bs, size_t size) > { > - return qemu_memalign(bdrv_opt_mem_align(bs), size); > + size_t align = bdrv_opt_mem_align(bs); > + if (align < 4096) { > + align = 4096; > + } > + return qemu_memalign(align, size); > } > > void *qemu_blockalign0(BlockDriverState *bs, size_t size) > @@ -5307,6 +5311,9 @@ void *qemu_try_blockalign(BlockDriverState *bs, size_t size) > > /* Ensure that NULL is never returned on success */ > assert(align > 0); > + if (align < 4096) { > + align = 4096; > + } > if (size == 0) { > size = align; > } This is the wrong place to make this change. First you're duplicating logic in the callers of bdrv_opt_mem_align() instead of making it return the right thing in the first place. Second, you're arguing with numbers from a simple test case for O_DIRECT on Linux, but you're changing the alignment for everyone instead of just the raw-posix driver which is responsible for accessing Linux files. Also, what's the real reason for the performance improvement? Having page alignment? If so, actually querying the page size instead of assuming 4k might be worth a thought. Kevin