[PATCH] dm snapshot: revise snapshot_resume's exception handover

[PATCH] dm snapshot: revise snapshot_resume's exception handover

[Mike Snitzer]

Until now snapshot_resume only allowed the resuming snapshot to get the
exception store from another snapshot.

Revise snapshot_resume's exception handover to allow a snapshot to
handover its exceptions to another snapshot prior to resuming.

This is needed to accomodate the following sequence:
new_snapshot->ctr
old_snapshot->suspend
old_snapshot->resume
new_snapshot->resume

lvm2 relies on this sequence if 'lvchange --refresh' is used to start a
snapshot merge that was deferred until both origin and snapshot were not
in use.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-snap.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

Index: linux-2.6-dev/drivers/md/dm-snap.c
===================================================================
--- linux-2.6-dev.orig/drivers/md/dm-snap.c
+++ linux-2.6-dev/drivers/md/dm-snap.c
@@ -1268,16 +1268,27 @@ static void snapshot_resume(struct dm_ta
 	struct dm_snapshot *s = ti->private;
 
 	down_write(&s->lock);
-	if (s->handover) {
-		/* Get exception store from another snapshot */
+	if (s->handover_snap) {
+		/*
+		 * Initially assumes this snapshot will get
+		 * exception store from another snapshot
+		 */
 		struct dm_snapshot *old_snap = s->handover_snap;
-		BUG_ON(!old_snap);
-		down_write_nested(&old_snap->lock, SINGLE_DEPTH_NESTING);
-		handover_exceptions(old_snap, s);
-		up_write(&old_snap->lock);
+		struct dm_snapshot *new_snap = s;
+		struct dm_snapshot *lock_snap = old_snap;
+		if (!s->handover) {
+			/* Handover exceptions to another snapshot */
+			old_snap = s;
+			new_snap = s->handover_snap;
+			lock_snap = new_snap;
+		}
+		down_write_nested(&lock_snap->lock,
+				  SINGLE_DEPTH_NESTING);
+		handover_exceptions(old_snap, new_snap);
+		up_write(&lock_snap->lock);
 	}
 	/* An incomplete exception handover is not allowed */
-	BUG_ON(s->handover || s->handover_snap);
+	BUG_ON(s->handover_snap);
 	s->active = 1;
 	s->suspended = 0;
 	up_write(&s->lock);

[PATCH v2] dm snapshot: revise snapshot_resume's exception handover

[Mike Snitzer]

Until now snapshot_resume only allowed the resuming snapshot to get the
exception store from another snapshot.

Revise snapshot_resume's exception handover to allow a snapshot to
handover its exceptions to another snapshot prior to resuming.

This is needed to accomodate the following sequence:
new_snapshot->ctr
old_snapshot->suspend
old_snapshot->resume
new_snapshot->resume

lvm2 relies on this sequence if 'lvchange --refresh' is used to start a
snapshot merge that was deferred until both origin and snapshot were not
in use.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-snap.c |   28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

v2: update 'handover' documentation

Index: linux-2.6-dev/drivers/md/dm-snap.c
===================================================================
--- linux-2.6-dev.orig/drivers/md/dm-snap.c
+++ linux-2.6-dev/drivers/md/dm-snap.c
@@ -83,8 +83,9 @@ struct dm_snapshot {
 	 * 'handover' is set during a new snapshot's constructor if it finds
 	 * a single old snapshot is using the same cow block device as it.
 	 * Handover operation is performed, and 'handover' is cleared,
-	 * when either of the following occurs:
+	 * when one of the following occurs:
 	 * - old snapshot, that is handing over, is destructed
+	 * - old snapshot, that is handing over, is resumed
 	 * - new snapshot, that is accepting the handover, is resumed
 	 */
 	int handover;
@@ -1268,16 +1269,27 @@ static void snapshot_resume(struct dm_ta
 	struct dm_snapshot *s = ti->private;
 
 	down_write(&s->lock);
-	if (s->handover) {
-		/* Get exception store from another snapshot */
+	if (s->handover_snap) {
+		/*
+		 * Initially assumes this snapshot will get
+		 * exception store from another snapshot
+		 */
 		struct dm_snapshot *old_snap = s->handover_snap;
-		BUG_ON(!old_snap);
-		down_write_nested(&old_snap->lock, SINGLE_DEPTH_NESTING);
-		handover_exceptions(old_snap, s);
-		up_write(&old_snap->lock);
+		struct dm_snapshot *new_snap = s;
+		struct dm_snapshot *lock_snap = old_snap;
+		if (!s->handover) {
+			/* Handover exceptions to another snapshot */
+			old_snap = s;
+			new_snap = s->handover_snap;
+			lock_snap = new_snap;
+		}
+		down_write_nested(&lock_snap->lock,
+				  SINGLE_DEPTH_NESTING);
+		handover_exceptions(old_snap, new_snap);
+		up_write(&lock_snap->lock);
 	}
 	/* An incomplete exception handover is not allowed */
-	BUG_ON(s->handover || s->handover_snap);
+	BUG_ON(s->handover_snap);
 	s->active = 1;
 	s->suspended = 0;
 	up_write(&s->lock);

Re: [PATCH v2] dm snapshot: revise snapshot_resume's exception handover

[Mike Snitzer]

On Wed, Nov 04 2009 at  5:11pm -0500,
Mike Snitzer <snitzer@redhat.com> wrote:

> Until now snapshot_resume only allowed the resuming snapshot to get the
> exception store from another snapshot.
> 
> Revise snapshot_resume's exception handover to allow a snapshot to
> handover its exceptions to another snapshot prior to resuming.
> 
> This is needed to accomodate the following sequence:
> new_snapshot->ctr
> old_snapshot->suspend
> old_snapshot->resume
> new_snapshot->resume
> 
> lvm2 relies on this sequence if 'lvchange --refresh' is used to start a
> snapshot merge that was deferred until both origin and snapshot were not
> in use.

FYI, the above is the last of 3 patches in the "exception handover"
series, the patch order is:

dm snapshot: initial support for exception handover
http://patchwork.kernel.org/patch/55022/

dm snapshot: exception handover improvements
http://patchwork.kernel.org/patch/55569/

dm snapshot: revise snapshot_resume's exception handover
http://patchwork.kernel.org/patch/57787/

Re: [PATCH v2] dm snapshot: revise snapshot_resume's exception handover

[Mike Snitzer]

On Wed, Nov 04 2009 at  5:20pm -0500,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Wed, Nov 04 2009 at  5:11pm -0500,
> Mike Snitzer <snitzer@redhat.com> wrote:
> 
> > Until now snapshot_resume only allowed the resuming snapshot to get the
> > exception store from another snapshot.
> > 
> > Revise snapshot_resume's exception handover to allow a snapshot to
> > handover its exceptions to another snapshot prior to resuming.
> > 
> > This is needed to accomodate the following sequence:
> > new_snapshot->ctr
> > old_snapshot->suspend
> > old_snapshot->resume
> > new_snapshot->resume
> > 
> > lvm2 relies on this sequence if 'lvchange --refresh' is used to start a
> > snapshot merge that was deferred until both origin and snapshot were not
> > in use.
> 
> FYI, the above is the last of 3 patches in the "exception handover"
> series, the patch order is:

Ok, actually this 'suspended' flag patch is the first; so its 4 patches:

dm snapshot: add suspended flag to dm_snapshot
http://patchwork.kernel.org/patch/55021/
 
> dm snapshot: initial support for exception handover
> http://patchwork.kernel.org/patch/55022/
> 
> dm snapshot: exception handover improvements
> http://patchwork.kernel.org/patch/55569/
> 
> dm snapshot: revise snapshot_resume's exception handover
> http://patchwork.kernel.org/patch/57787/

Re: [PATCH v2] dm snapshot: revise snapshot_resume's exception handover

[Alasdair G Kergon]

On Wed, Nov 04, 2009 at 05:28:34PM -0500, Mike Snitzer wrote:
> Ok, actually this 'suspended' flag patch is the first; so its 4 patches:
> 
> dm snapshot: add suspended flag to dm_snapshot
> http://patchwork.kernel.org/patch/55021/
>  
> > dm snapshot: initial support for exception handover
> > http://patchwork.kernel.org/patch/55022/
> > 
> > dm snapshot: exception handover improvements
> > http://patchwork.kernel.org/patch/55569/
> > 
> > dm snapshot: revise snapshot_resume's exception handover
> > http://patchwork.kernel.org/patch/57787/

As quite a few changes get superceded by later changes, with added lines
later removed, I've merged the last three of those together into a single
patch for now.

Alasdair

Re: [PATCH v2] dm snapshot: revise snapshot_resume's exception handover

[Mike Snitzer]

On Wed, Nov 04 2009 at  8:44pm -0500,
Alasdair G Kergon <agk@redhat.com> wrote:

> On Wed, Nov 04, 2009 at 05:28:34PM -0500, Mike Snitzer wrote:
> > Ok, actually this 'suspended' flag patch is the first; so its 4 patches:
> > 
> > dm snapshot: add suspended flag to dm_snapshot
> > http://patchwork.kernel.org/patch/55021/
> >  
> > > dm snapshot: initial support for exception handover
> > > http://patchwork.kernel.org/patch/55022/
> > > 
> > > dm snapshot: exception handover improvements
> > > http://patchwork.kernel.org/patch/55569/
> > > 
> > > dm snapshot: revise snapshot_resume's exception handover
> > > http://patchwork.kernel.org/patch/57787/
> 
> As quite a few changes get superceded by later changes, with added lines
> later removed, I've merged the last three of those together into a single
> patch for now.

Sounds good.  I kept them split out so we could see the evolution during
the initial review.

Mike

Re: [PATCH v2] dm snapshot: revise snapshot_resume's exception handover

[Mike Snitzer]

On Wed, Nov 04 2009 at  8:44pm -0500,
Alasdair G Kergon <agk@redhat.com> wrote:

> On Wed, Nov 04, 2009 at 05:28:34PM -0500, Mike Snitzer wrote:
> > Ok, actually this 'suspended' flag patch is the first; so its 4 patches:
> > 
> > dm snapshot: add suspended flag to dm_snapshot
> > http://patchwork.kernel.org/patch/55021/
> >  
> > > dm snapshot: initial support for exception handover
> > > http://patchwork.kernel.org/patch/55022/
> > > 
> > > dm snapshot: exception handover improvements
> > > http://patchwork.kernel.org/patch/55569/
> > > 
> > > dm snapshot: revise snapshot_resume's exception handover
> > > http://patchwork.kernel.org/patch/57787/
> 
> As quite a few changes get superceded by later changes, with added lines
> later removed, I've merged the last three of those together into a single
> patch for now.

This patch goes along with the other handover patches too:

dm exception store: do not read metadata if going to handover exceptions
http://patchwork.kernel.org/patch/55024/