Re: [Qemu-devel] [PATCH 19/19] qcow2: Gather clusters in a looping loop

[Kevin Wolf <kwolf@redhat.com>]

Am 25.03.2013 um 18:30 hat Kevin Wolf geschrieben:
> Instead of just checking once in exactly this order if there are
> dependendies, non-COW clusters and new allocation, this starts looping
> around these. This way we can, for example, gather non-COW clusters after
> new allocations as long as the host cluster offsets stay contiguous.
> 
> Once handle_dependencies() is extended so that COW areas of in-flight
> allocations can be overwritten, this allows to continue with gathering
> other clusters (we wouldn't be able to do that without this change
> because we would have missed a possible second dependency in one of the
> next clusters).
> 
> This means that in the typical sequential write case, we can combine the
> COW overwrite of one cluster with the allocation of the next cluster as
> soon as something like Delayed COW gets actually implemented. It is only
> by avoiding splitting requests this way that Delayed COW actually starts
> improving performance noticably.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Self NAK.

> @@ -1159,9 +1178,12 @@ again:
>           *         the right synchronisation between the in-flight request and
>           *         the new one.
>           */
> -        cur_bytes = remaining;
>          ret = handle_dependencies(bs, start, &cur_bytes);
>          if (ret == -EAGAIN) {
> +            /* Currently handle_dependencies() doesn't yield if we already had
> +             * an allocation. If it did, we would have to clean up the L2Meta
> +             * structs before starting over. */
> +            assert(*m == NULL);

This assertion doesn't actually hold true. Reordering patches is
dangerous.

I also noticed that somewhere in the series I must introduce a
performance regression. This should serve as extra motivation for
reviewers - there is actually something to find!

Kevin