| Message ID | 20240513063203.113911-4-andrey.drobyshev@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: make subclusters discardable | expand |
On 5/13/24 08:31, Andrey Drobyshev wrote: > Normally discard requests are stored in the queue attached to BDRVQcow2State > to be processed later at once. Currently discard-no-unref option handling > causes these requests to be processed straight away. Let's fix that. > > Note that when doing regular discards qcow2_free_any_cluster() would check > for the presence of external data files for us and redirect request to > underlying data_file. Here we want to do the same but avoid refcount updates, > thus we perform the same checks. > > Suggested-by: Hanna Czenczek <hreitz@redhat.com> > Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> > --- > block/qcow2-cluster.c | 39 +++++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 10 deletions(-) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index 5f057ba2fd..7dff0bd5a1 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -1893,6 +1893,28 @@ again: > return 0; > } > > +/* > + * Helper for adding a discard request to the queue without any refcount > + * modifications. If external data file is used redirects the request to > + * the corresponding BdrvChild. > + */ > +static inline void > +discard_no_unref_any_file(BlockDriverState *bs, uint64_t offset, > + uint64_t length, QCow2ClusterType ctype, > + enum qcow2_discard_type dtype) > +{ > + BDRVQcow2State *s = bs->opaque; > + > + if (s->discard_passthrough[dtype] && > + (ctype == QCOW2_CLUSTER_NORMAL || ctype == QCOW2_CLUSTER_ZERO_ALLOC)) { > + if (has_data_file(bs)) { > + bdrv_pdiscard(s->data_file, offset, length); > + } else { > + qcow2_queue_discard(bs, offset, length); > + } > + } > +} > + > /* > * This discards as many clusters of nb_clusters as possible at once (i.e. > * all clusters in the same L2 slice) and returns the number of discarded > @@ -1974,12 +1996,10 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters, > if (!keep_reference) { > /* Then decrease the refcount */ > qcow2_free_any_cluster(bs, old_l2_entry, type); > - } else if (s->discard_passthrough[type] && > - (cluster_type == QCOW2_CLUSTER_NORMAL || > - cluster_type == QCOW2_CLUSTER_ZERO_ALLOC)) { > + } else { > /* If we keep the reference, pass on the discard still */ > - bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK, > - s->cluster_size); > + discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK, > + s->cluster_size, cluster_type, type); > } > } > > @@ -2088,12 +2108,11 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset, > if (!keep_reference) { > /* Then decrease the refcount */ > qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST); > - } else if (s->discard_passthrough[QCOW2_DISCARD_REQUEST] && > - (type == QCOW2_CLUSTER_NORMAL || > - type == QCOW2_CLUSTER_ZERO_ALLOC)) { > + } else { > /* If we keep the reference, pass on the discard still */ > - bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK, > - s->cluster_size); > + discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK, > + s->cluster_size, type, > + QCOW2_DISCARD_REQUEST); > } > } > } Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 5f057ba2fd..7dff0bd5a1 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1893,6 +1893,28 @@ again: return 0; } +/* + * Helper for adding a discard request to the queue without any refcount + * modifications. If external data file is used redirects the request to + * the corresponding BdrvChild. + */ +static inline void +discard_no_unref_any_file(BlockDriverState *bs, uint64_t offset, + uint64_t length, QCow2ClusterType ctype, + enum qcow2_discard_type dtype) +{ + BDRVQcow2State *s = bs->opaque; + + if (s->discard_passthrough[dtype] && + (ctype == QCOW2_CLUSTER_NORMAL || ctype == QCOW2_CLUSTER_ZERO_ALLOC)) { + if (has_data_file(bs)) { + bdrv_pdiscard(s->data_file, offset, length); + } else { + qcow2_queue_discard(bs, offset, length); + } + } +} + /* * This discards as many clusters of nb_clusters as possible at once (i.e. * all clusters in the same L2 slice) and returns the number of discarded @@ -1974,12 +1996,10 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters, if (!keep_reference) { /* Then decrease the refcount */ qcow2_free_any_cluster(bs, old_l2_entry, type); - } else if (s->discard_passthrough[type] && - (cluster_type == QCOW2_CLUSTER_NORMAL || - cluster_type == QCOW2_CLUSTER_ZERO_ALLOC)) { + } else { /* If we keep the reference, pass on the discard still */ - bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK, - s->cluster_size); + discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK, + s->cluster_size, cluster_type, type); } } @@ -2088,12 +2108,11 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset, if (!keep_reference) { /* Then decrease the refcount */ qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST); - } else if (s->discard_passthrough[QCOW2_DISCARD_REQUEST] && - (type == QCOW2_CLUSTER_NORMAL || - type == QCOW2_CLUSTER_ZERO_ALLOC)) { + } else { /* If we keep the reference, pass on the discard still */ - bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK, - s->cluster_size); + discard_no_unref_any_file(bs, old_l2_entry & L2E_OFFSET_MASK, + s->cluster_size, type, + QCOW2_DISCARD_REQUEST); } } }
Normally discard requests are stored in the queue attached to BDRVQcow2State to be processed later at once. Currently discard-no-unref option handling causes these requests to be processed straight away. Let's fix that. Note that when doing regular discards qcow2_free_any_cluster() would check for the presence of external data files for us and redirect request to underlying data_file. Here we want to do the same but avoid refcount updates, thus we perform the same checks. Suggested-by: Hanna Czenczek <hreitz@redhat.com> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> --- block/qcow2-cluster.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-)