From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47138) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vkzcx-0005cQ-4m for qemu-devel@nongnu.org; Mon, 25 Nov 2013 12:07:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vkzcs-00061F-7I for qemu-devel@nongnu.org; Mon, 25 Nov 2013 12:07:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:4776) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vkzcr-000616-VF for qemu-devel@nongnu.org; Mon, 25 Nov 2013 12:07:10 -0500 Message-ID: <52937BF9.40309@redhat.com> Date: Mon, 25 Nov 2013 17:34:01 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1385387840-17307-1-git-send-email-pl@kamp.de> <1385387840-17307-6-git-send-email-pl@kamp.de> <529368A8.8050807@redhat.com> <529376C8.6080708@kamp.de> In-Reply-To: <529376C8.6080708@kamp.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1.8 5/6] qemu-img: add option to align writes to cluster_sectors during convert List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com Il 25/11/2013 17:11, Peter Lieven ha scritto: > On 25.11.2013 16:11, Paolo Bonzini wrote: >> Il 25/11/2013 14:57, Peter Lieven ha scritto: >>> Signed-off-by: Peter Lieven >> Ok, given this patch I think the cluster_size is the right one to use >> here---and also the way you used the optimal unmap granularity makes >> sense; you could also use MAX(optimal unmap granularity, optimal >> transfer length granularity). >> >> However, there is no need to write one cluster at a time. What matters, >> I think, is to align the *end* of the transfer, so that the next >> transfer can start aligned. >> >>> + if (align && cluster_sectors > 0) { >>> + int64_t next_aligned_sector = (sector_num + >>> cluster_sectors); >> So this should be "+ n", not "+ cluster_sectors". >> >> Perhaps it could be conditional on "n > cluster_sectors" (small requests >> happen when you have sparse region, and breaking them doesn't help). > > would you also agree to n >= cluster_sectors. In my case > and if especially if n is bound by iobuf_size the case n > cluster_sectors > will be hard to meet. Of course. In fact > alone is wrong ("n > cluster_sectors || n == iobuf_size" could be right, but perhaps it's a useless complication). Paolo