qcow2: Order concurrent AIO requests on the same unallocated cluster
PATCH 1/1 qcow2: Order concurrent AIO requests on the same unallocated cluster
When two AIO requests write to the same cluster, and this cluster is unallocated, currently both requests allocate a new cluster and the second one merges the first one when it is completed. This means an cluster allocation, a read and a cluster deallocation which cause some overhead. If we simply let the second request wait until the first one is done, we improve overall performance with AIO requests (specifially, qcow2/virtio combinations).
This patch maintains a list of in-flight requests that have allocated new clusters. A second request touching the same cluster is limited so that it either doesn't touch the allocation of the first request (so it can have a non-overlapping allocation) or it waits for the first request to complete.
--- block/qcow2-cluster.c | 39 ++++++++++++++++++++++++++++++++++ block/qcow2.c | 55 ++++++++++++++++++++++++++++++++++++++++++++---- block/qcow2.h | 7 ++++++ 3 files changed, 96 insertions(+), 5 deletions(-)
block/qcow2-cluster.c +39 -0
@@ -684,6 +684,7 @@ uint64_t qcow2_alloc_cluster_offset(BlockDriverState *bs,int l2_index, ret;uint64_t l2_offset, *l2_table, cluster_offset;int nb_clusters, i =0 ; +QCowL2Meta *old_alloc; ret =get_cluster_table (bs, offset, &l2_table, &l2_offset, &l2_index);if (ret ==0 )
@@ -732,6 +733,44 @@ uint64_t qcow2_alloc_cluster_offset(BlockDriverState *bs, } nb_clusters = i; +/* +* Check if there already is an AIO write request in flight which allocates +* the same cluster. In this case we need to wait until the previous +* request has completed and updated the L2 table accordingly. +*/ + LIST_FOREACH(old_alloc, &s->cluster_allocs, next_in_flight) { + +uint64_t end_offset = offset + nb_clusters * s->cluster_size; +uint64_t old_offset = old_alloc->offset; +uint64_t old_end_offset = old_alloc->offset + + old_alloc->nb_clusters * s->cluster_size; + +if (end_offset < old_offset || offset > old_end_offset) { +/* No intersection */ + }else { +if (offset < old_offset) { +/* Stop at the start of a running allocation */ + nb_clusters = (old_offset - offset) >> s->cluster_bits; + }else { + nb_clusters =0 ; + } + +if (nb_clusters ==0 ) { +/* Set dependency and wait for a callback */ + m->depends_on = old_alloc; + m->nb_clusters =0 ; + *num =0 ; +return 0 ; + } + } + } + +if (!nb_clusters) { +abort (); + } + +LIST_INSERT_HEAD (&s->cluster_allocs, m, next_in_flight); +/* allocate a new cluster */ cluster_offset =qcow2_alloc_clusters (bs, nb_clusters * s->cluster_size);
block/qcow2.c +50 -5
106 lines not on this page · raw message
block/qcow2.h +7 -0
@@ -98,6 +98,7 @@ typedef struct BDRVQcowState {uint8_t *cluster_cache;uint8_t *cluster_data;uint64_t cluster_cache_offset; + LIST_HEAD(QCowClusterAlloc, QCowL2Meta) cluster_allocs;uint64_t *refcount_table;uint64_t refcount_table_offset;
@@ -128,6 +129,8 @@ typedef struct QCowCreateState {int64_t refcount_block_offset; } QCowCreateState; +struct QCowAIOCB ; +/* XXX This could be private for qcow2-cluster.c */ typedef struct QCowL2Meta {
@@ -135,6 +138,10 @@ typedef struct QCowL2Metaint n_start;int nb_available;int nb_clusters; +struct QCowL2Meta *depends_on; + LIST_HEAD(QCowAioDependencies, QCowAIOCB) dependent_requests; + +LIST_ENTRY (QCowL2Meta) next_in_flight; } QCowL2Meta;static inline int size_to_clusters (BDRVQcowState *s,int64_t size)
Signature · 1 line
1.6.0.6
Quoted 7 lines · Kevin Wolf
When two AIO requests write to the same cluster, and this cluster is unallocated, currently both requests allocate a new cluster and the second one merges the first one when it is completed. This means an cluster allocation, a read and a cluster deallocation which cause some overhead. If we simply let the second request wait until the first one is done, we improve overall performance with AIO requests (specifially, qcow2/virtio combinations).
Quoted 5 lines
This patch maintains a list of in-flight requests that have allocated new clusters. A second request touching the same cluster is limited so that it either doesn't touch the allocation of the first request (so it can have a non-overlapping allocation) or it waits for the first request to complete.
Can't this cause an even/odd pattern where all even-numbered requests run first, then all the odd-numbered requests?
(0 goes to disk, 1 depends on it, 2 depends on 1, which isn't allocating now, so it goes to disk, 3 depends on 2, ...)
Do you have performance numbers?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 14 lines · Avi Kivity
Quoted 7 lines · Kevin Wolf
When two AIO requests write to the same cluster, and this cluster is unallocated, currently both requests allocate a new cluster and the second one merges the first one when it is completed. This means an cluster allocation, a read and a cluster deallocation which cause some overhead. If we simply let the second request wait until the first one is done, we improve overall performance with AIO requests (specifially, qcow2/virtio combinations).
Quoted 5 lines
This patch maintains a list of in-flight requests that have allocated new clusters. A second request touching the same cluster is limited so that it either doesn't touch the allocation of the first request (so it can have a non-overlapping allocation) or it waits for the first request to complete.
Quoted 5 lines
Can't this cause an even/odd pattern where all even-numbered requests run first, then all the odd-numbered requests?
(0 goes to disk, 1 depends on it, 2 depends on 1, which isn't allocating now, so it goes to disk, 3 depends on 2, ...)
I guess it can happen in theory, not sure if it matters in practice. You are worried about image fragmentation? I think we could do something about it with a cleverer cluster allocation.
However, I don't think it's an argument against this patch. What currently happens isn't much better: Allocate n clusters, free n-1. Almost as good in producing fragmentation.
Quoted 1 line
Do you have performance numbers?
No really detailed numbers. Installation time for RHEL on qcow2/virtio went down from 34 min to 19 min, though.
Kevin
Quoted 8 lines · Kevin Wolf
Quoted 7 lines
Can't this cause an even/odd pattern where all even-numbered requests run first, then all the odd-numbered requests?
(0 goes to disk, 1 depends on it, 2 depends on 1, which isn't allocating now, so it goes to disk, 3 depends on 2, ...)
I guess it can happen in theory, not sure if it matters in practice.
We should check then.
Quoted 4 lines
You
are worried about image fragmentation? I think we could do something about it with a cleverer cluster allocation.
Not only image fragmentation - the odd requests will require RMW.
Quoted 4 lines
However, I don't think it's an argument against this patch. What currently happens isn't much better: Allocate n clusters, free n-1. Almost as good in producing fragmentation.
The patch introduces complexity so it makes working towards a non-fragmenting solution harder. I'm not saying it could be simplified, it's a side effect of using a state machine design.
Quoted 5 lines
Quoted 2 lines
Do you have performance numbers?
No really detailed numbers. Installation time for RHEL on qcow2/virtio went down from 34 min to 19 min, though.
That's very impressive. cache=none or cache=wt?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 25 lines · Avi Kivity
Quoted 7 lines · Kevin Wolf
Quoted 6 lines
Can't this cause an even/odd pattern where all even-numbered requests run first, then all the odd-numbered requests?
(0 goes to disk, 1 depends on it, 2 depends on 1, which isn't allocating now, so it goes to disk, 3 depends on 2, ...)
I guess it can happen in theory, not sure if it matters in practice.
We should check then.
Quoted 3 lines
You
are worried about image fragmentation? I think we could do something about it with a cleverer cluster allocation.Not only image fragmentation - the odd requests will require RMW.
Quoted 3 lines
However, I don't think it's an argument against this patch. What currently happens isn't much better: Allocate n clusters, free n-1. Almost as good in producing fragmentation.
The patch introduces complexity so it makes working towards a non-fragmenting solution harder. I'm not saying it could be simplified, it's a side effect of using a state machine design.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Do you have performance numbers?
No really detailed numbers. Installation time for RHEL on qcow2/virtio went down from 34 min to 19 min, though.
That's very impressive. cache=none or cache=wt?
And how it compares with raw?
--
Gleb.
Quoted 5 lines · Gleb Natapov
Quoted 4 lines
Quoted 2 lines
No really detailed numbers. Installation time for RHEL on qcow2/virtio went down from 34 min to 19 min, though.
That's very impressive. cache=none or cache=wt?
And how it compares with raw?
I just ran another installation on raw and it took about 16:30 min.
And with the next patch series that I'm going to send today we'll be in the same region with qcow2.
Kevin
Quoted 8 lines · Kevin Wolf
Quoted 6 lines · Gleb Natapov
Quoted 6 lines
Quoted 3 lines
No really detailed numbers. Installation time for RHEL on qcow2/virtio went down from 34 min to 19 min, though.
That's very impressive. cache=none or cache=wt?
Quoted 7 lines
Quoted 2 lines
And how it compares with raw?
I just ran another installation on raw and it took about 16:30 min.
And with the next patch series that I'm going to send today we'll be in the same region with qcow2.
Looks pretty good then. Was that 'raw' as in 'disk partition', or as 'raw file on ext3'?
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 11 lines · Avi Kivity
Quoted 10 lines · Kevin Wolf
Quoted 8 lines · Gleb Natapov
Quoted 6 lines
Quoted 3 lines
No really detailed numbers. Installation time for RHEL on qcow2/virtio went down from 34 min to 19 min, though.
That's very impressive. cache=none or cache=wt?
And how it compares with raw?
Quoted 8 lines
Quoted 5 lines
I just ran another installation on raw and it took about 16:30 min.
And with the next patch series that I'm going to send today we'll be in the same region with qcow2.
Looks pretty good then. Was that 'raw' as in 'disk partition', or as 'raw file on ext3'?
raw file on ext3. As I said this is just manual ad-hoc testing on my laptop, so you certainly can do better benchmarks than these. And I think I'll do some later this week.
Kevin
Quoted 7 lines · Avi Kivity
Quoted 4 lines · Kevin Wolf
You
are worried about image fragmentation? I think we could do something about it with a cleverer cluster allocation.Not only image fragmentation - the odd requests will require RMW.
How that?
The case you're thinking of is that the first and third request are already completed and then the second one starts, right? Assuming that request 2 involves some sectors in the last cluster of 1 and the first one of 3.
Then request 2 is written in three phases: The first one overwrites the last sectors of requests 1 (cluster already allocated => no RMW). The second one writes to unallocated, cluster-aligned space (writing complete clusters => no RMW). The third one overwrites the first sectors of request 3 (cluster already allocated => no RMW).
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Do you have performance numbers?
No really detailed numbers. Installation time for RHEL on qcow2/virtio went down from 34 min to 19 min, though.
That's very impressive. cache=none or cache=wt?
This was with cache=none as are most of my ad-hoc tests (with cache=none being the default for RHEL).
Kevin
Quoted 18 lines · Kevin Wolf
Quoted 9 lines · Avi Kivity
Quoted 5 lines · Kevin Wolf
You
are worried about image fragmentation? I think we could do something about it with a cleverer cluster allocation.Not only image fragmentation - the odd requests will require RMW.
How that?
The case you're thinking of is that the first and third request are already completed and then the second one starts, right? Assuming that request 2 involves some sectors in the last cluster of 1 and the first one of 3.
Quoted 6 lines
Then request 2 is written in three phases: The first one overwrites the last sectors of requests 1 (cluster already allocated => no RMW). The second one writes to unallocated, cluster-aligned space (writing complete clusters => no RMW). The third one overwrites the first sectors of request 3 (cluster already allocated => no RMW).
Ah, ok. No RMW, but a three-way split as well as fragmentation.
Signature · 1 line
error compiling committee.c: too many arguments to function