Before table_load() stages an inactive table atomically check if its type conflicts with a previously established device type (md->type). Introduce 'type_lock' in mapped_device structure and use it to protect md->type access. table_load() sets md->type without concern for: - another table_load() racing to set conflicting md->type. - do_resume() making a conflicting table live. Allowed transitions: UNKNOWN_MD_TYPE => BIO_BASED_MD_TYPE UNKNOWN_MD_TYPE => REQUEST_BASED_MD_TYPE This prevents a table reload from switching the inactive table directly to a conflicting type (even if a table hasn't yet been made live via resume). Switching table type is still possible, before resume, if an explicit table clear is performed. Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> --- drivers/md/dm-ioctl.c | 43 ++++++++++++++++++++++++++++ drivers/md/dm.c | 76 +++++++++++++++++++++++++++++++++++++++++++++----- drivers/md/dm.h | 7 ++++ 3 files changed, 119 insertions(+), 7 deletions(-) Index: linux-2.6/drivers/md/dm-ioctl.c =================================================================== --- linux-2.6.orig/drivers/md/dm-ioctl.c +++ linux-2.6/drivers/md/dm-ioctl.c @@ -1153,6 +1153,7 @@ static int table_load(struct dm_ioctl *p struct hash_cell *hc; struct dm_table *t; struct mapped_device *md; + int initial_table_load = 0; md = find_device(param); if (!md) @@ -1183,12 +1184,43 @@ static int table_load(struct dm_ioctl *p goto out; } + /* + * Protect md->type against concurrent table loads. + * Locking strategy: + * + Leverage fact that md's type cannot change after initial table load. + * - Only protect type in table_load() -- not in do_resume(). + * + * + Protect type while working to stage an inactive table: + * - check if table's type conflicts with md->type + * (holding: md->type_lock) + * - stage inactive table (hc->new_map) + * (holding: md->type_lock + _hash_lock) + */ + dm_lock_md_type(md); + + if (dm_unknown_md_type(md)) { + /* set md's type based on table's type */ + dm_set_md_type(md, t); + /* note initial_table_load to clear md's type on error */ + initial_table_load = 1; + } else if (!dm_md_type_matches_table(md, t)) { + DMWARN("can't change device type after initial table load."); + dm_table_destroy(t); + dm_unlock_md_type(md); + r = -EINVAL; + goto out; + } + + /* stage inactive table */ down_write(&_hash_lock); hc = dm_get_mdptr(md); if (!hc || hc->md != md) { DMWARN("device has been removed from the dev hash table."); dm_table_destroy(t); up_write(&_hash_lock); + if (initial_table_load) + dm_clear_md_type(md); + dm_unlock_md_type(md); r = -ENXIO; goto out; } @@ -1198,6 +1230,8 @@ static int table_load(struct dm_ioctl *p hc->new_map = t; up_write(&_hash_lock); + dm_unlock_md_type(md); + param->flags |= DM_INACTIVE_PRESENT_FLAG; r = __dev_status(md, param); @@ -1212,6 +1246,7 @@ static int table_clear(struct dm_ioctl * int r; struct hash_cell *hc; struct mapped_device *md; + struct dm_table *live_table; down_write(&_hash_lock); @@ -1226,6 +1261,7 @@ static int table_clear(struct dm_ioctl * up_write(&_hash_lock); dm_lock_resume(md); + dm_lock_md_type(md); /* May need to clear md's type */ down_write(&_hash_lock); hc = dm_get_mdptr(md); @@ -1236,6 +1272,12 @@ static int table_clear(struct dm_ioctl * goto out; } + /* Clear md's type if there is no live table */ + live_table = dm_get_live_table(md); + if (!live_table) + dm_clear_md_type(md); + dm_table_put(live_table); + if (hc->new_map) { dm_table_destroy(hc->new_map); hc->new_map = NULL; @@ -1246,6 +1288,7 @@ static int table_clear(struct dm_ioctl * r = __dev_status(hc->md, param); up_write(&_hash_lock); out: + dm_unlock_md_type(md); dm_unlock_resume(md); dm_put(md); return r; Index: linux-2.6/drivers/md/dm.c =================================================================== --- linux-2.6.orig/drivers/md/dm.c +++ linux-2.6/drivers/md/dm.c @@ -111,6 +111,15 @@ EXPORT_SYMBOL_GPL(dm_get_rq_mapinfo); #define DMF_QUEUE_IO_TO_THREAD 6 /* + * Type for md->type field. + */ +enum mapped_device_type { + UNKNOWN_MD_TYPE, + BIO_BASED_MD_TYPE, + REQUEST_BASED_MD_TYPE, +}; + +/* * Work processed by per-device workqueue. */ struct mapped_device { @@ -129,6 +138,12 @@ struct mapped_device { unsigned long flags; struct request_queue *queue; + enum mapped_device_type type; + /* + * Protect type from concurrent access. + */ + struct mutex type_lock; + struct gendisk *disk; char name[16]; @@ -1880,9 +1895,11 @@ static struct mapped_device *alloc_dev(i if (r < 0) goto bad_minor; + md->type = UNKNOWN_MD_TYPE; init_rwsem(&md->io_lock); mutex_init(&md->suspend_lock); mutex_init(&md->resume_lock); + mutex_init(&md->type_lock); spin_lock_init(&md->deferred_lock); spin_lock_init(&md->barrier_error_lock); rwlock_init(&md->map_lock); @@ -2145,6 +2162,58 @@ int dm_create(int minor, struct mapped_d return 0; } +/* + * Functions to manage md->type. + * All are required to hold md->type_lock. + */ +void dm_lock_md_type(struct mapped_device *md) +{ + mutex_lock(&md->type_lock); +} + +void dm_unlock_md_type(struct mapped_device *md) +{ + mutex_unlock(&md->type_lock); +} + +void dm_set_md_type(struct mapped_device *md, struct dm_table* t) +{ + if (dm_table_request_based(t)) + md->type = REQUEST_BASED_MD_TYPE; + else + md->type = BIO_BASED_MD_TYPE; +} + +void dm_clear_md_type(struct mapped_device *md) +{ + md->type = UNKNOWN_MD_TYPE; +} + +bool dm_unknown_md_type(struct mapped_device *md) +{ + return md->type == UNKNOWN_MD_TYPE; +} + +static bool dm_bio_based_md_type(struct mapped_device *md) +{ + return md->type == BIO_BASED_MD_TYPE; +} + +static bool dm_request_based_md_type(struct mapped_device *md) +{ + return md->type == REQUEST_BASED_MD_TYPE; +} + +bool dm_md_type_matches_table(struct mapped_device *md, struct dm_table* t) +{ + if (dm_request_based_md_type(md)) + return dm_table_request_based(t); + else if (dm_bio_based_md_type(md)) + return !dm_table_request_based(t); + + return 0; +} + static struct mapped_device *dm_find_md(dev_t dev) { struct mapped_device *md; @@ -2420,13 +2489,6 @@ struct dm_table *dm_swap_table(struct ma goto out; } - /* cannot change the device type, once a table is bound */ - if (md->map && - (dm_table_get_type(md->map) != dm_table_get_type(table))) { - DMWARN("can't change the device type after a table is bound"); - goto out; - } - map = __bind(md, table, &limits); out: Index: linux-2.6/drivers/md/dm.h =================================================================== --- linux-2.6.orig/drivers/md/dm.h +++ linux-2.6/drivers/md/dm.h @@ -66,6 +66,13 @@ int dm_table_alloc_md_mempools(struct dm void dm_table_free_md_mempools(struct dm_table *t); struct dm_md_mempools *dm_table_get_md_mempools(struct dm_table *t); +void dm_lock_md_type(struct mapped_device *md); +void dm_unlock_md_type(struct mapped_device *md); +void dm_set_md_type(struct mapped_device *md, struct dm_table* t); +void dm_clear_md_type(struct mapped_device *md); +bool dm_unknown_md_type(struct mapped_device *md); +bool dm_md_type_matches_table(struct mapped_device *md, struct dm_table* t); + void dm_lock_resume(struct mapped_device *md); void dm_unlock_resume(struct mapped_device *md); -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel