From 3f8d3f5432078a558151e27230e20bcf93c23ffe Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Sat, 25 Mar 2023 17:52:01 -0400 Subject: [PATCH] dm bio prison v1: add dm_cell_key_has_valid_range Don't have bio_detain() BUG_ON if a dm_cell_key is beyond BIO_PRISON_MAX_RANGE or spans a boundary. Update dm-thin.c:build_key() to use dm_cell_key_has_valid_range() which will do this checking without using BUG_ON. Also update process_discard_bio() to check the discard bio that DM core passes in (having first imposed max_discard_granularity based splitting). dm_cell_key_has_valid_range() will merely WARN_ON_ONCE if it returns false because if it does: it is programmer error that should be caught with proper testing. So relax the BUG_ONs to be WARN_ON_ONCE. Signed-off-by: Mike Snitzer --- drivers/md/dm-bio-prison-v1.c | 14 +++++++++----- drivers/md/dm-bio-prison-v1.h | 5 +++++ drivers/md/dm-thin.c | 21 +++++++++++++++------ 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/drivers/md/dm-bio-prison-v1.c b/drivers/md/dm-bio-prison-v1.c index 2b8af86..78bb559 100644 --- a/drivers/md/dm-bio-prison-v1.c +++ b/drivers/md/dm-bio-prison-v1.c @@ -120,12 +120,17 @@ static unsigned lock_nr(struct dm_cell_key *key) return (key->block_begin >> BIO_PRISON_MAX_RANGE_SHIFT) & LOCK_MASK; } -static void check_range(struct dm_cell_key *key) +bool dm_cell_key_has_valid_range(struct dm_cell_key *key) { - BUG_ON(key->block_end - key->block_begin > BIO_PRISON_MAX_RANGE); - BUG_ON((key->block_begin >> BIO_PRISON_MAX_RANGE_SHIFT) != - ((key->block_end - 1) >> BIO_PRISON_MAX_RANGE_SHIFT)); + if (WARN_ON_ONCE(key->block_end - key->block_begin > BIO_PRISON_MAX_RANGE)) + return false; + if (WARN_ON_ONCE((key->block_begin >> BIO_PRISON_MAX_RANGE_SHIFT) != + (key->block_end - 1) >> BIO_PRISON_MAX_RANGE_SHIFT)) + return false; + + return true; } +EXPORT_SYMBOL(dm_cell_key_has_valid_range); static int __bio_detain(struct rb_root *root, struct dm_cell_key *key, @@ -172,7 +177,6 @@ static int bio_detain(struct dm_bio_prison *prison, { int r; unsigned l = lock_nr(key); - check_range(key); spin_lock_irq(&prison->regions[l].lock); r = __bio_detain(&prison->regions[l].cell, key, inmate, cell_prealloc, cell_result); diff --git a/drivers/md/dm-bio-prison-v1.h b/drivers/md/dm-bio-prison-v1.h index 0b8acd6..2a097ed 100644 --- a/drivers/md/dm-bio-prison-v1.h +++ b/drivers/md/dm-bio-prison-v1.h @@ -84,6 +84,11 @@ int dm_get_cell(struct dm_bio_prison *prison, struct dm_bio_prison_cell **cell_result); /* + * Returns false if key is beyond BIO_PRISON_MAX_RANGE or spans a boundary. + */ +bool dm_cell_key_has_valid_range(struct dm_cell_key *key); + +/* * An atomic op that combines retrieving or creating a cell, and adding a * bio to it. * diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c index 33ad569..2b13c94 100644 --- a/drivers/md/dm-thin.c +++ b/drivers/md/dm-thin.c @@ -118,25 +118,27 @@ enum lock_space { PHYSICAL }; -static void build_key(struct dm_thin_device *td, enum lock_space ls, +static bool build_key(struct dm_thin_device *td, enum lock_space ls, dm_block_t b, dm_block_t e, struct dm_cell_key *key) { key->virtual = (ls == VIRTUAL); key->dev = dm_thin_dev_id(td); key->block_begin = b; key->block_end = e; + + return dm_cell_key_has_valid_range(key); } static void build_data_key(struct dm_thin_device *td, dm_block_t b, struct dm_cell_key *key) { - build_key(td, PHYSICAL, b, b + 1llu, key); + (void) build_key(td, PHYSICAL, b, b + 1llu, key); } static void build_virtual_key(struct dm_thin_device *td, dm_block_t b, struct dm_cell_key *key) { - build_key(td, VIRTUAL, b, b + 1llu, key); + (void) build_key(td, VIRTUAL, b, b + 1llu, key); } /*----------------------------------------------------------------*/ @@ -1702,7 +1704,8 @@ static void break_up_discard_bio(struct thin_c *tc, dm_block_t begin, dm_block_t << BIO_PRISON_MAX_RANGE_SHIFT; len = min_t(sector_t, data_end - data_begin, next_boundary - data_begin); - build_key(tc->td, PHYSICAL, data_begin, data_begin + len, &data_key); + /* This key is certainly within range given the above splitting */ + (void) build_key(tc->td, PHYSICAL, data_begin, data_begin + len, &data_key); if (bio_detain(tc->pool, &data_key, NULL, &data_cell)) { /* contention, we'll give up with this range */ data_begin += len; @@ -1778,8 +1781,13 @@ static void process_discard_bio(struct thin_c *tc, struct bio *bio) return; } - build_key(tc->td, VIRTUAL, begin, end, &virt_key); - if (bio_detain(tc->pool, &virt_key, bio, &virt_cell)) + if (unlikely(!build_key(tc->td, VIRTUAL, begin, end, &virt_key))) { + DMERR_LIMIT("Discard doesn't respect bio prison limits"); + bio_endio(bio); + return; + } + + if (bio_detain(tc->pool, &virt_key, bio, &virt_cell)) { /* * Potential starvation issue: We're relying on the * fs/application being well behaved, and not trying to @@ -1788,6 +1796,7 @@ static void process_discard_bio(struct thin_c *tc, struct bio *bio) * cell will never be granted. */ return; + } tc->pool->process_discard_cell(tc, virt_cell); } -- 2.7.4