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

Re: [dm-devel] [PATCH] dm-snapshot.c: Allocate an origin structure in origin_ctr()



On Tuesday 29 July 2003 15:22, Kevin Corry wrote:
> Joe,
>
> We talked on Saturday about adding code in the origin_ctr() path to
> allocate an origin structure if one was not already present. This would
> allow an origin device to be active without any active snapshots, and thus
> prevent hitting the BUG in do_origin. You said you had a patch that did
> this, but in case you've misplaced it, I put one together this afternoon.
> If you do have your patch available, could you please post it here, and I
> will test with your version instead of this one.
>
> Thanks!

Damn.

Well, I found a bug in the patch I sent in yesterday. :(  If you create an origin
device, then create a snapshot device, then delete the snapshot device, the
struct origin gets deleted by the snapshot_dtr, which leads right back to the
BUG we're trying to avoid in do_origin().

This new version adds a referrence count to the struct origin and deallocates
it only when the count goes to zero.

(diff did something weird with this one, so the patch looks a bit strange in
places...but it still works.)

-- 
Kevin Corry
kevcorry us ibm com
http://evms.sourceforge.net/

--- a/drivers/md/dm-snapshot.c	4 Jul 2003 19:38:50 -0000
+++ b/drivers/md/dm-snapshot.c	30 Jul 2003 19:29:25 -0000
@@ -92,6 +92,9 @@
 
 	/* List of snapshots for this origin */
 	struct list_head snapshots;
+
+	/* Count of snapshots and origins referrencing this structure. */
+	unsigned int count;
 };
 
 /*
@@ -155,6 +158,36 @@
 }
 
 /*
+ * Allocate and initialize an origin structure.
+ */
+static struct origin * __alloc_origin(kdev_t dev)
+{
+	struct origin *o = kmalloc(sizeof(*o), GFP_KERNEL);
+	if (o) {
+		o->dev = dev;
+		o->count = 0;
+		INIT_LIST_HEAD(&o->hash_list);
+		INIT_LIST_HEAD(&o->snapshots);
+		__insert_origin(o);
+	}
+	return o;
+}
+
+static void __get_origin(struct origin *o)
+{
+	o->count++;
+}
+
+static void __put_origin(struct origin *o)
+{
+	o->count--;
+	if (o->count == 0) {
+		list_del(&o->hash_list);
+		kfree(o);
+	}
+}
+
+/*
  * Make a note of the snapshot and its origin so we can look it
  * up when the origin has a write on it.
  */
@@ -168,20 +201,37 @@
 
 	if (!o) {
 		/* New origin */
-		o = kmalloc(sizeof(*o), GFP_KERNEL);
+		o = __alloc_origin(dev);
 		if (!o) {
 			up_write(&_origins_lock);
 			return -ENOMEM;
 		}
+	}
 
-		/* Initialise the struct */
-		INIT_LIST_HEAD(&o->snapshots);
-		o->dev = dev;
+	__get_origin(o);
+	list_add_tail(&snap->list, &o->snapshots);
 
-		__insert_origin(o);
+	up_write(&_origins_lock);
+	return 0;
+}
+
+static int register_origin(kdev_t dev)
+{
+	struct origin *o;
+
+	down_write(&_origins_lock);
+	o = __lookup_origin(dev);
+
+	if (!o) {
+		/* New origin */
+		o = __alloc_origin(dev);
+		if (!o) {
+			up_write(&_origins_lock);
+			return -ENOMEM;
+		}
 	}
 
-	list_add_tail(&snap->list, &o->snapshots);
+	__get_origin(o);
 
 	up_write(&_origins_lock);
 	return 0;
@@ -195,11 +245,18 @@
 	o = __lookup_origin(s->origin->dev);
 
 	list_del(&s->list);
-	if (list_empty(&o->snapshots)) {
-		list_del(&o->hash_list);
-		kfree(o);
-	}
+	__put_origin(o);
+
+	up_write(&_origins_lock);
+}
+
+static void unregister_origin(kdev_t dev)
+{
+	struct origin *o;
 
+	down_write(&_origins_lock);
+	o = __lookup_origin(dev);
+	__put_origin(o);
 	up_write(&_origins_lock);
 }
 
@@ -1050,6 +1104,13 @@
 		return r;
 	}
 
+	r = register_origin(dev->dev);
+	if (r) {
+		ti->error = "Cannot register origin";
+		dm_put_device(ti, dev);
+		return r;
+	}
+
 	ti->private = dev;
 	return 0;
 }
@@ -1057,6 +1118,7 @@
 static void origin_dtr(struct dm_target *ti)
 {
 	struct dm_dev *dev = (struct dm_dev *) ti->private;
+	unregister_origin(dev->dev);
 	dm_put_device(ti, dev);
 }
 




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