| Message ID | 1461106788-14285-3-git-send-email-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 04/20 00:59, Max Reitz wrote: > If the drive's dirty bitmap is dirtied while the mirror operation is > running, the cache of the iterator used by the mirror code may become > stale and not contain all dirty bits. > > This only becomes an issue if we are looking for contiguously dirty > chunks on the drive. In that case, we can easily detect the discrepancy > and just refresh the iterator if one occurs. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/mirror.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/block/mirror.c b/block/mirror.c > index 2714a77..9df1fae 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -334,6 +334,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) > } > > hbitmap_next = hbitmap_iter_next(&s->hbi); > + if (hbitmap_next > next_sector || hbitmap_next < 0) { > + /* The bitmap iterator's cache is stale, refresh it */ > + bdrv_set_dirty_iter(&s->hbi, next_sector); > + hbitmap_next = hbitmap_iter_next(&s->hbi); > + } > assert(hbitmap_next == next_sector); > nb_chunks++; > } > -- > 2.8.0 > Reviewed-by: Fam Zheng <famz@redhat.com>
Am 20.04.2016 um 00:59 hat Max Reitz geschrieben: > If the drive's dirty bitmap is dirtied while the mirror operation is > running, the cache of the iterator used by the mirror code may become > stale and not contain all dirty bits. > > This only becomes an issue if we are looking for contiguously dirty > chunks on the drive. In that case, we can easily detect the discrepancy > and just refresh the iterator if one occurs. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/mirror.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/block/mirror.c b/block/mirror.c > index 2714a77..9df1fae 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -334,6 +334,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) > } > > hbitmap_next = hbitmap_iter_next(&s->hbi); > + if (hbitmap_next > next_sector || hbitmap_next < 0) { > + /* The bitmap iterator's cache is stale, refresh it */ > + bdrv_set_dirty_iter(&s->hbi, next_sector); > + hbitmap_next = hbitmap_iter_next(&s->hbi); > + } > assert(hbitmap_next == next_sector); The iterator doesn't seem to be used afterwards anyway, so why not just use next_sector and stop using the iterator when we already know what result we want to get? Kevin
Am 20.04.2016 um 14:13 hat Kevin Wolf geschrieben: > Am 20.04.2016 um 00:59 hat Max Reitz geschrieben: > > If the drive's dirty bitmap is dirtied while the mirror operation is > > running, the cache of the iterator used by the mirror code may become > > stale and not contain all dirty bits. > > > > This only becomes an issue if we are looking for contiguously dirty > > chunks on the drive. In that case, we can easily detect the discrepancy > > and just refresh the iterator if one occurs. > > > > Signed-off-by: Max Reitz <mreitz@redhat.com> > > --- > > block/mirror.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/block/mirror.c b/block/mirror.c > > index 2714a77..9df1fae 100644 > > --- a/block/mirror.c > > +++ b/block/mirror.c > > @@ -334,6 +334,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) > > } > > > > hbitmap_next = hbitmap_iter_next(&s->hbi); > > + if (hbitmap_next > next_sector || hbitmap_next < 0) { > > + /* The bitmap iterator's cache is stale, refresh it */ > > + bdrv_set_dirty_iter(&s->hbi, next_sector); > > + hbitmap_next = hbitmap_iter_next(&s->hbi); > > + } > > assert(hbitmap_next == next_sector); > > The iterator doesn't seem to be used afterwards anyway, so why not just > use next_sector and stop using the iterator when we already know what > result we want to get? And of course I read that code completely wrong because in reality &s->hbi isn't local. Seems to be the right way to do things then. Kevin
diff --git a/block/mirror.c b/block/mirror.c index 2714a77..9df1fae 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -334,6 +334,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) } hbitmap_next = hbitmap_iter_next(&s->hbi); + if (hbitmap_next > next_sector || hbitmap_next < 0) { + /* The bitmap iterator's cache is stale, refresh it */ + bdrv_set_dirty_iter(&s->hbi, next_sector); + hbitmap_next = hbitmap_iter_next(&s->hbi); + } assert(hbitmap_next == next_sector); nb_chunks++; }
If the drive's dirty bitmap is dirtied while the mirror operation is running, the cache of the iterator used by the mirror code may become stale and not contain all dirty bits. This only becomes an issue if we are looking for contiguously dirty chunks on the drive. In that case, we can easily detect the discrepancy and just refresh the iterator if one occurs. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/mirror.c | 5 +++++ 1 file changed, 5 insertions(+)