On Mon, Dec 07 2009 at 6:44pm -0500, Alasdair G Kergon <agk@xxxxxxxxxx> wrote: > On Mon, Dec 07, 2009 at 04:10:53PM -0500, Mike Snitzer wrote: > > Subject: dm exception store: snapshot-merge usage accounting > > Comments updated a bit and folded into > dm-exception-store-add-merge-specific-methods.patch # 64745 OK, I did the following based on our chat.. but haven't looked at your version yet (ftp://sources.redhat.com appears to be down?). Mike --- From: Mike Snitzer <snitzer@xxxxxxxxxx> Subject: dm exception store: snapshot-merge usage accounting Adjust the persistent exception store's next_free to the last chunk that was processed in persistent_commit_merge(). prepare_merge() may change ps->current_area but not before commit_merge() processes 'nr_merged' chunks from it. Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> Cc: Mikulas Patocka <mpatocka@xxxxxxxxxx> --- drivers/md/dm-snap-persistent.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) Index: linux-rhel6/drivers/md/dm-snap-persistent.c =================================================================== --- linux-rhel6.orig/drivers/md/dm-snap-persistent.c +++ linux-rhel6/drivers/md/dm-snap-persistent.c @@ -119,7 +119,24 @@ struct pstore { chunk_t current_area; /* - * The next free chunk for an exception. + * 'next_free' can have two meanings. + * + * When creating exceptions: + * The next free chunk for an exception. 'next_free' means all + * the slots here and above are free. Though there may be + * holes in the exception store because chunks can be committed + * out of order -- if the system halts abruptly it could + * leave a hole. + * + * When merging exceptions: + * The last chunk that was merged from a linear extent of + * chunks [returned from persistent_prepare_merge()]. + * When merging 'next_free' does _not_ mean all the slots here + * and above are free. It holds the value it would have held + * if all chunks in an area had been committed in order. So + * the value may occasionally be slightly inaccurate, because + * chunks can be committed out of order, but since it's only + * used for 'status' this doesn't matter. */ chunk_t next_free; @@ -753,10 +770,16 @@ static int persistent_commit_merge(struc ps->current_committed -= nr_merged; /* - * Note that we make no attempt to keep ps->next_free up-to-date - * as exceptions may have been allocated out-of-order. - * Once a snapshot has become merging, nothing further uses it. - */ + * Update ps->next_free so persistent_usage() is accurate + * - it may be less than the actual 'next_free' chunk but we + * will not allocate exceptions again so this doesn't matter + * - must account for the first two metadata chunks + * - prepare_merge() may change ps->current_area but not before + * commit_merge() processes 'nr_merged' from the current_area + */ + ps->next_free = + (area_location(ps, ps->current_area) - 1 + + ps->current_committed + 2); return 0; } -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel