[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[dm-devel] removing "active" variable in snapshots



Hi

While working on snapshot merging, I found that there is "active" entry in struct dm_snapshot and it looks that it has no meaning:

This variable is set to 0 on snapshot creation in snapshot_ctr
It is set to 1 on snapshot resume (call is made after every creation)
This variable is never set to 0 again.

This variable is read only in __origin_write and snapshots with active == 0 are skipped.

It looks to me, like it could be removed, the origin is resumed after all snapshots, so there is no window where active could be "0" in __origin_write. Furthermore, if such condition existed, writing to origin without writing to snapshot would damage the snapshot --- so even if "active" ever has some effect, it would only result in data corruption.

I need it to be removed for merging, I start merging when snapshot is resumed and at this point all other snapshots must be able to accept new exceptions, if I excluded a snapshot, it would be corrupted.

Would you agree with this patch to remove the "active" field, or do you see some reason for it?

Mikulas

---
 drivers/md/dm-snap.c |   17 ++---------------
 drivers/md/dm-snap.h |    3 ---
 2 files changed, 2 insertions(+), 18 deletions(-)

Index: linux-2.6.25/drivers/md/dm-snap.c
===================================================================
--- linux-2.6.25.orig/drivers/md/dm-snap.c	2008-05-01 17:16:02.000000000 +0200
+++ linux-2.6.25/drivers/md/dm-snap.c	2008-05-01 17:17:41.000000000 +0200
@@ -557,7 +557,6 @@ static int snapshot_ctr(struct dm_target
 	s->type = persistent;

 	s->valid = 1;
-	s->active = 0;
 	s->last_percent = 0;
 	init_rwsem(&s->lock);
 	s->ti = ti;
@@ -616,7 +615,6 @@ static int snapshot_ctr(struct dm_target
 	}

 	/* Add snapshot to the list of snapshots for this origin */
-	/* Exceptions aren't triggered till snapshot_resume() is called */
 	if (register_snapshot(s)) {
 		r = -EINVAL;
 		ti->error = "Cannot register snapshot origin";
@@ -954,7 +952,6 @@ static int snapshot_map(struct dm_target
 	chunk = sector_to_chunk(s, bio->bi_sector);

 	/* Full snapshots are not usable */
-	/* To get here the table must be live so s->active is always set. */
 	if (!s->valid)
 		return -EIO;

@@ -1027,15 +1024,6 @@ static int snapshot_end_io(struct dm_tar
 	return 0;
 }

-static void snapshot_resume(struct dm_target *ti)
-{
-	struct dm_snapshot *s = ti->private;
-
-	down_write(&s->lock);
-	s->active = 1;
-	up_write(&s->lock);
-}
-
 static int snapshot_status(struct dm_target *ti, status_type_t type,
 			   char *result, unsigned int maxlen)
 {
@@ -1099,8 +1087,8 @@ static int __origin_write(struct list_he

 		down_write(&snap->lock);

-		/* Only deal with valid and active snapshots */
-		if (!snap->valid || !snap->active)
+		/* Only deal with valid snapshots */
+		if (!snap->valid)
 			goto next_snapshot;

 		/* Nothing to do if writing beyond end of snapshot */
@@ -1288,7 +1276,6 @@ static struct target_type snapshot_targe
 	.dtr     = snapshot_dtr,
 	.map     = snapshot_map,
 	.end_io  = snapshot_end_io,
-	.resume  = snapshot_resume,
 	.status  = snapshot_status,
 };

Index: linux-2.6.25/drivers/md/dm-snap.h
===================================================================
--- linux-2.6.25.orig/drivers/md/dm-snap.h	2008-05-01 17:15:37.000000000 +0200
+++ linux-2.6.25/drivers/md/dm-snap.h	2008-05-01 17:16:30.000000000 +0200
@@ -151,9 +151,6 @@ struct dm_snapshot {
 	/* You can't use a snapshot if this is 0 (e.g. if full) */
 	int valid;

-	/* Origin writes don't trigger exceptions until this is set */
-	int active;
-
 	/* Used for display of table */
 	char type;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]