Kevin Wolf <kw...@redhat.com> wrote:
> When the refcount table grows, it doesn't only grow by one entry but reserves
> some space for future refcount blocks. The algorithm to calculate the number
> of
> entries stays the same with the fixes, so factor it out before replacing the
> rest.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> block/qcow2-refcount.c | 34 +++++++++++++++++++++++-----------
> 1 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 2fdc26b..0e2ecd7 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -123,6 +123,28 @@ static int get_refcount(BlockDriverState *bs, int64_t
> cluster_index)
> return be16_to_cpu(s->refcount_block_cache[block_index]);
> }
>
> +/*
> + * Rounds the refcount table size up to avoid growing the table for each
> single
> + * refcount block that is allocated.
> + */
> +static unsigned int next_refcount_table_size(BDRVQcowState *s,
> + unsigned int min_size)
> +{
> + unsigned int refcount_table_clusters = 0;
> + unsigned int new_table_size = 1;
> +
> + while (min_size > new_table_size) {
> + if (refcount_table_clusters == 0) {
> + refcount_table_clusters = 1;
> + } else {
> + refcount_table_clusters = (refcount_table_clusters * 3 + 1) / 2;
> + }
> + new_table_size = refcount_table_clusters << (s->cluster_bits - 3);
> + }
> +
> + return new_table_size;
> +}
> +
couldn't something like:
static unsigned int next_refcount_table_size(BDRVQcowState *s,
unsigned int min_size)
{
unsigned int refcount_table_clusters = 1;
unsigned int min_clusters = (min_size >> (s->clusters_bits -3)) + 1;
while (min_clusters > refcount_table_clusters) {
refcount_table_clusters = (refcount_table_clusters * 3 + 1) / 2;
}
return refconut_table_clusters << (s->cluster_bits - 3);
}
That removes the if from the inside loop, and makes the comparison in
clusters not in size. What do you think?
> static int grow_refcount_table(BlockDriverState *bs, int min_size)
> {
> BDRVQcowState *s = bs->opaque;
> @@ -136,17 +158,7 @@ static int grow_refcount_table(BlockDriverState *bs, int
> min_size)
> if (min_size <= s->refcount_table_size)
> return 0;
> /* compute new table size */
> - refcount_table_clusters = s->refcount_table_size >> (s->cluster_bits -
> 3);
> - for(;;) {
> - if (refcount_table_clusters == 0) {
> - refcount_table_clusters = 1;
> - } else {
> - refcount_table_clusters = (refcount_table_clusters * 3 + 1) / 2;
> - }
> - new_table_size = refcount_table_clusters << (s->cluster_bits - 3);
> - if (min_size <= new_table_size)
> - break;
> - }
> + new_table_size = next_refcount_table_size(s, min_size);
> #ifdef DEBUG_ALLOC2
> printf("grow_refcount_table from %d to %d\n",
> s->refcount_table_size,
