From: Mikulas Patocka <mpatocka@redhat.com> Date: Wed, 5 Nov 2008 03:17:14 -0500 Subject: [md] crash in device mapper if the user removes snapshot Message-id: Pine.LNX.4.64.0811050314580.7557@hs20-bc2-1.build.redhat.com O-Subject: Re: [RHEL 5.3 bz468473] Crash in device mapper if the user removes snapshot Bugzilla: 468473 RH-Acked-by: Alasdair G Kergon <agk@redhat.com> RH-Acked-by: Milan Broz <mbroz@redhat.com> Hi This patch fixes a crash in device mapper when the user removes snapshot while there is pending exception for another snapshot. The bug was introduced in 5.3 with per-snapshot-mempool patch and should be fixed before release. RH Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=468473 (please set bugzilla flags to ack it) Upstream commit: 879129d208f725267366296b631aef31409cf304 (2.6.28-rc3) Testing: I created testcase for this bug: http://people.redhat.com/mpatocka/testcases/remove-used-snapshot.sh Before the patch, RHEL5.3 kernel crashes in less than a minute on my computer when running this script, after the patch, the kernel dosn't crash at all. I checked that Module.symvers is the same before and after the patch. Mikulas diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index 19b2c6c..121ca19 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c @@ -365,6 +365,7 @@ static struct pending_exception *alloc_pending_exception(struct dm_snapshot *s) { struct pending_exception *pe = mempool_alloc(s->pending_pool, GFP_NOIO); + atomic_inc(&s->pending_exceptions_count); pe->snap = s; return pe; @@ -372,7 +373,11 @@ static struct pending_exception *alloc_pending_exception(struct dm_snapshot *s) static void free_pending_exception(struct 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, @@ -609,6 +614,7 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv) s->valid = 1; s->active = 0; s->last_percent = 0; + atomic_set(&s->pending_exceptions_count, 0); init_rwsem(&s->lock); spin_lock_init(&s->pe_lock); s->table = ti->table; @@ -724,6 +730,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 c16a111..910f9e9 100644 --- a/drivers/md/dm-snap.h +++ b/drivers/md/dm-snap.h @@ -166,6 +166,8 @@ struct dm_snapshot { mempool_t *pending_pool; + atomic_t pending_exceptions_count; + struct exception_table pending; struct exception_table complete;