From: Mike Snitzer <snitzer@kernel.org>
To: dm-devel@redhat.com
Cc: axboe@kernel.dk, ebiggers@kernel.org, keescook@chromium.org,
heinzm@redhat.com, Mike Snitzer <snitzer@kernel.org>,
nhuck@google.com, linux-block@vger.kernel.org, ejt@redhat.com,
mpatocka@redhat.com, luomeng12@huawei.com
Subject: [dm-devel] [dm-6.4 PATCH v3 15/20] dm bio prison v1: add dm_cell_key_has_valid_range
Date: Mon, 27 Mar 2023 16:11:38 -0400 [thread overview]
Message-ID: <20230327201143.51026-16-snitzer@kernel.org> (raw)
In-Reply-To: <20230327201143.51026-1-snitzer@kernel.org>
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 <snitzer@kernel.org>
---
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 2b8af861e5f6..78bb559b521c 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 0b8acd6708fb..2a097ed0d85e 100644
--- a/drivers/md/dm-bio-prison-v1.h
+++ b/drivers/md/dm-bio-prison-v1.h
@@ -83,6 +83,11 @@ int dm_get_cell(struct dm_bio_prison *prison,
struct dm_bio_prison_cell *cell_prealloc,
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 33ad5695f959..2b13c949bd72 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.40.0
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2023-03-27 20:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-27 20:11 [dm-devel] [dm-6.4 PATCH v3 00/20] dm bufio, thin: improve concurrent IO performance Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 01/20] dm bufio: remove unused dm_bufio_release_move interface Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 02/20] dm bufio: use WARN_ON in dm_bufio_client_destroy and dm_bufio_exit Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 03/20] dm bufio: never crash if dm_bufio_in_request() Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 04/20] dm bufio: don't bug for clear developer oversight Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 05/20] dm bufio: add LRU abstraction Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 06/20] dm bufio: add dm_buffer_cache abstraction Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 07/20] dm bufio: improve concurrent IO performance Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 08/20] dm bufio: add lock_history optimization for cache iterators Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 09/20] dm bufio: move dm_bufio_client members to avoid spanning cachelines Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 10/20] dm bufio: use waitqueue_active in __free_buffer_wake Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 11/20] dm bufio: use multi-page bio vector Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 12/20] dm thin: speed up cell_defer_no_holder() Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 13/20] dm: split discards further if target sets max_discard_granularity Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 14/20] dm bio prison v1: improve concurrent IO performance Mike Snitzer
2023-03-27 20:11 ` Mike Snitzer [this message]
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 16/20] dm: add dm_num_sharded_locks() Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 17/20] dm bufio: prepare to intelligently size dm_buffer_cache's buffer_trees Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 18/20] dm bufio: " Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 19/20] dm bio prison v1: prepare to intelligently size dm_bio_prison's prison_regions Mike Snitzer
2023-03-27 20:11 ` [dm-devel] [dm-6.4 PATCH v3 20/20] dm bio prison v1: " Mike Snitzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230327201143.51026-16-snitzer@kernel.org \
--to=snitzer@kernel.org \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=ebiggers@kernel.org \
--cc=ejt@redhat.com \
--cc=heinzm@redhat.com \
--cc=keescook@chromium.org \
--cc=linux-block@vger.kernel.org \
--cc=luomeng12@huawei.com \
--cc=mpatocka@redhat.com \
--cc=nhuck@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).