On Fri, Oct 02 2009 at 1:26pm -0400, Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > I have updated the following quilt tree and folded Mikulas' clustered > snapshots patches into the end: > http://people.redhat.com/msnitzer/patches/snapshot-merge/kernel/2.6.31/ ... > I have done some light snapshot-merge testing to verify that this > updated snapshot-merge quilt tree builds and works as expected. I'll be > testing more extensively shortly (particularly back-merge). Mikulas, I had the harddrive die in the server I was using for snapshot-merge testing so my "more extensive testing" was delayed until today. In testing it was quite clear that back-merge was not working at all. I had to get creative with how to make back-merging work with the older-style snapshot-merge (which doesn't have jon's work to fold the 'complete' hashtable into the store; that really helped snapshot-merge support back-merges). I have updated my people page's quilt tree to include the following patch; I've now tested snapshot-merge to _really_ work with both just the snapshot-merge patches and also with your clustered snapshot patches ontop. Also, I'll be folding these changes into the appropriate snapshot-merge patches; the dm-snapshot-merge-support-insert-before-existing-chunk.patch at the end of the snapshot-merge patches is just a means to get all the working snapshot-merge bits in place. All things considered I think the following is about as clean as we can hope for but I welcome your (and others') review. Subject: [PATCH] Add clear_exception callback to the dm_exception_store_type's ->commit_merge This callback is used to clear in-core exceptions during the exception store's commit_merge. One _must_ clear the in-core exceptions prior to the on-disk exceptions. But this clearing of exceptions must be done fine-grained. One cannot clear all in-core exceptions and then clear all on-disk exceptions and arrive a a solution that is actually stable and works. Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> --- drivers/md/dm-exception-store.h | 5 ++ drivers/md/dm-snap-persistent.c | 30 ++++++++++------- drivers/md/dm-snap.c | 69 +++++++++++++++++++++++++--------------- 3 files changed, 66 insertions(+), 38 deletions(-) Index: linux-2.6/drivers/md/dm-exception-store.h =================================================================== --- linux-2.6.orig/drivers/md/dm-exception-store.h +++ linux-2.6/drivers/md/dm-exception-store.h @@ -91,8 +91,11 @@ struct dm_exception_store_type { /* * Clear the last n exceptions. * n must be <= the value returned by prepare_merge. + * callback is used to clear in-core exceptions. */ - int (*commit_merge) (struct dm_exception_store *store, int n); + int (*commit_merge) (struct dm_exception_store *store, int n, + int (*callback) (void *, chunk_t old, chunk_t new), + void *callback_context); /* * The snapshot is invalid, note this in the metadata. Index: linux-2.6/drivers/md/dm-snap-persistent.c =================================================================== --- linux-2.6.orig/drivers/md/dm-snap-persistent.c +++ linux-2.6/drivers/md/dm-snap-persistent.c @@ -410,15 +410,6 @@ static void write_exception(struct pstor e->new_chunk = cpu_to_le64(de->new_chunk); } -static void clear_exception(struct pstore *ps, uint32_t index) -{ - struct disk_exception *e = get_exception(ps, index); - - /* clear it */ - e->old_chunk = 0; - e->new_chunk = 0; -} - /* * Registers the exceptions that are present in the current area. * 'full' is filled in to indicate if the area has been @@ -726,15 +717,30 @@ static int persistent_prepare_merge(stru return i; } -static int persistent_commit_merge(struct dm_exception_store *store, int n) +static int persistent_commit_merge(struct dm_exception_store *store, int n, + int (*callback) (void *, + chunk_t old, chunk_t new), + void *callback_context) { int r, i; struct pstore *ps = get_info(store); BUG_ON(n > ps->current_committed); + BUG_ON(!callback); - for (i = 0; i < n; i++) - clear_exception(ps, ps->current_committed - 1 - i); + for (i = 0; i < n; i++) { + struct disk_exception *de = + get_exception(ps, ps->current_committed - 1 - i); + + /* clear in-core exception */ + r = callback(callback_context, de->old_chunk, de->new_chunk); + if (r < 0) + return r; + + /* clear disk exception */ + de->old_chunk = 0; + de->new_chunk = 0; + } r = area_io(ps, WRITE); if (r < 0) Index: linux-2.6/drivers/md/dm-snap.c =================================================================== --- linux-2.6.orig/drivers/md/dm-snap.c +++ linux-2.6/drivers/md/dm-snap.c @@ -764,11 +764,50 @@ static inline void release_write_interlo error_bios(b); } +static int clear_merged_exception(struct dm_snap_exception *e, + chunk_t old_chunk, chunk_t new_chunk) +{ + if (dm_consecutive_chunk_count(e)) { + if ((old_chunk == e->old_chunk) && + (new_chunk == dm_chunk_number(e->new_chunk))) { + e->old_chunk++; + e->new_chunk++; + } else if (old_chunk != e->old_chunk + + dm_consecutive_chunk_count(e) && + new_chunk != dm_chunk_number(e->new_chunk) + + dm_consecutive_chunk_count(e)) { + DMERR("merge from the middle of a chunk range"); + return -1; + } + dm_consecutive_chunk_count_dec(e); + } else { + remove_exception(e); + free_exception(e); + } + + return 0; +} + +static int merge_clear_exception_callback(void *context, + chunk_t old_chunk, chunk_t new_chunk) +{ + struct dm_snap_exception *e; + struct exception_table *complete_et = context; + + e = lookup_exception(complete_et, old_chunk); + if (!e) { + DMERR("exception for block %llu is on disk but not in memory", + (unsigned long long)old_chunk); + return -1; + } + + return clear_merged_exception(e, old_chunk, new_chunk); +} + static void merge_callback(int read_err, unsigned long write_err, void *context) { - int r, i; + int r; struct dm_snapshot *s = context; - struct dm_snap_exception *e; if (read_err || write_err) { if (read_err) @@ -778,35 +817,15 @@ static void merge_callback(int read_err, goto shut; } - r = s->store->type->commit_merge(s->store, s->merge_write_interlock_n); + r = s->store->type->commit_merge(s->store, s->merge_write_interlock_n, + merge_clear_exception_callback, + &s->complete); if (r < 0) { DMERR("Write error in exception store, shutting down merge"); goto shut; } down_write(&s->lock); - /* - * Must process chunks (and associated exceptions) in reverse - * 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); - if (!e) { - DMERR("exception for block %llu is on " - "disk but not in memory", - (unsigned long long) - s->merge_write_interlock + i); - up_write(&s->lock); - goto shut; - } - if (dm_consecutive_chunk_count(e)) { - dm_consecutive_chunk_count_dec(e); - } else { - remove_exception(e); - free_exception(e); - } - } release_write_interlock(s, 0); snapshot_merge_process(s); -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
