* avoid the ->make_request_fn indirect for blk-mq drivers
@ 2020-04-25 7:53 Christoph Hellwig
2020-04-25 7:53 ` [PATCH 1/3] bcache: remove a duplicate ->make_request_fn assignment Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Christoph Hellwig @ 2020-04-25 7:53 UTC (permalink / raw)
To: Jens Axboe; +Cc: dm-devel, linux-bcache, linux-block
Hi Jens,
this small series avoids an indirect call for every submitted bio that
eventually ends up being handled by blk-mq drivers. Let me know what
you think.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] bcache: remove a duplicate ->make_request_fn assignment
2020-04-25 7:53 avoid the ->make_request_fn indirect for blk-mq drivers Christoph Hellwig
@ 2020-04-25 7:53 ` Christoph Hellwig
2020-04-26 9:35 ` Coly Li
2020-04-25 7:53 ` [PATCH 2/3] dm: remove the make_request_fn check in device_area_is_invalid Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-04-25 7:53 UTC (permalink / raw)
To: Jens Axboe; +Cc: dm-devel, linux-bcache, linux-block
The make_request_fn pointer should only be assigned by blk_alloc_queue.
Fix a left over manual initialization.
Fixes: ff27668ce809 ("bcache: pass the make_request methods to blk_queue_make_request")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/bcache/request.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 71a90fbec314b..77d1a26975174 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -1372,7 +1372,6 @@ void bch_flash_dev_request_init(struct bcache_device *d)
{
struct gendisk *g = d->disk;
- g->queue->make_request_fn = flash_dev_make_request;
g->queue->backing_dev_info->congested_fn = flash_dev_congested;
d->cache_miss = flash_dev_cache_miss;
d->ioctl = flash_dev_ioctl;
--
2.26.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] dm: remove the make_request_fn check in device_area_is_invalid
2020-04-25 7:53 avoid the ->make_request_fn indirect for blk-mq drivers Christoph Hellwig
2020-04-25 7:53 ` [PATCH 1/3] bcache: remove a duplicate ->make_request_fn assignment Christoph Hellwig
@ 2020-04-25 7:53 ` Christoph Hellwig
2020-04-28 18:38 ` Mike Snitzer
2020-04-25 7:53 ` [PATCH 3/3] block: bypass ->make_request_fn for blk-mq drivers Christoph Hellwig
2020-04-25 15:45 ` avoid the ->make_request_fn indirect " Jens Axboe
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-04-25 7:53 UTC (permalink / raw)
To: Jens Axboe; +Cc: dm-devel, linux-bcache, linux-block
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/md/dm-table.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 0a2cc197f62b4..8277b959e00bd 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -279,7 +279,6 @@ static struct dm_dev_internal *find_device(struct list_head *l, dev_t dev)
static int device_area_is_invalid(struct dm_target *ti, struct dm_dev *dev,
sector_t start, sector_t len, void *data)
{
- struct request_queue *q;
struct queue_limits *limits = data;
struct block_device *bdev = dev->bdev;
sector_t dev_size =
@@ -288,22 +287,6 @@ static int device_area_is_invalid(struct dm_target *ti, struct dm_dev *dev,
limits->logical_block_size >> SECTOR_SHIFT;
char b[BDEVNAME_SIZE];
- /*
- * Some devices exist without request functions,
- * such as loop devices not yet bound to backing files.
- * Forbid the use of such devices.
- */
- q = bdev_get_queue(bdev);
- if (!q || !q->make_request_fn) {
- DMWARN("%s: %s is not yet initialised: "
- "start=%llu, len=%llu, dev_size=%llu",
- dm_device_name(ti->table->md), bdevname(bdev, b),
- (unsigned long long)start,
- (unsigned long long)len,
- (unsigned long long)dev_size);
- return 1;
- }
-
if (!dev_size)
return 0;
--
2.26.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] block: bypass ->make_request_fn for blk-mq drivers
2020-04-25 7:53 avoid the ->make_request_fn indirect for blk-mq drivers Christoph Hellwig
2020-04-25 7:53 ` [PATCH 1/3] bcache: remove a duplicate ->make_request_fn assignment Christoph Hellwig
2020-04-25 7:53 ` [PATCH 2/3] dm: remove the make_request_fn check in device_area_is_invalid Christoph Hellwig
@ 2020-04-25 7:53 ` Christoph Hellwig
2020-04-28 18:40 ` Mike Snitzer
2020-04-25 15:45 ` avoid the ->make_request_fn indirect " Jens Axboe
3 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2020-04-25 7:53 UTC (permalink / raw)
To: Jens Axboe; +Cc: dm-devel, linux-bcache, linux-block
Call blk_mq_make_request when no ->make_request_fn is set. This is
safe now that blk_alloc_queue always sets up the pointer for make_request
based drivers. This avoids an indirect call in the blk-mq driver I/O
fast path, which is rather expensive due to spectre mitigations.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-core.c | 26 +++++++++++++++++---------
block/blk-mq.c | 4 ++--
drivers/md/dm.c | 3 +++
include/linux/blk-mq.h | 2 ++
4 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 311596d5dbc41..0e9e1c83e5e84 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1072,7 +1072,10 @@ blk_qc_t generic_make_request(struct bio *bio)
/* Create a fresh bio_list for all subordinate requests */
bio_list_on_stack[1] = bio_list_on_stack[0];
bio_list_init(&bio_list_on_stack[0]);
- ret = q->make_request_fn(q, bio);
+ if (q->make_request_fn)
+ ret = q->make_request_fn(q, bio);
+ else
+ ret = blk_mq_make_request(q, bio);
blk_queue_exit(q);
@@ -1112,9 +1115,7 @@ EXPORT_SYMBOL(generic_make_request);
*
* This function behaves like generic_make_request(), but does not protect
* against recursion. Must only be used if the called driver is known
- * to not call generic_make_request (or direct_make_request) again from
- * its make_request function. (Calling direct_make_request again from
- * a workqueue is perfectly fine as that doesn't recurse).
+ * to be blk-mq based.
*/
blk_qc_t direct_make_request(struct bio *bio)
{
@@ -1122,20 +1123,27 @@ blk_qc_t direct_make_request(struct bio *bio)
bool nowait = bio->bi_opf & REQ_NOWAIT;
blk_qc_t ret;
+ if (WARN_ON_ONCE(q->make_request_fn))
+ goto io_error;
if (!generic_make_request_checks(bio))
return BLK_QC_T_NONE;
if (unlikely(blk_queue_enter(q, nowait ? BLK_MQ_REQ_NOWAIT : 0))) {
if (nowait && !blk_queue_dying(q))
- bio_wouldblock_error(bio);
- else
- bio_io_error(bio);
- return BLK_QC_T_NONE;
+ goto would_block;
+ goto io_error;
}
- ret = q->make_request_fn(q, bio);
+ ret = blk_mq_make_request(q, bio);
blk_queue_exit(q);
return ret;
+
+would_block:
+ bio_wouldblock_error(bio);
+ return BLK_QC_T_NONE;
+io_error:
+ bio_io_error(bio);
+ return BLK_QC_T_NONE;
}
EXPORT_SYMBOL_GPL(direct_make_request);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 71d0894ce1c58..bcc3a2397d4ae 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1984,7 +1984,7 @@ static void blk_add_rq_to_plug(struct blk_plug *plug, struct request *rq)
*
* Returns: Request queue cookie.
*/
-static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
+blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
{
const int is_sync = op_is_sync(bio->bi_opf);
const int is_flush_fua = op_is_flush(bio->bi_opf);
@@ -2096,6 +2096,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
return cookie;
}
+EXPORT_SYMBOL_GPL(blk_mq_make_request); /* only for request based dm */
void blk_mq_free_rqs(struct blk_mq_tag_set *set, struct blk_mq_tags *tags,
unsigned int hctx_idx)
@@ -2955,7 +2956,6 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
INIT_LIST_HEAD(&q->requeue_list);
spin_lock_init(&q->requeue_lock);
- q->make_request_fn = blk_mq_make_request;
q->nr_requests = set->queue_depth;
/*
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index db9e461146531..0eb93da44ea2a 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1788,6 +1788,9 @@ static blk_qc_t dm_make_request(struct request_queue *q, struct bio *bio)
int srcu_idx;
struct dm_table *map;
+ if (dm_get_md_type(md) == DM_TYPE_REQUEST_BASED)
+ return blk_mq_make_request(q, bio);
+
map = dm_get_live_table(md, &srcu_idx);
/* if we're suspended, we have to queue this io for later */
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 51fbf6f76593a..d7307795439a4 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -578,4 +578,6 @@ static inline void blk_mq_cleanup_rq(struct request *rq)
rq->q->mq_ops->cleanup_rq(rq);
}
+blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio);
+
#endif
--
2.26.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: avoid the ->make_request_fn indirect for blk-mq drivers
2020-04-25 7:53 avoid the ->make_request_fn indirect for blk-mq drivers Christoph Hellwig
` (2 preceding siblings ...)
2020-04-25 7:53 ` [PATCH 3/3] block: bypass ->make_request_fn for blk-mq drivers Christoph Hellwig
@ 2020-04-25 15:45 ` Jens Axboe
3 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2020-04-25 15:45 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: dm-devel, linux-bcache, linux-block
On 4/25/20 1:53 AM, Christoph Hellwig wrote:
> Hi Jens,
>
> this small series avoids an indirect call for every submitted bio that
> eventually ends up being handled by blk-mq drivers. Let me know what
> you think.
I like it, I've been pondering something like this for a bit, but
I like the simplicity of this one and changing it so that only
non-regular make_request_fn is set.
I'll apply this.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] bcache: remove a duplicate ->make_request_fn assignment
2020-04-25 7:53 ` [PATCH 1/3] bcache: remove a duplicate ->make_request_fn assignment Christoph Hellwig
@ 2020-04-26 9:35 ` Coly Li
0 siblings, 0 replies; 8+ messages in thread
From: Coly Li @ 2020-04-26 9:35 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe; +Cc: dm-devel, linux-bcache, linux-block
On 2020/4/25 15:53, Christoph Hellwig wrote:
> The make_request_fn pointer should only be assigned by blk_alloc_queue.
> Fix a left over manual initialization.
>
> Fixes: ff27668ce809 ("bcache: pass the make_request methods to blk_queue_make_request")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
It looks good to me.
Reviewed-by: Coly Li <colyli@suse.de>
Thanks.
Coly Li
> ---
> drivers/md/bcache/request.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index 71a90fbec314b..77d1a26975174 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -1372,7 +1372,6 @@ void bch_flash_dev_request_init(struct bcache_device *d)
> {
> struct gendisk *g = d->disk;
>
> - g->queue->make_request_fn = flash_dev_make_request;
> g->queue->backing_dev_info->congested_fn = flash_dev_congested;
> d->cache_miss = flash_dev_cache_miss;
> d->ioctl = flash_dev_ioctl;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dm: remove the make_request_fn check in device_area_is_invalid
2020-04-25 7:53 ` [PATCH 2/3] dm: remove the make_request_fn check in device_area_is_invalid Christoph Hellwig
@ 2020-04-28 18:38 ` Mike Snitzer
0 siblings, 0 replies; 8+ messages in thread
From: Mike Snitzer @ 2020-04-28 18:38 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel, linux-bcache
On Sat, Apr 25 2020 at 3:53am -0400,
Christoph Hellwig <hch@lst.de> wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Think it'd be useful to add a commit message like you did for revert
commit f01b411f41f91fc3196eae4317cf8b4d872830a6 , e.g.:
We can't have queues without a make_request_fn any more (and the loop
device uses blk-mq these days anyway..).
But that aside:
Acked-by: Mike Snitzer <snitzer@redhat.com>
Thanks.
> ---
> drivers/md/dm-table.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 0a2cc197f62b4..8277b959e00bd 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -279,7 +279,6 @@ static struct dm_dev_internal *find_device(struct list_head *l, dev_t dev)
> static int device_area_is_invalid(struct dm_target *ti, struct dm_dev *dev,
> sector_t start, sector_t len, void *data)
> {
> - struct request_queue *q;
> struct queue_limits *limits = data;
> struct block_device *bdev = dev->bdev;
> sector_t dev_size =
> @@ -288,22 +287,6 @@ static int device_area_is_invalid(struct dm_target *ti, struct dm_dev *dev,
> limits->logical_block_size >> SECTOR_SHIFT;
> char b[BDEVNAME_SIZE];
>
> - /*
> - * Some devices exist without request functions,
> - * such as loop devices not yet bound to backing files.
> - * Forbid the use of such devices.
> - */
> - q = bdev_get_queue(bdev);
> - if (!q || !q->make_request_fn) {
> - DMWARN("%s: %s is not yet initialised: "
> - "start=%llu, len=%llu, dev_size=%llu",
> - dm_device_name(ti->table->md), bdevname(bdev, b),
> - (unsigned long long)start,
> - (unsigned long long)len,
> - (unsigned long long)dev_size);
> - return 1;
> - }
> -
> if (!dev_size)
> return 0;
>
> --
> 2.26.1
>
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] block: bypass ->make_request_fn for blk-mq drivers
2020-04-25 7:53 ` [PATCH 3/3] block: bypass ->make_request_fn for blk-mq drivers Christoph Hellwig
@ 2020-04-28 18:40 ` Mike Snitzer
0 siblings, 0 replies; 8+ messages in thread
From: Mike Snitzer @ 2020-04-28 18:40 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, dm-devel, linux-bcache
On Sat, Apr 25 2020 at 3:53am -0400,
Christoph Hellwig <hch@lst.de> wrote:
> Call blk_mq_make_request when no ->make_request_fn is set. This is
> safe now that blk_alloc_queue always sets up the pointer for make_request
> based drivers. This avoids an indirect call in the blk-mq driver I/O
> fast path, which is rather expensive due to spectre mitigations.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Makes me cringe thinking about all the indirect calls sprinkled
throughout DM...
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-04-28 18:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-25 7:53 avoid the ->make_request_fn indirect for blk-mq drivers Christoph Hellwig
2020-04-25 7:53 ` [PATCH 1/3] bcache: remove a duplicate ->make_request_fn assignment Christoph Hellwig
2020-04-26 9:35 ` Coly Li
2020-04-25 7:53 ` [PATCH 2/3] dm: remove the make_request_fn check in device_area_is_invalid Christoph Hellwig
2020-04-28 18:38 ` Mike Snitzer
2020-04-25 7:53 ` [PATCH 3/3] block: bypass ->make_request_fn for blk-mq drivers Christoph Hellwig
2020-04-28 18:40 ` Mike Snitzer
2020-04-25 15:45 ` avoid the ->make_request_fn indirect " Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.