From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46729) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNiC8-0002Q4-Gj for qemu-devel@nongnu.org; Tue, 08 Nov 2011 04:42:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RNiC6-00073n-JN for qemu-devel@nongnu.org; Tue, 08 Nov 2011 04:42:16 -0500 Received: from mtagate3.uk.ibm.com ([194.196.100.163]:34029) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNiC6-00073J-9O for qemu-devel@nongnu.org; Tue, 08 Nov 2011 04:42:14 -0500 Received: from d06nrmr1707.portsmouth.uk.ibm.com (d06nrmr1707.portsmouth.uk.ibm.com [9.149.39.225]) by mtagate3.uk.ibm.com (8.13.1/8.13.1) with ESMTP id pA89gBub026224 for ; Tue, 8 Nov 2011 09:42:11 GMT Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by d06nrmr1707.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pA89gB9V2191588 for ; Tue, 8 Nov 2011 09:42:11 GMT Received: from d06av08.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pA89gATs001382 for ; Tue, 8 Nov 2011 09:42:11 GMT Date: Tue, 8 Nov 2011 08:16:03 +0000 From: Stefan Hajnoczi Message-ID: <20111108081603.GA7209@stefanha-thinkpad.localdomain> References: <1318866452-30026-1-git-send-email-stefanha@linux.vnet.ibm.com> <1318866452-30026-5-git-send-email-stefanha@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC 4/6] block: request overlap detection List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhi Yong Wu Cc: Kevin Wolf , Stefan Hajnoczi , Marcelo Tosatti , qemu-devel@nongnu.org, Paolo Bonzini On Tue, Nov 08, 2011 at 02:34:22PM +0800, Zhi Yong Wu wrote: > On Mon, Nov 7, 2011 at 10:37 PM, Stefan Hajnoczi w= rote: > > On Mon, Nov 7, 2011 at 11:49 AM, Zhi Yong Wu w= rote: > >> On Mon, Oct 17, 2011 at 11:47 PM, Stefan Hajnoczi > >> wrote: > >>> Detect overlapping requests and remember to align to cluster bounda= ries > >>> if the image format uses them. =A0This assumes that allocating I/O = is > >>> performed in cluster granularity - which is true for qcow2, qed, et= c. > >>> > >>> Signed-off-by: Stefan Hajnoczi > >>> --- > >>> =A0block.c | =A0 39 +++++++++++++++++++++++++++++++++++++-- > >>> =A01 files changed, 37 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/block.c b/block.c > >>> index cc3202c..0c22741 100644 > >>> --- a/block.c > >>> +++ b/block.c > >>> @@ -1052,21 +1052,56 @@ static BdrvTrackedRequest *tracked_request_= add(BlockDriverState *bs, > >>> =A0 =A0 return req; > >>> =A0} > >>> > >>> +/** > >>> + * Round a region to cluster boundaries > >>> + */ > >>> +static void round_to_clusters(BlockDriverState *bs, > >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int64_= t sector_num, int nb_sectors, > >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int64_= t *cluster_sector_num, > >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int *c= luster_nb_sectors) > >>> +{ > >>> + =A0 =A0BlockDriverInfo bdi; > >>> + > >>> + =A0 =A0if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size =3D=3D= 0) { > >>> + =A0 =A0 =A0 =A0*cluster_sector_num =3D sector_num; > >>> + =A0 =A0 =A0 =A0*cluster_nb_sectors =3D nb_sectors; > >>> + =A0 =A0} else { > >>> + =A0 =A0 =A0 =A0int64_t c =3D bdi.cluster_size / BDRV_SECTOR_SIZE; > >>> + =A0 =A0 =A0 =A0*cluster_sector_num =3D (sector_num / c) * c; > >> =A0 =A0 =A0 =A0 =A0 =A0 I can understand the above formula, but the = one below is > >> very magic. :) and can not be understood by me. > >>> + =A0 =A0 =A0 =A0*cluster_nb_sectors =3D ((sector_num % c) + nb_sec= tors + c - 1) / c * c; > > > > I agree this is ugly. =A0Here is what is going on: > > > > c =3D number of sectors per cluster > > cluster_sector_num =3D sector number rounded *down* to cluster bounda= ry > > cluster_nb_sectors =3D number of sectors from cluster_sector_num to > > rounded up sector_num+nb_sectors > > > > So the magic expression is takes the original sector_num to > > sector_num+nb_sectors region: > > > > |---XXX|XXX---| > > > > Where |-----| is a cluster and XXXX is the region from sector_num to > > sector_num+nb_sectors, then the output should be: > > > > |RRRRRR|RRRRRR| > > > > Everything has been rounded to clusters. =A0So here is the expression= broken down: > > > > *cluster_nb_sectors =3D ((sector_num % c) + nb_sectors + c - 1) / c *= c; > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0AAAAAAAAAAAAAA =A0 =A0= XXXXXXXXXX =A0 BBBBBBBBBBBBBB > > > > |AAAXXX|XXXBBB| > > > > A is actually equivalent to sector_num - cluster_sector_num. > > > > X is the original unrounded region. > > > > B is the rounding up to the next cluster bounary. > > > > Another way of writing this: > > > > *cluster_nb_sectors =3D ROUND_UP((sector_num - cluster_sector_num) + > > nb_sectors, c); > Above expression seems to not be correct; > It should be > *cluster_nb_sectors =3D ROUND_UP((sector_num - cluster_sector_num) + > nb_sectors, c) * c; >=20 > *cluster_nb_sectors =3D ((sector_num % c) + nb_sectors + c - 1) / c * c= ; >=20 > #define ROUND_UP(x,y) (((x)+(y)-1)/(y)) ALIGN_UP() may be a better macro name, for example: ALIGN_UP(1024, 4096) =3D 4096 I see how you're defining ROUND_UP() and it is different. Stefan