[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.6 1/2] block/mirror: Revive dead yielding
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH for-2.6 1/2] block/mirror: Revive dead yielding code |
Date: |
Wed, 20 Apr 2016 13:47:12 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, 04/20 00:59, Max Reitz wrote:
> mirror_iteration() is supposed to wait if the current chunk is subject
> to a still in-flight mirroring operation. However, it mixed checking
> this conflict situation with checking the dirty status of a chunk. A
> simplification for the latter condition (the first chunk encountered is
> always dirty) led to neglecting the former: We just skip the first chunk
> and thus never test whether it conflicts with an in-flight operation.
>
> To fix this, pull out the code which waits for in-flight operations on
> the first chunk of the range to be mirrored to settle.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/mirror.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index c2cfc1a..2714a77 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -298,7 +298,7 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s,
> static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
> {
> BlockDriverState *source = s->common.bs;
> - int64_t sector_num;
> + int64_t sector_num, first_chunk;
> uint64_t delay_ns = 0;
> /* At least the first dirty chunk is mirrored in one iteration. */
> int nb_chunks = 1;
> @@ -313,6 +313,12 @@ static uint64_t coroutine_fn
> mirror_iteration(MirrorBlockJob *s)
> assert(sector_num >= 0);
> }
>
> + first_chunk = sector_num / sectors_per_chunk;
> + while (test_bit(first_chunk, s->in_flight_bitmap)) {
> + trace_mirror_yield_in_flight(s, first_chunk, s->in_flight);
> + mirror_wait_for_io(s);
> + }
> +
> /* Find the number of consective dirty chunks following the first dirty
> * one, and wait for in flight requests in them. */
> while (nb_chunks * sectors_per_chunk < (s->buf_size >>
> BDRV_SECTOR_BITS)) {
> @@ -324,17 +330,12 @@ static uint64_t coroutine_fn
> mirror_iteration(MirrorBlockJob *s)
> break;
> }
> if (test_bit(next_chunk, s->in_flight_bitmap)) {
Now this can be combiled into the previous if block.
> - if (nb_chunks > 0) {
> - break;
> - }
> - trace_mirror_yield_in_flight(s, next_sector, s->in_flight);
> - mirror_wait_for_io(s);
> - /* Now retry. */
> - } else {
> - hbitmap_next = hbitmap_iter_next(&s->hbi);
> - assert(hbitmap_next == next_sector);
> - nb_chunks++;
> + break;
> }
> +
> + hbitmap_next = hbitmap_iter_next(&s->hbi);
> + assert(hbitmap_next == next_sector);
> + nb_chunks++;
> }
>
> /* Clear dirty bits before querying the block status, because
> --
> 2.8.0
>
Reviewed-by: Fam Zheng <address@hidden>