Skip to content

Commit 3f8d3f5

Browse files
author
Mike Snitzer
committed
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 <snitzer@kernel.org>
1 parent e2dd8ac commit 3f8d3f5

3 files changed

Lines changed: 29 additions & 11 deletions

File tree

drivers/md/dm-bio-prison-v1.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,17 @@ static unsigned lock_nr(struct dm_cell_key *key)
120120
return (key->block_begin >> BIO_PRISON_MAX_RANGE_SHIFT) & LOCK_MASK;
121121
}
122122

123-
static void check_range(struct dm_cell_key *key)
123+
bool dm_cell_key_has_valid_range(struct dm_cell_key *key)
124124
{
125-
BUG_ON(key->block_end - key->block_begin > BIO_PRISON_MAX_RANGE);
126-
BUG_ON((key->block_begin >> BIO_PRISON_MAX_RANGE_SHIFT) !=
127-
((key->block_end - 1) >> BIO_PRISON_MAX_RANGE_SHIFT));
125+
if (WARN_ON_ONCE(key->block_end - key->block_begin > BIO_PRISON_MAX_RANGE))
126+
return false;
127+
if (WARN_ON_ONCE((key->block_begin >> BIO_PRISON_MAX_RANGE_SHIFT) !=
128+
(key->block_end - 1) >> BIO_PRISON_MAX_RANGE_SHIFT))
129+
return false;
130+
131+
return true;
128132
}
133+
EXPORT_SYMBOL(dm_cell_key_has_valid_range);
129134

130135
static int __bio_detain(struct rb_root *root,
131136
struct dm_cell_key *key,
@@ -172,7 +177,6 @@ static int bio_detain(struct dm_bio_prison *prison,
172177
{
173178
int r;
174179
unsigned l = lock_nr(key);
175-
check_range(key);
176180

177181
spin_lock_irq(&prison->regions[l].lock);
178182
r = __bio_detain(&prison->regions[l].cell, key, inmate, cell_prealloc, cell_result);

drivers/md/dm-bio-prison-v1.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ int dm_get_cell(struct dm_bio_prison *prison,
8383
struct dm_bio_prison_cell *cell_prealloc,
8484
struct dm_bio_prison_cell **cell_result);
8585

86+
/*
87+
* Returns false if key is beyond BIO_PRISON_MAX_RANGE or spans a boundary.
88+
*/
89+
bool dm_cell_key_has_valid_range(struct dm_cell_key *key);
90+
8691
/*
8792
* An atomic op that combines retrieving or creating a cell, and adding a
8893
* bio to it.

drivers/md/dm-thin.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,25 +118,27 @@ enum lock_space {
118118
PHYSICAL
119119
};
120120

121-
static void build_key(struct dm_thin_device *td, enum lock_space ls,
121+
static bool build_key(struct dm_thin_device *td, enum lock_space ls,
122122
dm_block_t b, dm_block_t e, struct dm_cell_key *key)
123123
{
124124
key->virtual = (ls == VIRTUAL);
125125
key->dev = dm_thin_dev_id(td);
126126
key->block_begin = b;
127127
key->block_end = e;
128+
129+
return dm_cell_key_has_valid_range(key);
128130
}
129131

130132
static void build_data_key(struct dm_thin_device *td, dm_block_t b,
131133
struct dm_cell_key *key)
132134
{
133-
build_key(td, PHYSICAL, b, b + 1llu, key);
135+
(void) build_key(td, PHYSICAL, b, b + 1llu, key);
134136
}
135137

136138
static void build_virtual_key(struct dm_thin_device *td, dm_block_t b,
137139
struct dm_cell_key *key)
138140
{
139-
build_key(td, VIRTUAL, b, b + 1llu, key);
141+
(void) build_key(td, VIRTUAL, b, b + 1llu, key);
140142
}
141143

142144
/*----------------------------------------------------------------*/
@@ -1702,7 +1704,8 @@ static void break_up_discard_bio(struct thin_c *tc, dm_block_t begin, dm_block_t
17021704
<< BIO_PRISON_MAX_RANGE_SHIFT;
17031705
len = min_t(sector_t, data_end - data_begin, next_boundary - data_begin);
17041706

1705-
build_key(tc->td, PHYSICAL, data_begin, data_begin + len, &data_key);
1707+
/* This key is certainly within range given the above splitting */
1708+
(void) build_key(tc->td, PHYSICAL, data_begin, data_begin + len, &data_key);
17061709
if (bio_detain(tc->pool, &data_key, NULL, &data_cell)) {
17071710
/* contention, we'll give up with this range */
17081711
data_begin += len;
@@ -1778,8 +1781,13 @@ static void process_discard_bio(struct thin_c *tc, struct bio *bio)
17781781
return;
17791782
}
17801783

1781-
build_key(tc->td, VIRTUAL, begin, end, &virt_key);
1782-
if (bio_detain(tc->pool, &virt_key, bio, &virt_cell))
1784+
if (unlikely(!build_key(tc->td, VIRTUAL, begin, end, &virt_key))) {
1785+
DMERR_LIMIT("Discard doesn't respect bio prison limits");
1786+
bio_endio(bio);
1787+
return;
1788+
}
1789+
1790+
if (bio_detain(tc->pool, &virt_key, bio, &virt_cell)) {
17831791
/*
17841792
* Potential starvation issue: We're relying on the
17851793
* 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)
17881796
* cell will never be granted.
17891797
*/
17901798
return;
1799+
}
17911800

17921801
tc->pool->process_discard_cell(tc, virt_cell);
17931802
}

0 commit comments

Comments
 (0)