Skip to content

Commit

Permalink
dm snapshot: suspend origin when doing exception handover
Browse files Browse the repository at this point in the history 
In the function snapshot_resume we perform exception store handover.  If
there is another active snapshot target, the exception store is moved
from this target to the target that is being resumed.

The problem is that if there is some pending exception, it will point to
an incorrect exception store after that handover, causing a crash due to
dm-snap-persistent.c:get_exception()'s BUG_ON.

This bug can be triggered by repeatedly changing snapshot permissions
with "lvchange -p r" and "lvchange -p rw" while there are writes on the
associated origin device.

To fix this bug, we must suspend the origin device when doing the
exception store handover to make sure that there are no pending
exceptions:
- introduce _origin_hash that keeps track of dm_origin structures.
- introduce functions __lookup_dm_origin, __insert_dm_origin and
  __remove_dm_origin that manipulate the origin hash.
- modify snapshot_resume so that it calls dm_internal_suspend_fast() and
  dm_internal_resume_fast() on the origin device.

NOTE to stable@ people:

When backporting to kernels 3.12-3.18, use dm_internal_suspend and
dm_internal_resume instead of dm_internal_suspend_fast and
dm_internal_resume_fast.

When backporting to kernels older than 3.12, you need to pick functions
dm_internal_suspend and dm_internal_resume from the commit
fd2ed4d.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: stable@vger.kernel.org
  • Loading branch information
@snitm
Mikulas Patocka authored and snitm committed Feb 27, 2015
1 parent ab7c7bb commit b735fed
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 9 deletions.
93 changes: 84 additions & 9 deletions drivers/md/dm-snap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include <linux/log2.h>
#include <linux/dm-kcopyd.h>

#include "dm.h"

#include "dm-exception-store.h"

#define DM_MSG_PREFIX "snapshots"
Expand Down Expand Up @@ -290,13 +292,24 @@ struct origin {
struct list_head snapshots;
};

/*
* This structure is allocated for each origin target
*/
struct dm_origin {
struct dm_dev *dev;
struct dm_target *ti;
unsigned split_boundary;
struct list_head hash_list;
};

/*
* Size of the hash table for origin volumes. If we make this
* the size of the minors list then it should be nearly perfect
*/
#define ORIGIN_HASH_SIZE 256
#define ORIGIN_MASK 0xFF
static struct list_head *_origins;
static struct list_head *_dm_origins;
static struct rw_semaphore _origins_lock;

static DECLARE_WAIT_QUEUE_HEAD(_pending_exceptions_done);
Expand All @@ -310,12 +323,22 @@ static int init_origin_hash(void)
_origins = kmalloc(ORIGIN_HASH_SIZE * sizeof(struct list_head),
GFP_KERNEL);
if (!_origins) {
DMERR("unable to allocate memory");
DMERR("unable to allocate memory for _origins");
return -ENOMEM;
}

for (i = 0; i < ORIGIN_HASH_SIZE; i++)
INIT_LIST_HEAD(_origins + i);

_dm_origins = kmalloc(ORIGIN_HASH_SIZE * sizeof(struct list_head),
GFP_KERNEL);
if (!_dm_origins) {
DMERR("unable to allocate memory for _dm_origins");
kfree(_origins);
return -ENOMEM;
}
for (i = 0; i < ORIGIN_HASH_SIZE; i++)
INIT_LIST_HEAD(_dm_origins + i);

init_rwsem(&_origins_lock);

return 0;
Expand All @@ -324,6 +347,7 @@ static int init_origin_hash(void)
static void exit_origin_hash(void)
{
kfree(_origins);
kfree(_dm_origins);
}

static unsigned origin_hash(struct block_device *bdev)
Expand All @@ -350,6 +374,30 @@ static void __insert_origin(struct origin *o)
list_add_tail(&o->hash_list, sl);
}

static struct dm_origin *__lookup_dm_origin(struct block_device *origin)
{
struct list_head *ol;
struct dm_origin *o;

ol = &_dm_origins[origin_hash(origin)];
list_for_each_entry (o, ol, hash_list)
if (bdev_equal(o->dev->bdev, origin))
return o;

return NULL;
}

static void __insert_dm_origin(struct dm_origin *o)
{
struct list_head *sl = &_dm_origins[origin_hash(o->dev->bdev)];
list_add_tail(&o->hash_list, sl);
}

static void __remove_dm_origin(struct dm_origin *o)
{
list_del(&o->hash_list);
}

/*
* _origins_lock must be held when calling this function.
* Returns number of snapshots registered using the supplied cow device, plus:
Expand Down Expand Up @@ -1841,8 +1889,20 @@ static void snapshot_resume(struct dm_target *ti)
{
struct dm_snapshot *s = ti->private;
struct dm_snapshot *snap_src = NULL, *snap_dest = NULL;
struct dm_origin *o;
struct mapped_device *origin_md = NULL;

down_read(&_origins_lock);

o = __lookup_dm_origin(s->origin->bdev);
if (o)
origin_md = dm_table_get_md(o->ti->table);
if (origin_md == dm_table_get_md(ti->table))
origin_md = NULL;

if (origin_md)
dm_internal_suspend_fast(origin_md);

(void) __find_snapshots_sharing_cow(s, &snap_src, &snap_dest, NULL);
if (snap_src && snap_dest) {
down_write(&snap_src->lock);
Expand All @@ -1851,6 +1911,10 @@ static void snapshot_resume(struct dm_target *ti)
up_write(&snap_dest->lock);
up_write(&snap_src->lock);
}

if (origin_md)
dm_internal_resume_fast(origin_md);

up_read(&_origins_lock);

/* Now we have correct chunk size, reregister */
Expand Down Expand Up @@ -2133,11 +2197,6 @@ static int origin_write_extent(struct dm_snapshot *merging_snap,
* Origin: maps a linear range of a device, with hooks for snapshotting.
*/

struct dm_origin {
struct dm_dev *dev;
unsigned split_boundary;
};

/*
* Construct an origin mapping: <dev_path>
* The context for an origin is merely a 'struct dm_dev *'
Expand Down Expand Up @@ -2166,6 +2225,7 @@ static int origin_ctr(struct dm_target *ti, unsigned int argc, char **argv)
goto bad_open;
}

o->ti = ti;
ti->private = o;
ti->num_flush_bios = 1;

Expand All @@ -2180,6 +2240,7 @@ static int origin_ctr(struct dm_target *ti, unsigned int argc, char **argv)
static void origin_dtr(struct dm_target *ti)
{
struct dm_origin *o = ti->private;

dm_put_device(ti, o->dev);
kfree(o);
}
Expand Down Expand Up @@ -2216,6 +2277,19 @@ static void origin_resume(struct dm_target *ti)
struct dm_origin *o = ti->private;

o->split_boundary = get_origin_minimum_chunksize(o->dev->bdev);

down_write(&_origins_lock);
__insert_dm_origin(o);
up_write(&_origins_lock);
}

static void origin_postsuspend(struct dm_target *ti)
{
struct dm_origin *o = ti->private;

down_write(&_origins_lock);
__remove_dm_origin(o);
up_write(&_origins_lock);
}

static void origin_status(struct dm_target *ti, status_type_t type,
Expand Down Expand Up @@ -2258,20 +2332,21 @@ static int origin_iterate_devices(struct dm_target *ti,

static struct target_type origin_target = {
.name = "snapshot-origin",
.version = {1, 8, 1},
.version = {1, 9, 0},
.module = THIS_MODULE,
.ctr = origin_ctr,
.dtr = origin_dtr,
.map = origin_map,
.resume = origin_resume,
.postsuspend = origin_postsuspend,
.status = origin_status,
.merge = origin_merge,
.iterate_devices = origin_iterate_devices,
};

static struct target_type snapshot_target = {
.name = "snapshot",
.version = {1, 12, 0},
.version = {1, 13, 0},
.module = THIS_MODULE,
.ctr = snapshot_ctr,
.dtr = snapshot_dtr,
Expand Down
2 changes: 2 additions & 0 deletions drivers/md/dm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3121,6 +3121,7 @@ void dm_internal_suspend_fast(struct mapped_device *md)
flush_workqueue(md->wq);
dm_wait_for_completion(md, TASK_UNINTERRUPTIBLE);
}
EXPORT_SYMBOL_GPL(dm_internal_suspend_fast);

void dm_internal_resume_fast(struct mapped_device *md)
{
Expand All @@ -3132,6 +3133,7 @@ void dm_internal_resume_fast(struct mapped_device *md)
done:
mutex_unlock(&md->suspend_lock);
}
EXPORT_SYMBOL_GPL(dm_internal_resume_fast);

/*-----------------------------------------------------------------
* Event notification.
Expand Down

0 comments on commit b735fed

Please sign in to comment.