[dm-devel] Re: [PATCH v5 03/13] dm exception store: snapshot-merge usage accounting

[Mikulas Patocka]

On Mon, 7 Dec 2009, Mike Snitzer wrote:

> On Mon, Dec 07 2009 at  6:44pm -0500,
> Alasdair G Kergon <agk at redhat.com> 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 at redhat.com>
> 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 at redhat.com>
> Cc: Mikulas Patocka <mpatocka at redhat.com>
> ---
>  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;
>  }
> 

Acked-By: Mikulas Patocka <mpatocka at redhat.com>