From b34578a48459ed1bd5396631aaa4a65d6bcc7726 Mon Sep 17 00:00:00 2001 From: Ilpo Jarvinen Date: Thu, 30 Oct 2008 13:33:07 +0000 Subject: dm raid1: fix do_failures Missing braces. Commit 1f965b1943 (dm raid1: separate region_hash interface part1) broke it. Signed-off-by: Ilpo Jarvinen Signed-off-by: Alasdair G Kergon Cc: Heinz Mauelshagen diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c index 92dcc06..9d7b53e 100644 --- a/drivers/md/dm-raid1.c +++ b/drivers/md/dm-raid1.c @@ -656,9 +656,10 @@ static void do_failures(struct mirror_set *ms, struct bio_list *failures) return; if (!ms->log_failure) { - while ((bio = bio_list_pop(failures))) + while ((bio = bio_list_pop(failures))) { ms->in_sync = 0; dm_rh_mark_nosync(ms->rh, bio, bio->bi_size, 0); + } return; } -- cgit v0.10.2 From 60c856c8e2f57a3f69c505735ef66e3719ea0bd6 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Thu, 30 Oct 2008 13:33:12 +0000 Subject: dm snapshot: fix register_snapshot deadlock register_snapshot() performs a GFP_KERNEL allocation while holding _origins_lock for write, but that could write out dirty pages onto a device that attempts to acquire _origins_lock for read, resulting in deadlock. So move the allocation up before taking the lock. This path is not performance-critical, so it doesn't matter that we allocate memory and free it if we find that we won't need it. Signed-off-by: Mikulas Patocka Signed-off-by: Alasdair G Kergon diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index b2d9d1a..746603b 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -229,19 +229,21 @@ static void __insert_origin(struct origin *o) */ 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); + if (!new_o) + return -ENOMEM; + down_write(&_origins_lock); o = __lookup_origin(bdev); - if (!o) { + if (o) + kfree(new_o); + else { /* New origin */ - o = kmalloc(sizeof(*o), GFP_KERNEL); - if (!o) { - up_write(&_origins_lock); - return -ENOMEM; - } + o = new_o; /* Initialise the struct */ INIT_LIST_HEAD(&o->snapshots); -- cgit v0.10.2 From 879129d208f725267366296b631aef31409cf304 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Thu, 30 Oct 2008 13:33:16 +0000 Subject: dm snapshot: wait for chunks in destructor If there are several snapshots sharing an origin and one is removed while the origin is being written to, the snapshot's mempool may get deleted while elements are still referenced. Prior to dm-snapshot-use-per-device-mempools.patch the pending exceptions may still have been referenced after the snapshot was destroyed, but this was not a problem because the shared mempool was still there. This patch fixes the problem by tracking the number of mempool elements in use. The scenario: - You have an origin and two snapshots 1 and 2. - Someone writes to the origin. - It creates two exceptions in the snapshots, snapshot 1 will be primary exception, snapshot 2's pending_exception->primary_pe will point to the exception in snapshot 1. - The exceptions are being relocated, relocation of exception 1 finishes (but it's pending_exception is still allocated, because it is referenced by an exception from snapshot 2) - The user lvremoves snapshot 1 --- it calls just suspend (does nothing) and destructor. md->pending is zero (there is no I/O submitted to the snapshot by md layer), so it won't help us. - The destructor waits for kcopyd jobs to finish on snapshot 1 --- but there are none. - The destructor on snapshot 1 cleans up everything. - The relocation of exception on snapshot 2 finishes, it drops reference on primary_pe. This frees its primary_pe pointer. Primary_pe points to pending exception created for snapshot 1. So it frees memory into non-existing mempool. Signed-off-by: Mikulas Patocka Signed-off-by: Alasdair G Kergon diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index 746603b..6c96db2 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -370,6 +370,7 @@ static struct dm_snap_pending_exception *alloc_pending_exception(struct dm_snaps struct dm_snap_pending_exception *pe = mempool_alloc(s->pending_pool, GFP_NOIO); + atomic_inc(&s->pending_exceptions_count); pe->snap = s; return pe; @@ -377,7 +378,11 @@ static struct dm_snap_pending_exception *alloc_pending_exception(struct dm_snaps static void free_pending_exception(struct dm_snap_pending_exception *pe) { - mempool_free(pe, pe->snap->pending_pool); + struct dm_snapshot *s = pe->snap; + + mempool_free(pe, s->pending_pool); + smp_mb__before_atomic_dec(); + atomic_dec(&s->pending_exceptions_count); } static void insert_completed_exception(struct dm_snapshot *s, @@ -602,6 +607,7 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv) s->valid = 1; s->active = 0; + atomic_set(&s->pending_exceptions_count, 0); init_rwsem(&s->lock); spin_lock_init(&s->pe_lock); s->ti = ti; @@ -728,6 +734,14 @@ static void snapshot_dtr(struct dm_target *ti) /* After this returns there can be no new kcopyd jobs. */ unregister_snapshot(s); + while (atomic_read(&s->pending_exceptions_count)) + yield(); + /* + * Ensure instructions in mempool_destroy aren't reordered + * before atomic_read. + */ + smp_mb(); + #ifdef CONFIG_DM_DEBUG for (i = 0; i < DM_TRACKED_CHUNK_HASH_SIZE; i++) BUG_ON(!hlist_empty(&s->tracked_chunk_hash[i])); diff --git a/drivers/md/dm-snap.h b/drivers/md/dm-snap.h index f07315f..99c0106 100644 --- a/drivers/md/dm-snap.h +++ b/drivers/md/dm-snap.h @@ -160,6 +160,8 @@ struct dm_snapshot { mempool_t *pending_pool; + atomic_t pending_exceptions_count; + struct exception_table pending; struct exception_table complete; -- cgit v0.10.2