QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Qemu-devel] [PATCH] qcow2: Stop overwriting compressed clusters one by one
@ 2019-09-11 15:16 Alberto Garcia
  2019-09-11 23:33 ` [Qemu-devel] [Qemu-block] " John Snow
  0 siblings, 1 reply; 4+ messages in thread
From: Alberto Garcia @ 2019-09-11 15:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Alberto Garcia, qemu-block, Max Reitz

handle_alloc() tries to find as many contiguous clusters that need
copy-on-write as possible in order to allocate all of them at the same
time.

However, compressed clusters are only overwritten one by one, so let's
say that we have an image with 1024 consecutive compressed clusters:

   qemu-img create -f qcow2 hd.qcow2 64M
   for f in `seq 0 64 65472`; do
      qemu-io -c "write -c ${f}k 64k" hd.qcow2
   done

In this case trying to overwrite the whole image with one large write
request results in 1024 separate allocations:

   qemu-io -c "write 0 64M" hd.qcow2

This restriction comes from commit 095a9c58ce12afeeb90c2 from 2018.
Nowadays QEMU can overwrite multiple compressed clusters just fine,
and in fact it already does: as long as the first cluster that
handle_alloc() finds is not compressed, all other compressed clusters
in the same batch will be overwritten in one go:

   qemu-img create -f qcow2 hd.qcow2 64M
   qemu-io -c "write -z 0 64k" hd.qcow2
   for f in `seq 64 64 65472`; do
      qemu-io -c "write -c ${f}k 64k" hd.qcow2
   done

Compared to the previous one, overwriting this image on my computer
goes from 8.35s down to 230ms.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2-cluster.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f09cc992af..dcacd3c450 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1351,13 +1351,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset,
     }
 
     entry = be64_to_cpu(l2_slice[l2_index]);
-
-    /* For the moment, overwrite compressed clusters one by one */
-    if (entry & QCOW_OFLAG_COMPRESSED) {
-        nb_clusters = 1;
-    } else {
-        nb_clusters = count_cow_clusters(bs, nb_clusters, l2_slice, l2_index);
-    }
+    nb_clusters = count_cow_clusters(bs, nb_clusters, l2_slice, l2_index);
 
     /* This function is only called when there were no non-COW clusters, so if
      * we can't find any unallocated or COW clusters either, something is
-- 
2.20.1



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2: Stop overwriting compressed clusters one by one
  2019-09-11 15:16 [Qemu-devel] [PATCH] qcow2: Stop overwriting compressed clusters one by one Alberto Garcia
@ 2019-09-11 23:33 ` " John Snow
  2019-09-12  7:25   ` Alberto Garcia
  0 siblings, 1 reply; 4+ messages in thread
From: John Snow @ 2019-09-11 23:33 UTC (permalink / raw)
  To: Alberto Garcia, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz



On 9/11/19 11:16 AM, Alberto Garcia wrote:
> handle_alloc() tries to find as many contiguous clusters that need
> copy-on-write as possible in order to allocate all of them at the same
> time.
> 
> However, compressed clusters are only overwritten one by one, so let's
> say that we have an image with 1024 consecutive compressed clusters:
> 
>    qemu-img create -f qcow2 hd.qcow2 64M
>    for f in `seq 0 64 65472`; do
>       qemu-io -c "write -c ${f}k 64k" hd.qcow2
>    done
> 
> In this case trying to overwrite the whole image with one large write
> request results in 1024 separate allocations:
> 
>    qemu-io -c "write 0 64M" hd.qcow2
> 
> This restriction comes from commit 095a9c58ce12afeeb90c2 from 2018.

You accidentally typed a reasonably modern date. It's from *2008*!

> Nowadays QEMU can overwrite multiple compressed clusters just fine,
> and in fact it already does: as long as the first cluster that
> handle_alloc() finds is not compressed, all other compressed clusters
> in the same batch will be overwritten in one go:
> 
>    qemu-img create -f qcow2 hd.qcow2 64M
>    qemu-io -c "write -z 0 64k" hd.qcow2
>    for f in `seq 64 64 65472`; do
>       qemu-io -c "write -c ${f}k 64k" hd.qcow2
>    done
> 
> Compared to the previous one, overwriting this image on my computer
> goes from 8.35s down to 230ms.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2-cluster.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index f09cc992af..dcacd3c450 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1351,13 +1351,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset,
>      }
>  
>      entry = be64_to_cpu(l2_slice[l2_index]);
> -
> -    /* For the moment, overwrite compressed clusters one by one */
> -    if (entry & QCOW_OFLAG_COMPRESSED) {
> -        nb_clusters = 1;
> -    } else {
> -        nb_clusters = count_cow_clusters(bs, nb_clusters, l2_slice, l2_index);
> -    }
> +    nb_clusters = count_cow_clusters(bs, nb_clusters, l2_slice, l2_index);
>  
>      /* This function is only called when there were no non-COW clusters, so if
>       * we can't find any unallocated or COW clusters either, something is
> 

Well, given that count_cow_clusters already works this way, this doesn't
break anything that wasn't broken before, at least.

Reviewed-by: John Snow <jsnow@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2: Stop overwriting compressed clusters one by one
  2019-09-11 23:33 ` [Qemu-devel] [Qemu-block] " John Snow
@ 2019-09-12  7:25   ` Alberto Garcia
  2019-09-12  9:28     ` Kevin Wolf
  0 siblings, 1 reply; 4+ messages in thread
From: Alberto Garcia @ 2019-09-12  7:25 UTC (permalink / raw)
  To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz

On Thu 12 Sep 2019 01:33:05 AM CEST, John Snow <jsnow@redhat.com> wrote:
>> This restriction comes from commit 095a9c58ce12afeeb90c2 from 2018.
>
> You accidentally typed a reasonably modern date. It's from *2008*!

Oh my, and I reviewed the message 3 times ... if this one gets committed
please correct the date.

Berto


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2: Stop overwriting compressed clusters one by one
  2019-09-12  7:25   ` Alberto Garcia
@ 2019-09-12  9:28     ` Kevin Wolf
  0 siblings, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2019-09-12  9:28 UTC (permalink / raw)
  To: Alberto Garcia; +Cc: John Snow, qemu-devel, qemu-block, Max Reitz

Am 12.09.2019 um 09:25 hat Alberto Garcia geschrieben:
> On Thu 12 Sep 2019 01:33:05 AM CEST, John Snow <jsnow@redhat.com> wrote:
> >> This restriction comes from commit 095a9c58ce12afeeb90c2 from 2018.
> >
> > You accidentally typed a reasonably modern date. It's from *2008*!
> 
> Oh my, and I reviewed the message 3 times ... if this one gets committed
> please correct the date.

It was actually one of the very first QEMU patches I reviewed. :-)

Thanks, fixed the year and applied to the block branch.

Kevin


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 15:16 [Qemu-devel] [PATCH] qcow2: Stop overwriting compressed clusters one by one Alberto Garcia
2019-09-11 23:33 ` [Qemu-devel] [Qemu-block] " John Snow
2019-09-12  7:25   ` Alberto Garcia
2019-09-12  9:28     ` Kevin Wolf

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org qemu-devel@archiver.kernel.org
	public-inbox-index qemu-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox