diff options
author | Mikulas Patocka <mpatocka@redhat.com> | 2008-07-21 07:00:34 -0400 |
---|---|---|
committer | Alasdair G Kergon <agk@redhat.com> | 2008-07-21 07:00:34 -0400 |
commit | a8d41b59f3f5a7ac19452ef442a7fc1b5fa17366 (patch) | |
tree | f9435bed2d582e4cd3e91e4d6fb18a18f62aa019 /drivers/md | |
parent | cd45daffd1f7b53aac0835b23e97f814ec3f10dc (diff) |
dm snapshot: fix race during exception creation
Fix a race condition that returns incorrect data when a write causes an
exception to be allocated whilst a read is still in flight.
The race condition happens as follows:
* A read to non-reallocated sector in the snapshot is submitted so that the
read is routed to the original device.
* A write to the original device is submitted. The write causes an exception
that reallocates the block. The write proceeds.
* The original read is dequeued and reads the wrong data.
This race can be triggered with CFQ scheduler and one thread writing and
multiple threads reading simultaneously.
(This patch relies upon the earlier dm-kcopyd-per-device.patch to avoid a
deadlock.)
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>
Diffstat (limited to 'drivers/md')
-rw-r--r-- | drivers/md/dm-snap.c | 28 |
1 files changed, 28 insertions, 0 deletions
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c index de302702ab3e..f4fd0cee9c3d 100644 --- a/drivers/md/dm-snap.c +++ b/drivers/md/dm-snap.c | |||
@@ -134,6 +134,27 @@ static void stop_tracking_chunk(struct dm_snapshot *s, | |||
134 | mempool_free(c, s->tracked_chunk_pool); | 134 | mempool_free(c, s->tracked_chunk_pool); |
135 | } | 135 | } |
136 | 136 | ||
137 | static int __chunk_is_tracked(struct dm_snapshot *s, chunk_t chunk) | ||
138 | { | ||
139 | struct dm_snap_tracked_chunk *c; | ||
140 | struct hlist_node *hn; | ||
141 | int found = 0; | ||
142 | |||
143 | spin_lock_irq(&s->tracked_chunk_lock); | ||
144 | |||
145 | hlist_for_each_entry(c, hn, | ||
146 | &s->tracked_chunk_hash[DM_TRACKED_CHUNK_HASH(chunk)], node) { | ||
147 | if (c->chunk == chunk) { | ||
148 | found = 1; | ||
149 | break; | ||
150 | } | ||
151 | } | ||
152 | |||
153 | spin_unlock_irq(&s->tracked_chunk_lock); | ||
154 | |||
155 | return found; | ||
156 | } | ||
157 | |||
137 | /* | 158 | /* |
138 | * One of these per registered origin, held in the snapshot_origins hash | 159 | * One of these per registered origin, held in the snapshot_origins hash |
139 | */ | 160 | */ |
@@ -840,6 +861,13 @@ static void pending_complete(struct dm_snap_pending_exception *pe, int success) | |||
840 | } | 861 | } |
841 | 862 | ||
842 | /* | 863 | /* |
864 | * Check for conflicting reads. This is extremely improbable, | ||
865 | * so yield() is sufficient and there is no need for a wait queue. | ||
866 | */ | ||
867 | while (__chunk_is_tracked(s, pe->e.old_chunk)) | ||
868 | yield(); | ||
869 | |||
870 | /* | ||
843 | * Add a proper exception, and remove the | 871 | * Add a proper exception, and remove the |
844 | * in-flight exception from the list. | 872 | * in-flight exception from the list. |
845 | */ | 873 | */ |