[PATCH] Re: [dm-devel] kmalloc after down_write?

[Kevin Corry]

On Friday 19 November 2004 12:36 pm, Ming Zhang wrote:
> yes, i know. semaphore can sleep. but why not release lock, kmalloc, and
> regain lock? i think any code should reduce the chance that make others
> wait for it. especially it hold a write lock, not a read lock.

Releasing the lock and regaining it means having to search the hash table
all over again to make sure another process hasn't added a new entry for
the same origin. Very wasteful. If we're simply concerned about the amount
of time that we're holding that semaphore, the following patch would do it.

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



In register_snapshot(), move the kmalloc() outside the _origins_lock. If we
later find that the origin already exists, we can simply free the memory.
This reduces the amount of time spent holding the semaphore.

Signed-off-by: Kevin Corry <kevcorry at us.ibm.com>

--- diff/drivers/md/dm-snap.c	2004-11-19 12:56:35.730830080 -0600
+++ source/drivers/md/dm-snap.c	2004-11-19 14:11:59.402127512 -0600
@@ -148,15 +148,17 @@
  */
 static int register_snapshot(struct dm_snapshot *snap)
 {
-	struct origin *o;
+	struct origin *o, *new_o;
 	struct block_device *bdev = snap->origin->bdev;
 
+	new_o = kmalloc(sizeof(*new_o), GFP_KERNEL);
+
 	down_write(&_origins_lock);
 	o = __lookup_origin(bdev);
 
 	if (!o) {
 		/* New origin */
-		o = kmalloc(sizeof(*o), GFP_KERNEL);
+		o = new_o;
 		if (!o) {
 			up_write(&_origins_lock);
 			return -ENOMEM;
@@ -172,6 +174,11 @@
 	list_add_tail(&snap->list, &o->snapshots);
 
 	up_write(&_origins_lock);
+
+	if (o != new_o) {
+		kfree(new_o);
+	}
+
 	return 0;
 }