Commit d110fd51 authored by Mikulas Patocka's avatar Mikulas Patocka Committed by Greg Kroah-Hartman

dm snapshot: avoid snapshot space leak on crash

commit 230c83af upstream.

There is a possible leak of snapshot space in case of crash.

The reason for space leaking is that chunks in the snapshot device are
allocated sequentially, but they are finished (and stored in the metadata)
out of order, depending on the order in which copying finished.

For example, supposed that the metadata contains the following records
SUPERBLOCK
METADATA (blocks 0 ... 250)
DATA 0
DATA 1
DATA 2
...
DATA 250

Now suppose that you allocate 10 new data blocks 251-260. Suppose that
copying of these blocks finish out of order (block 260 finished first
and the block 251 finished last). Now, the snapshot device looks like
this:
SUPERBLOCK
METADATA (blocks 0 ... 250, 260, 259, 258, 257, 256)
DATA 0
DATA 1
DATA 2
...
DATA 250
DATA 251
DATA 252
DATA 253
DATA 254
DATA 255
METADATA (blocks 255, 254, 253, 252, 251)
DATA 256
DATA 257
DATA 258
DATA 259
DATA 260

Now, if the machine crashes after writing the first metadata block but
before writing the second metadata block, the space for areas DATA 250-255
is leaked, it contains no valid data and it will never be used in the
future.

This patch makes dm-snapshot complete exceptions in the same order they
were allocated, thus fixing this bug.

Note: when backporting this patch to the stable kernel, change the version
field in the following way:
* if version in the stable kernel is {1, 11, 1}, change it to {1, 12, 0}
* if version in the stable kernel is {1, 10, 0} or {1, 10, 1}, change it
  to {1, 10, 2}
Userspace reads the version to determine if the bug was fixed, so the
version change is needed.
Signed-off-by: default avatarMikulas Patocka <mpatocka@redhat.com>
Signed-off-by: default avatarMike Snitzer <snitzer@redhat.com>
[xr: Backported to 3.4: adjust version]
Signed-off-by: default avatarRui Xiang <rui.xiang@huawei.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 4834ca94
...@@ -66,6 +66,18 @@ struct dm_snapshot { ...@@ -66,6 +66,18 @@ struct dm_snapshot {
atomic_t pending_exceptions_count; atomic_t pending_exceptions_count;
/* Protected by "lock" */
sector_t exception_start_sequence;
/* Protected by kcopyd single-threaded callback */
sector_t exception_complete_sequence;
/*
* A list of pending exceptions that completed out of order.
* Protected by kcopyd single-threaded callback.
*/
struct list_head out_of_order_list;
mempool_t *pending_pool; mempool_t *pending_pool;
struct dm_exception_table pending; struct dm_exception_table pending;
...@@ -171,6 +183,14 @@ struct dm_snap_pending_exception { ...@@ -171,6 +183,14 @@ struct dm_snap_pending_exception {
*/ */
int started; int started;
/* There was copying error. */
int copy_error;
/* A sequence number, it is used for in-order completion. */
sector_t exception_sequence;
struct list_head out_of_order_entry;
/* /*
* For writing a complete chunk, bypassing the copy. * For writing a complete chunk, bypassing the copy.
*/ */
...@@ -1090,6 +1110,9 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv) ...@@ -1090,6 +1110,9 @@ static int snapshot_ctr(struct dm_target *ti, unsigned int argc, char **argv)
s->valid = 1; s->valid = 1;
s->active = 0; s->active = 0;
atomic_set(&s->pending_exceptions_count, 0); atomic_set(&s->pending_exceptions_count, 0);
s->exception_start_sequence = 0;
s->exception_complete_sequence = 0;
INIT_LIST_HEAD(&s->out_of_order_list);
init_rwsem(&s->lock); init_rwsem(&s->lock);
INIT_LIST_HEAD(&s->list); INIT_LIST_HEAD(&s->list);
spin_lock_init(&s->pe_lock); spin_lock_init(&s->pe_lock);
...@@ -1448,6 +1471,19 @@ static void commit_callback(void *context, int success) ...@@ -1448,6 +1471,19 @@ static void commit_callback(void *context, int success)
pending_complete(pe, success); pending_complete(pe, success);
} }
static void complete_exception(struct dm_snap_pending_exception *pe)
{
struct dm_snapshot *s = pe->snap;
if (unlikely(pe->copy_error))
pending_complete(pe, 0);
else
/* Update the metadata if we are persistent */
s->store->type->commit_exception(s->store, &pe->e,
commit_callback, pe);
}
/* /*
* Called when the copy I/O has finished. kcopyd actually runs * Called when the copy I/O has finished. kcopyd actually runs
* this code so don't block. * this code so don't block.
...@@ -1457,13 +1493,32 @@ static void copy_callback(int read_err, unsigned long write_err, void *context) ...@@ -1457,13 +1493,32 @@ static void copy_callback(int read_err, unsigned long write_err, void *context)
struct dm_snap_pending_exception *pe = context; struct dm_snap_pending_exception *pe = context;
struct dm_snapshot *s = pe->snap; struct dm_snapshot *s = pe->snap;
if (read_err || write_err) pe->copy_error = read_err || write_err;
pending_complete(pe, 0);
else if (pe->exception_sequence == s->exception_complete_sequence) {
/* Update the metadata if we are persistent */ s->exception_complete_sequence++;
s->store->type->commit_exception(s->store, &pe->e, complete_exception(pe);
commit_callback, pe);
while (!list_empty(&s->out_of_order_list)) {
pe = list_entry(s->out_of_order_list.next,
struct dm_snap_pending_exception, out_of_order_entry);
if (pe->exception_sequence != s->exception_complete_sequence)
break;
s->exception_complete_sequence++;
list_del(&pe->out_of_order_entry);
complete_exception(pe);
}
} else {
struct list_head *lh;
struct dm_snap_pending_exception *pe2;
list_for_each_prev(lh, &s->out_of_order_list) {
pe2 = list_entry(lh, struct dm_snap_pending_exception, out_of_order_entry);
if (pe2->exception_sequence < pe->exception_sequence)
break;
}
list_add(&pe->out_of_order_entry, lh);
}
} }
/* /*
...@@ -1558,6 +1613,8 @@ __find_pending_exception(struct dm_snapshot *s, ...@@ -1558,6 +1613,8 @@ __find_pending_exception(struct dm_snapshot *s,
return NULL; return NULL;
} }
pe->exception_sequence = s->exception_start_sequence++;
dm_insert_exception(&s->pending, &pe->e); dm_insert_exception(&s->pending, &pe->e);
return pe; return pe;
...@@ -2200,7 +2257,7 @@ static struct target_type origin_target = { ...@@ -2200,7 +2257,7 @@ static struct target_type origin_target = {
static struct target_type snapshot_target = { static struct target_type snapshot_target = {
.name = "snapshot", .name = "snapshot",
.version = {1, 10, 0}, .version = {1, 10, 2},
.module = THIS_MODULE, .module = THIS_MODULE,
.ctr = snapshot_ctr, .ctr = snapshot_ctr,
.dtr = snapshot_dtr, .dtr = snapshot_dtr,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment