On Mon, Oct 05 2009 at 12:33am -0400,
Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
> On Sun, Oct 04 2009 at 10:25pm -0400,
> Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
> > Hi
> >
> > I don't understand the purpose of this patch. I think it's useless.
>
> You don't understand it, so you think it's useless.. nice to know where
> I'm starting.
>
> > - during merge, nothing except the merge process reads the on-disk
> > exceptions. So it doesn't matter when you clean them. You must clean them
> > before dropping the interlock, but the order is not important.
>
> Sure, I thought the same but quickly realized that in practice order
> does matter if you'd like to avoid failing the in-core exception cleanup
> (IFF we allow chunks to be inserted before existing chunks; aka
> "back-merges").
>
> Your original approach to cleaning all on-disk exceptions then all
> in-core has no hope of working for "back-merges". Your original
> snapshot-merge completely eliminated insertions before existing chunks.
<snip - my long-winded defensiveness>
> Lesson I learned is that the in-core dm_snap_exception's consecutive
> chunk accounting is _really_ subtle. And while I agree there should be
> no relation between the proper cleanup of on-disk and in-core
> exceptions; doing the cleanup of each in lock-step appears to be the
> only way forward (that I found).
If I just judge the location of the chunk within the in-core exception
support for back-merges works just as well. Here is a minimalist patch
the accomplishes the same:
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index bcfbe85..b271f56 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -790,17 +790,25 @@ static void merge_callback(int read_err, unsigned long write_err, void *context)
* so that dm_consecutive_chunk_count_dec() accounting works
*/
for (i = s->merge_write_interlock_n - 1; i >= 0; i--) {
- e = lookup_exception(&s->complete,
- s->merge_write_interlock + i);
+ chunk_t old_chunk = s->merge_write_interlock + i;
+ e = lookup_exception(&s->complete, old_chunk);
if (!e) {
DMERR("exception for block %llu is on "
"disk but not in memory",
- (unsigned long long)
- s->merge_write_interlock + i);
+ (unsigned long long)old_chunk);
up_write(&s->lock);
goto shut;
}
if (dm_consecutive_chunk_count(e)) {
+ if (old_chunk == e->old_chunk) {
+ e->old_chunk++;
+ e->new_chunk++;
+ } else if (old_chunk != e->old_chunk +
+ dm_consecutive_chunk_count(e)) {
+ DMERR("merge from the middle of a chunk range");
+ up_write(&s->lock);
+ goto shut;
+ }
dm_consecutive_chunk_count_dec(e);
} else {
remove_exception(e);
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
