Il 06/11/2013 13:56, Charlie Shepherd ha scritto:
> Rather than unnecessarily setting bits that are already set, re-use
> cow_find_streak to find how
> many bits are already set for this sector, and only set unset bits. Do this
> before the flush to
> avoid it if no bits need to be set at all.
>
> Signed-off-by: Charlie Shepherd <char...@ctshepherd.com>
> ---
> block/cow.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/block/cow.c b/block/cow.c
> index 41097d8..93207eb 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -203,7 +203,7 @@ static int cow_update_bitmap(BlockDriverState *bs,
> int64_t sector_num,
> bool first = true;
>
> while (nb_sectors) {
You still need to make this a "for" and put "offset += BDRV_SECTOR_SIZE"
in the third clause.
> - int ret;
> + int ret, set;
> uint8_t bitmap[BDRV_SECTOR_SIZE];
>
> bitnum &= BITS_PER_BITMAP_SECTOR - 1;
> @@ -214,6 +214,15 @@ static int cow_update_bitmap(BlockDriverState *bs,
> int64_t sector_num,
> return ret;
> }
>
> + /* Skip over any already set bits */
> + set = cow_find_streak(bitmap, 1, bitnum, sector_bits);
> + bitnum += set;
> + sector_bits -= set;
> + nb_sectors -= set;
> + if (set == sector_bits) {
> + continue;
> + }
This now has to be "if (set == 0)". With the change to the "for" above,
that's correct.
> if (first) {
> ret = bdrv_flush(bs->file);
> if (ret < 0) {
> @@ -228,7 +237,6 @@ static int cow_update_bitmap(BlockDriverState *bs,
> int64_t sector_num,
> if (ret < 0) {
> return ret;
> }
> -
> bitnum += sector_bits;
> nb_sectors -= sector_bits;
> offset += BDRV_SECTOR_SIZE;
>
I also noticed that patch 1 introduces a small regression, in that it
will always flush data even if the metadata doesn't change. This patch
fixes it, because the "bdrv_flush" is preceded by the check to skip over
any already set bits.
So I suggest that, together with the above fixes, you squash patches 1
and 3 together.
Paolo
