From e2460f2a4bc740fae9e23f14d653cf53e90b3f9a Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Tue, 18 Apr 2017 16:51:48 -0400 Subject: dm: mark targets that pass integrity data A dm-crypt on dm-integrity device incorrectly advertises an integrity profile on the DM crypt device. It can be seen in the files "/sys/block/dm-*/integrity/*" that both dm-integrity and dm-crypt target advertise the integrity profile. That is incorrect, only the dm-integrity target should advertise the integrity profile. A general problem in DM is that if we have a DM device that depends on another device with an integrity profile, the upper device will always advertise the integrity profile, even when the target driver doesn't support handling integrity data. Most targets don't support integrity data, so we provide a whitelist of targets that support it (linear, delay and striped). The targets that support passing integrity data to the lower device are marked with the flag DM_TARGET_PASSES_INTEGRITY. The DM core will now advertise integrity data on a DM device only if all the targets support the integrity data. Signed-off-by: Mikulas Patocka Signed-off-by: Mike Snitzer --- drivers/md/dm.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'drivers/md/dm.c') diff --git a/drivers/md/dm.c b/drivers/md/dm.c index f4ffd1eb8f44..e602ae0d5d75 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1089,8 +1089,18 @@ static int clone_bio(struct dm_target_io *tio, struct bio *bio, __bio_clone_fast(clone, bio); - if (bio_integrity(bio)) { - int r = bio_integrity_clone(clone, bio, GFP_NOIO); + if (unlikely(bio_integrity(bio) != NULL)) { + int r; + + if (unlikely(!dm_target_has_integrity(tio->ti->type) && + !dm_target_passes_integrity(tio->ti->type))) { + DMWARN("%s: the target %s doesn't support integrity data.", + dm_device_name(tio->io->md), + tio->ti->type->name); + return -EIO; + } + + r = bio_integrity_clone(clone, bio, GFP_NOIO); if (r < 0) return r; } @@ -1098,7 +1108,7 @@ static int clone_bio(struct dm_target_io *tio, struct bio *bio, bio_advance(clone, to_bytes(sector - clone->bi_iter.bi_sector)); clone->bi_iter.bi_size = to_bytes(len); - if (bio_integrity(bio)) + if (unlikely(bio_integrity(bio) != NULL)) bio_integrity_trim(clone, 0, len); return 0; -- cgit v1.2.3 From 1ea0654e46eb62acc379000be2f16350101ebf85 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 27 Apr 2017 10:11:21 -0700 Subject: dm: verify suspend_locking assumptions at runtime Ensure that the assumptions about the caller holding suspend_lock are checked at runtime if lockdep is enabled. Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Signed-off-by: Mike Snitzer --- drivers/md/dm-table.c | 4 ++++ drivers/md/dm.c | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers/md/dm.c') diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index c68757f94e16..515136f29115 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -1680,6 +1680,8 @@ static void suspend_targets(struct dm_table *t, enum suspend_mode mode) int i = t->num_targets; struct dm_target *ti = t->targets; + lockdep_assert_held(&t->md->suspend_lock); + while (i--) { switch (mode) { case PRESUSPEND: @@ -1727,6 +1729,8 @@ int dm_table_resume_targets(struct dm_table *t) { int i, r = 0; + lockdep_assert_held(&t->md->suspend_lock); + for (i = 0; i < t->num_targets; i++) { struct dm_target *ti = t->targets + i; diff --git a/drivers/md/dm.c b/drivers/md/dm.c index e602ae0d5d75..9940c9a42665 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1698,6 +1698,8 @@ static void event_callback(void *context) */ static void __set_size(struct mapped_device *md, sector_t size) { + lockdep_assert_held(&md->suspend_lock); + set_capacity(md->disk, size); i_size_write(md->bdev->bd_inode, (loff_t)size << SECTOR_SHIFT); @@ -2147,8 +2149,6 @@ static void unlock_fs(struct mapped_device *md) * If __dm_suspend returns 0, the device is completely quiescent * now. There is no request-processing activity. All new requests * are being added to md->deferred list. - * - * Caller must hold md->suspend_lock */ static int __dm_suspend(struct mapped_device *md, struct dm_table *map, unsigned suspend_flags, long task_state, @@ -2364,6 +2364,8 @@ static void __dm_internal_suspend(struct mapped_device *md, unsigned suspend_fla { struct dm_table *map = NULL; + lockdep_assert_held(&md->suspend_lock); + if (md->internal_suspend_count++) return; /* nested internal suspend */ -- cgit v1.2.3 From 7e0d574f2683a2346c978613a72ff07afc89b17a Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 27 Apr 2017 10:11:23 -0700 Subject: dm: introduce enum dm_queue_mode to cleanup related code Introduce an enumeration type for the queue mode. This patch does not change any functionality but makes the DM code easier to read. Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Signed-off-by: Mike Snitzer --- drivers/md/dm-core.h | 2 +- drivers/md/dm-ioctl.c | 2 +- drivers/md/dm-mpath.c | 5 ++++- drivers/md/dm-table.c | 14 +++++++------- drivers/md/dm.c | 11 +++++++---- drivers/md/dm.h | 8 ++++---- include/linux/device-mapper.h | 14 ++++++++------ 7 files changed, 32 insertions(+), 24 deletions(-) (limited to 'drivers/md/dm.c') diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h index 136fda3ff9e5..b92f74d9a982 100644 --- a/drivers/md/dm-core.h +++ b/drivers/md/dm-core.h @@ -47,7 +47,7 @@ struct mapped_device { struct request_queue *queue; int numa_node_id; - unsigned type; + enum dm_queue_mode type; /* Protect queue and type against concurrent access. */ struct mutex type_lock; diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c index ddda8107aa7e..2d5d7064acbf 100644 --- a/drivers/md/dm-ioctl.c +++ b/drivers/md/dm-ioctl.c @@ -1260,7 +1260,7 @@ static int populate_table(struct dm_table *table, return dm_table_complete(table); } -static bool is_valid_type(unsigned cur, unsigned new) +static bool is_valid_type(enum dm_queue_mode cur, enum dm_queue_mode new) { if (cur == new || (cur == DM_TYPE_BIO_BASED && new == DM_TYPE_DAX_BIO_BASED)) diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 730ec0be1afa..8336332bd61f 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -90,7 +90,7 @@ struct multipath { atomic_t pg_init_in_progress; /* Only one pg_init allowed at once */ atomic_t pg_init_count; /* Number of times pg_init called */ - unsigned queue_mode; + enum dm_queue_mode queue_mode; struct mutex work_mutex; struct work_struct trigger_event; @@ -1700,6 +1700,9 @@ static void multipath_status(struct dm_target *ti, status_type_t type, case DM_TYPE_MQ_REQUEST_BASED: DMEMIT("queue_mode mq "); break; + default: + WARN_ON_ONCE(true); + break; } } } diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index 515136f29115..a02a04829156 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -30,7 +30,7 @@ struct dm_table { struct mapped_device *md; - unsigned type; + enum dm_queue_mode type; /* btree table */ unsigned int depth; @@ -825,19 +825,19 @@ void dm_consume_args(struct dm_arg_set *as, unsigned num_args) } EXPORT_SYMBOL(dm_consume_args); -static bool __table_type_bio_based(unsigned table_type) +static bool __table_type_bio_based(enum dm_queue_mode table_type) { return (table_type == DM_TYPE_BIO_BASED || table_type == DM_TYPE_DAX_BIO_BASED); } -static bool __table_type_request_based(unsigned table_type) +static bool __table_type_request_based(enum dm_queue_mode table_type) { return (table_type == DM_TYPE_REQUEST_BASED || table_type == DM_TYPE_MQ_REQUEST_BASED); } -void dm_table_set_type(struct dm_table *t, unsigned type) +void dm_table_set_type(struct dm_table *t, enum dm_queue_mode type) { t->type = type; } @@ -879,7 +879,7 @@ static int dm_table_determine_type(struct dm_table *t) struct dm_target *tgt; struct dm_dev_internal *dd; struct list_head *devices = dm_table_get_devices(t); - unsigned live_md_type = dm_get_md_type(t->md); + enum dm_queue_mode live_md_type = dm_get_md_type(t->md); if (t->type != DM_TYPE_NONE) { /* target already set the table's type */ @@ -988,7 +988,7 @@ verify_rq_based: return 0; } -unsigned dm_table_get_type(struct dm_table *t) +enum dm_queue_mode dm_table_get_type(struct dm_table *t) { return t->type; } @@ -1039,7 +1039,7 @@ bool dm_table_all_blk_mq_devices(struct dm_table *t) static int dm_table_alloc_md_mempools(struct dm_table *t, struct mapped_device *md) { - unsigned type = dm_table_get_type(t); + enum dm_queue_mode type = dm_table_get_type(t); unsigned per_io_data_size = 0; struct dm_target *tgt; unsigned i; diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 9940c9a42665..45660246e8f5 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1807,13 +1807,13 @@ void dm_unlock_md_type(struct mapped_device *md) mutex_unlock(&md->type_lock); } -void dm_set_md_type(struct mapped_device *md, unsigned type) +void dm_set_md_type(struct mapped_device *md, enum dm_queue_mode type) { BUG_ON(!mutex_is_locked(&md->type_lock)); md->type = type; } -unsigned dm_get_md_type(struct mapped_device *md) +enum dm_queue_mode dm_get_md_type(struct mapped_device *md) { return md->type; } @@ -1840,7 +1840,7 @@ EXPORT_SYMBOL_GPL(dm_get_queue_limits); int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t) { int r; - unsigned type = dm_get_md_type(md); + enum dm_queue_mode type = dm_get_md_type(md); switch (type) { case DM_TYPE_REQUEST_BASED: @@ -1871,6 +1871,9 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t) if (type == DM_TYPE_DAX_BIO_BASED) queue_flag_set_unlocked(QUEUE_FLAG_DAX, md->queue); break; + case DM_TYPE_NONE: + WARN_ON_ONCE(true); + break; } return 0; @@ -2556,7 +2559,7 @@ int dm_noflush_suspending(struct dm_target *ti) } EXPORT_SYMBOL_GPL(dm_noflush_suspending); -struct dm_md_mempools *dm_alloc_md_mempools(struct mapped_device *md, unsigned type, +struct dm_md_mempools *dm_alloc_md_mempools(struct mapped_device *md, enum dm_queue_mode type, unsigned integrity, unsigned per_io_data_size) { struct dm_md_mempools *pools = kzalloc_node(sizeof(*pools), GFP_KERNEL, md->numa_node_id); diff --git a/drivers/md/dm.h b/drivers/md/dm.h index f298b01f7ab3..38c84c0a35d4 100644 --- a/drivers/md/dm.h +++ b/drivers/md/dm.h @@ -64,7 +64,7 @@ void dm_table_presuspend_undo_targets(struct dm_table *t); void dm_table_postsuspend_targets(struct dm_table *t); int dm_table_resume_targets(struct dm_table *t); int dm_table_any_congested(struct dm_table *t, int bdi_bits); -unsigned dm_table_get_type(struct dm_table *t); +enum dm_queue_mode dm_table_get_type(struct dm_table *t); struct target_type *dm_table_get_immutable_target_type(struct dm_table *t); struct dm_target *dm_table_get_immutable_target(struct dm_table *t); struct dm_target *dm_table_get_wildcard_target(struct dm_table *t); @@ -76,8 +76,8 @@ 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, unsigned type); -unsigned dm_get_md_type(struct mapped_device *md); +void dm_set_md_type(struct mapped_device *md, enum dm_queue_mode type); +enum dm_queue_mode dm_get_md_type(struct mapped_device *md); struct target_type *dm_get_immutable_target_type(struct mapped_device *md); int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t); @@ -204,7 +204,7 @@ void dm_kcopyd_exit(void); /* * Mempool operations */ -struct dm_md_mempools *dm_alloc_md_mempools(struct mapped_device *md, unsigned type, +struct dm_md_mempools *dm_alloc_md_mempools(struct mapped_device *md, enum dm_queue_mode type, unsigned integrity, unsigned per_bio_data_size); void dm_free_md_mempools(struct dm_md_mempools *pools); diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h index 98f981026e4e..1ce4036224eb 100644 --- a/include/linux/device-mapper.h +++ b/include/linux/device-mapper.h @@ -22,11 +22,13 @@ struct bio_vec; /* * Type of table, mapped_device's mempool and request_queue */ -#define DM_TYPE_NONE 0 -#define DM_TYPE_BIO_BASED 1 -#define DM_TYPE_REQUEST_BASED 2 -#define DM_TYPE_MQ_REQUEST_BASED 3 -#define DM_TYPE_DAX_BIO_BASED 4 +enum dm_queue_mode { + DM_TYPE_NONE = 0, + DM_TYPE_BIO_BASED = 1, + DM_TYPE_REQUEST_BASED = 2, + DM_TYPE_MQ_REQUEST_BASED = 3, + DM_TYPE_DAX_BIO_BASED = 4, +}; typedef enum { STATUSTYPE_INFO, STATUSTYPE_TABLE } status_type_t; @@ -476,7 +478,7 @@ void dm_table_add_target_callbacks(struct dm_table *t, struct dm_target_callback * Useful for "hybrid" target (supports both bio-based * and request-based). */ -void dm_table_set_type(struct dm_table *t, unsigned type); +void dm_table_set_type(struct dm_table *t, enum dm_queue_mode type); /* * Finally call this to make the table ready for use. -- cgit v1.2.3 From 86331f39a5935b092d3ea59446d416563ed05d16 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 27 Apr 2017 10:11:26 -0700 Subject: dm mpath: make it easier to detect unintended I/O request flushes I/O errors triggered by multipathd incorrectly not enabling the no-flush flag for DM_DEVICE_SUSPEND or DM_DEVICE_RESUME are hard to debug. Add more logging to make it easier to debug this. Signed-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Signed-off-by: Mike Snitzer --- drivers/md/dm-mpath.c | 25 +++++++++++++++++++++---- drivers/md/dm.c | 2 ++ 2 files changed, 23 insertions(+), 4 deletions(-) (limited to 'drivers/md/dm.c') diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index cc529537c8ce..fd7cdc4ce2a5 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -441,6 +441,23 @@ failed: return NULL; } +/* + * dm_report_EIO() is a macro instead of a function to make pr_debug() + * report the function name and line number of the function from which + * it has been invoked. + */ +#define dm_report_EIO(m) \ +({ \ + struct mapped_device *md = dm_table_get_md((m)->ti->table); \ + \ + pr_debug("%s: returning EIO; QIFNP = %d; SQIFNP = %d; DNFS = %d\n", \ + dm_device_name(md), \ + test_bit(MPATHF_QUEUE_IF_NO_PATH, &(m)->flags), \ + test_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &(m)->flags), \ + dm_noflush_suspending((m)->ti)); \ + -EIO; \ +}) + /* * Map cloned requests (request-based multipath) */ @@ -464,7 +481,7 @@ static int multipath_clone_and_map(struct dm_target *ti, struct request *rq, if (!pgpath) { if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) return DM_MAPIO_DELAY_REQUEUE; - return -EIO; /* Failed */ + return dm_report_EIO(m); /* Failed */ } else if (test_bit(MPATHF_QUEUE_IO, &m->flags) || test_bit(MPATHF_PG_INIT_REQUIRED, &m->flags)) { if (pg_init_all_paths(m)) @@ -541,7 +558,7 @@ static int __multipath_map_bio(struct multipath *m, struct bio *bio, struct dm_m if (!pgpath) { if (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) return DM_MAPIO_REQUEUE; - return -EIO; + return dm_report_EIO(m); } mpio->pgpath = pgpath; @@ -1476,7 +1493,7 @@ static int do_end_io(struct multipath *m, struct request *clone, if (atomic_read(&m->nr_valid_paths) == 0 && !test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) - r = -EIO; + r = dm_report_EIO(m); return r; } @@ -1519,7 +1536,7 @@ static int do_end_io_bio(struct multipath *m, struct bio *clone, if (atomic_read(&m->nr_valid_paths) == 0 && !test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) - return -EIO; + return dm_report_EIO(m); /* Queue for the daemon to resubmit */ dm_bio_restore(get_bio_details_from_bio(clone), clone); diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 45660246e8f5..dbfaf6dde657 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -2169,6 +2169,8 @@ static int __dm_suspend(struct mapped_device *md, struct dm_table *map, */ if (noflush) set_bit(DMF_NOFLUSH_SUSPENDING, &md->flags); + else + pr_debug("%s: suspending with flush\n", dm_device_name(md)); /* * This gets reverted if there's an error later and the targets -- cgit v1.2.3