diff options
author | Jeff Moyer <jmoyer@redhat.com> | 2014-08-08 11:03:41 -0400 |
---|---|---|
committer | Mike Snitzer <snitzer@redhat.com> | 2014-08-10 20:54:49 -0400 |
commit | 200612ec33e555a356eebc717630b866ae2b694f (patch) | |
tree | 56f9ed8e6816988f64d375e9ba15f757641eed58 /drivers/md/dm-table.c | |
parent | 56b1ebf2d9798257c5932c8a0dd9da16796dbf36 (diff) |
dm table: propagate QUEUE_FLAG_NO_SG_MERGE
Commit 05f1dd5 ("block: add queue flag for disabling SG merging")
introduced a new queue flag: QUEUE_FLAG_NO_SG_MERGE. This gets set by
default in blk_mq_init_queue for mq-enabled devices. The effect of
the flag is to bypass the SG segment merging. Instead, the
bio->bi_vcnt is used as the number of hardware segments.
With a device mapper target on top of a device with
QUEUE_FLAG_NO_SG_MERGE set, we can end up sending down more segments
than a driver is prepared to handle. I ran into this when backporting
the virtio_blk mq support. It triggerred this BUG_ON, in
virtio_queue_rq:
BUG_ON(req->nr_phys_segments + 2 > vblk->sg_elems);
The queue's max is set here:
blk_queue_max_segments(q, vblk->sg_elems-2);
Basically, what happens is that a bio is built up for the dm device
(which does not have the QUEUE_FLAG_NO_SG_MERGE flag set) using
bio_add_page. That path will call into __blk_recalc_rq_segments, so
what you end up with is bi_phys_segments being much smaller than bi_vcnt
(and bi_vcnt grows beyond the maximum sg elements). Then, when the bio
is submitted, it gets cloned. When the cloned bio is submitted, it will
end up in blk_recount_segments, here:
if (test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags))
bio->bi_phys_segments = bio->bi_vcnt;
and now we've set bio->bi_phys_segments to a number that is beyond what
was registered as queue_max_segments by the driver.
The right way to fix this is to propagate the queue flag up the stack.
The rules for propagating the flag are simple:
- if the flag is set for any underlying device, it must be set for the
upper device
- consequently, if the flag is not set for any underlying device, it
should not be set for the upper device.
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: stable@vger.kernel.org # 3.16+
Diffstat (limited to 'drivers/md/dm-table.c')
-rw-r--r-- | drivers/md/dm-table.c | 13 |
1 files changed, 13 insertions, 0 deletions
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index 3c72bf10e9dc..f9c6cb8dbcf8 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c | |||
@@ -1386,6 +1386,14 @@ static int device_is_not_random(struct dm_target *ti, struct dm_dev *dev, | |||
1386 | return q && !blk_queue_add_random(q); | 1386 | return q && !blk_queue_add_random(q); |
1387 | } | 1387 | } |
1388 | 1388 | ||
1389 | static int queue_supports_sg_merge(struct dm_target *ti, struct dm_dev *dev, | ||
1390 | sector_t start, sector_t len, void *data) | ||
1391 | { | ||
1392 | struct request_queue *q = bdev_get_queue(dev->bdev); | ||
1393 | |||
1394 | return q && !test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags); | ||
1395 | } | ||
1396 | |||
1389 | static bool dm_table_all_devices_attribute(struct dm_table *t, | 1397 | static bool dm_table_all_devices_attribute(struct dm_table *t, |
1390 | iterate_devices_callout_fn func) | 1398 | iterate_devices_callout_fn func) |
1391 | { | 1399 | { |
@@ -1501,6 +1509,11 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, | |||
1501 | if (!dm_table_supports_write_same(t)) | 1509 | if (!dm_table_supports_write_same(t)) |
1502 | q->limits.max_write_same_sectors = 0; | 1510 | q->limits.max_write_same_sectors = 0; |
1503 | 1511 | ||
1512 | if (dm_table_all_devices_attribute(t, queue_supports_sg_merge)) | ||
1513 | queue_flag_clear_unlocked(QUEUE_FLAG_NO_SG_MERGE, q); | ||
1514 | else | ||
1515 | queue_flag_set_unlocked(QUEUE_FLAG_NO_SG_MERGE, q); | ||
1516 | |||
1504 | dm_table_set_integrity(t); | 1517 | dm_table_set_integrity(t); |
1505 | 1518 | ||
1506 | /* | 1519 | /* |