From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KSYwo-0004AN-BJ for qemu-devel@nongnu.org; Mon, 11 Aug 2008 11:04:38 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KSYwm-00048n-OS for qemu-devel@nongnu.org; Mon, 11 Aug 2008 11:04:37 -0400 Received: from [199.232.76.173] (port=48607 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KSYwm-00048i-MW for qemu-devel@nongnu.org; Mon, 11 Aug 2008 11:04:36 -0400 Received: from mx2.suse.de ([195.135.220.15]:36951) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KSYwm-00010A-0x for qemu-devel@nongnu.org; Mon, 11 Aug 2008 11:04:36 -0400 Message-ID: <48A054C6.1010104@suse.de> Date: Mon, 11 Aug 2008 17:03:34 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [patch 5/5][v2] Try to aggregate free clusters and freed clusters References: <20080729141352.573798859@bull.net> <20080729141447.763322831@bull.net> <48A03AE7.7050908@suse.de> <1218463441.3871.29.camel@frecb07144> In-Reply-To: <1218463441.3871.29.camel@frecb07144> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: qemu-devel@nongnu.org Laurent Vivier schrieb: > Le lundi 11 ao=C3=BBt 2008 =C3=A0 15:13 +0200, Kevin Wolf a =C3=A9crit = : >> Hi Laurent, >> >> Laurent Vivier schrieb: >>> In free_used_clusters(), try to aggregate free clusters and freed clu= sters. >>> >>> Signed-off-by: Laurent Vivier >>> --- >>> block-qcow2.c | 61 ++++++++++++++++++++++++++++++++++-------------= ----------- >>> 1 file changed, 36 insertions(+), 25 deletions(-) >> I know that this patch will be rewritten as well, but I'll comment on = it >> nevertheless. Maybe the comments apply to your new version, too. >> >> >>> Index: qemu/block-qcow2.c >>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>> --- qemu.orig/block-qcow2.c 2008-07-29 15:22:28.000000000 +0200 >>> +++ qemu/block-qcow2.c 2008-07-29 15:22:30.000000000 +0200 >>> @@ -714,22 +703,44 @@ static uint64_t free_used_clusters(Block >>> return 0; >>> } >>> =20 >>> - if (nb_clusters) { >>> - int i =3D 1; >>> - uint64_t current; >>> - while (i < *nb_clusters) { >>> - current =3D be64_to_cpu((*l2_table)[(*l2_index) + i]); >>> - if (cluster_offset + (i << s->cluster_bits) !=3D current= ) >>> - break; >>> - i++; >>> - } >>> - *nb_clusters =3D i; >>> - free_clusters(bs, cluster_offset, i << s->cluster_bits); >>> + if (!nb_clusters) { >>> + if (cluster_offset) >>> + free_clusters(bs, cluster_offset, s->cluster_size); >>> return 0; >>> } >>> =20 >>> - free_clusters(bs, cluster_offset, s->cluster_size); >>> + i =3D 0; >>> + do_loop =3D 1; >>> + while (i < *nb_clusters && do_loop) { >>> + i++; >>> + if (!cluster_offset) { >>> + while (i < *nb_clusters) { >>> + cluster_offset =3D (*l2_table)[(*l2_index) + i]; >>> + if (cluster_offset) >>> + break; >>> + i++; >>> + } >>> + if ((cluster_offset & QCOW_OFLAG_COPIED) || >>> + (cluster_offset & QCOW_OFLAG_COMPRESSED)) >>> + do_loop =3D 0; >>> + } else { >>> + j =3D 1; >>> + current =3D 0; >>> + while (i < *nb_clusters) { >>> + current =3D be64_to_cpu((*l2_table)[(*l2_index) + i]= ); >>> + if (cluster_offset + (j << s->cluster_bits) !=3D cur= rent) >>> + break; >>> + i++; >>> + j++; >>> + } >>> + free_clusters(bs, cluster_offset, j << s->cluster_bits); >>> + cluster_offset =3D current; >>> + if (current) >>> + do_loop =3D 0; >> The else block changes its behaviour with this patch: I think you shou= ld >> take QCOW_OFLAG_COPIED into consideration. In this implementation, wh= en >> a cluster with QCOW_OFLAG_COPIED is found, the inner loop aborts, but >> the outer one will just call the else block once again. So you're also >> accumulating clusters with QCOW_OFLAG_COPIED set. >=20 > No, > before the first loop QCOW_OFLAG_COPIED is not set. > if cluster_offset is 0 and flag appears, we set do_loop to 0 and exit > from the main loop. > if cluster_offset is !=3D 0 we go into the else until current becomes 0= or > a flag is set and then if current is not 0 (i.e. a flag or a non > contiguous cluster) we set do_loop to 0 and exit from the loop. You're right. I really should learn reading. Sorry for the noise. Kevin