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@xxxxxxxxxx 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@xxxxxxxxxx> --- 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; }