All of lore.kernel.org
 help / color / mirror / Atom feed
* simplify queue allocation
@ 2020-03-27  8:30 Christoph Hellwig
  2020-03-27  8:30 ` [PATCH 1/5] block: add a blk_mq_init_queue_data helper Christoph Hellwig
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27  8:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-bcache, linux-block

Hi Jens,

this series ensures all allocated queues have a valid ->make_request_fn
and also nicely consolidates the code for allocating queues.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/5] block: add a blk_mq_init_queue_data helper
  2020-03-27  8:30 simplify queue allocation Christoph Hellwig
@ 2020-03-27  8:30 ` Christoph Hellwig
  2020-03-27 10:05   ` Johannes Thumshirn
  2020-03-27  8:30 ` [PATCH 2/5] null_blk: use blk_mq_init_queue_data Christoph Hellwig
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27  8:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-bcache, linux-block

This allows a driver to pass a queuedata member before ->init_hctx is
called.  null_blk currently open codes this logic, but I'd rather have
it in the core to ease future maintainance.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c         | 10 +++++++++-
 include/linux/blk-mq.h |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 745ec592a513..216bf62e88b6 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2724,13 +2724,15 @@ void blk_mq_release(struct request_queue *q)
 	blk_mq_sysfs_deinit(q);
 }
 
-struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *set)
+struct request_queue *blk_mq_init_queue_data(struct blk_mq_tag_set *set,
+		void *queuedata)
 {
 	struct request_queue *uninit_q, *q;
 
 	uninit_q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
 	if (!uninit_q)
 		return ERR_PTR(-ENOMEM);
+	uninit_q->queuedata = queuedata;
 
 	/*
 	 * Initialize the queue without an elevator. device_add_disk() will do
@@ -2742,6 +2744,12 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *set)
 
 	return q;
 }
+EXPORT_SYMBOL_GPL(blk_mq_init_queue_data);
+
+struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *set)
+{
+	return blk_mq_init_queue_data(set, NULL);
+}
 EXPORT_SYMBOL(blk_mq_init_queue);
 
 /*
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 31344d5f83e2..f389d7c724bd 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -412,6 +412,8 @@ enum {
 		<< BLK_MQ_F_ALLOC_POLICY_START_BIT)
 
 struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *);
+struct request_queue *blk_mq_init_queue_data(struct blk_mq_tag_set *set,
+		void *queuedata);
 struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 						  struct request_queue *q,
 						  bool elevator_init);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] null_blk: use blk_mq_init_queue_data
  2020-03-27  8:30 simplify queue allocation Christoph Hellwig
  2020-03-27  8:30 ` [PATCH 1/5] block: add a blk_mq_init_queue_data helper Christoph Hellwig
@ 2020-03-27  8:30 ` Christoph Hellwig
  2020-03-27 10:06   ` Johannes Thumshirn
  2020-03-27  8:30 ` [PATCH 3/5] bcache: pass the make_request methods to blk_queue_make_request Christoph Hellwig
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27  8:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-bcache, linux-block

Use the new blk_mq_init_queue_data instead of open coding the queue
allocation and initialization.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/null_blk_main.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 89bb16a99007..2f864782550d 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -1698,27 +1698,6 @@ static bool null_setup_fault(void)
 	return true;
 }
 
-/*
- * This function is identical to blk_mq_init_queue() except that it sets
- * queuedata before .init_hctx is called.
- */
-static struct request_queue *nullb_alloc_queue(struct nullb *nullb)
-{
-	struct request_queue *uninit_q, *q;
-	struct blk_mq_tag_set *set = nullb->tag_set;
-
-	uninit_q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
-	if (!uninit_q)
-		return ERR_PTR(-ENOMEM);
-
-	uninit_q->queuedata = nullb;
-	q = blk_mq_init_allocated_queue(set, uninit_q, false);
-	if (IS_ERR(q))
-		blk_cleanup_queue(uninit_q);
-
-	return q;
-}
-
 static int null_add_dev(struct nullb_device *dev)
 {
 	struct nullb *nullb;
@@ -1758,7 +1737,7 @@ static int null_add_dev(struct nullb_device *dev)
 			goto out_cleanup_queues;
 
 		nullb->tag_set->timeout = 5 * HZ;
-		nullb->q = nullb_alloc_queue(nullb);
+		nullb->q = blk_mq_init_queue_data(nullb->tag_set, nullb);
 		if (IS_ERR(nullb->q)) {
 			rv = -ENOMEM;
 			goto out_cleanup_tags;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] bcache: pass the make_request methods to blk_queue_make_request
  2020-03-27  8:30 simplify queue allocation Christoph Hellwig
  2020-03-27  8:30 ` [PATCH 1/5] block: add a blk_mq_init_queue_data helper Christoph Hellwig
  2020-03-27  8:30 ` [PATCH 2/5] null_blk: use blk_mq_init_queue_data Christoph Hellwig
@ 2020-03-27  8:30 ` Christoph Hellwig
  2020-03-30 11:15   ` Coly Li
  2020-03-27  8:30 ` [PATCH 4/5] block: simplify queue allocation Christoph Hellwig
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27  8:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-bcache, linux-block

bcache is the only driver not actually passing its make_request
methods to blk_queue_make_request, but instead just sets them up
manually a little later.  Make bcache follow the common way of
setting up make_request based queues.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/bcache/request.c |  7 ++-----
 drivers/md/bcache/request.h |  3 +++
 drivers/md/bcache/super.c   | 10 ++++++----
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 820d8402a1dc..71a90fbec314 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -1161,8 +1161,7 @@ static void quit_max_writeback_rate(struct cache_set *c,
 
 /* Cached devices - read & write stuff */
 
-static blk_qc_t cached_dev_make_request(struct request_queue *q,
-					struct bio *bio)
+blk_qc_t cached_dev_make_request(struct request_queue *q, struct bio *bio)
 {
 	struct search *s;
 	struct bcache_device *d = bio->bi_disk->private_data;
@@ -1266,7 +1265,6 @@ void bch_cached_dev_request_init(struct cached_dev *dc)
 {
 	struct gendisk *g = dc->disk.disk;
 
-	g->queue->make_request_fn		= cached_dev_make_request;
 	g->queue->backing_dev_info->congested_fn = cached_dev_congested;
 	dc->disk.cache_miss			= cached_dev_cache_miss;
 	dc->disk.ioctl				= cached_dev_ioctl;
@@ -1301,8 +1299,7 @@ static void flash_dev_nodata(struct closure *cl)
 	continue_at(cl, search_free, NULL);
 }
 
-static blk_qc_t flash_dev_make_request(struct request_queue *q,
-					     struct bio *bio)
+blk_qc_t flash_dev_make_request(struct request_queue *q, struct bio *bio)
 {
 	struct search *s;
 	struct closure *cl;
diff --git a/drivers/md/bcache/request.h b/drivers/md/bcache/request.h
index c64dbd7a91aa..bb005c93dd72 100644
--- a/drivers/md/bcache/request.h
+++ b/drivers/md/bcache/request.h
@@ -37,7 +37,10 @@ unsigned int bch_get_congested(const struct cache_set *c);
 void bch_data_insert(struct closure *cl);
 
 void bch_cached_dev_request_init(struct cached_dev *dc);
+blk_qc_t cached_dev_make_request(struct request_queue *q, struct bio *bio);
+
 void bch_flash_dev_request_init(struct bcache_device *d);
+blk_qc_t flash_dev_make_request(struct request_queue *q, struct bio *bio);
 
 extern struct kmem_cache *bch_search_cache;
 
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 0c3c5419c52b..5e38a167c85e 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -816,7 +816,7 @@ static void bcache_device_free(struct bcache_device *d)
 }
 
 static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
-			      sector_t sectors)
+			      sector_t sectors, make_request_fn make_request_fn)
 {
 	struct request_queue *q;
 	const size_t max_stripes = min_t(size_t, INT_MAX,
@@ -870,7 +870,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 	if (!q)
 		return -ENOMEM;
 
-	blk_queue_make_request(q, NULL);
+	blk_queue_make_request(q, make_request_fn);
 	d->disk->queue			= q;
 	q->queuedata			= d;
 	q->backing_dev_info->congested_data = d;
@@ -1339,7 +1339,8 @@ static int cached_dev_init(struct cached_dev *dc, unsigned int block_size)
 			q->limits.raid_partial_stripes_expensive;
 
 	ret = bcache_device_init(&dc->disk, block_size,
-			 dc->bdev->bd_part->nr_sects - dc->sb.data_offset);
+			 dc->bdev->bd_part->nr_sects - dc->sb.data_offset,
+			 cached_dev_make_request);
 	if (ret)
 		return ret;
 
@@ -1451,7 +1452,8 @@ static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
 
 	kobject_init(&d->kobj, &bch_flash_dev_ktype);
 
-	if (bcache_device_init(d, block_bytes(c), u->sectors))
+	if (bcache_device_init(d, block_bytes(c), u->sectors,
+			flash_dev_make_request))
 		goto err;
 
 	bcache_device_attach(d, c, u - c->uuids);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] block: simplify queue allocation
  2020-03-27  8:30 simplify queue allocation Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-03-27  8:30 ` [PATCH 3/5] bcache: pass the make_request methods to blk_queue_make_request Christoph Hellwig
@ 2020-03-27  8:30 ` Christoph Hellwig
  2020-03-27 10:11   ` Johannes Thumshirn
  2020-03-27  8:30 ` [PATCH 5/5] Revert "blkdev: check for valid request queue before issuing flush" Christoph Hellwig
  2020-03-27 15:53 ` simplify queue allocation Jens Axboe
  5 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27  8:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-bcache, linux-block

Current make_request based drivers use either blk_alloc_queue_node or
blk_alloc_queue to allocate a queue, and then set up the make_request_fn
function pointer and a few parameters using the blk_queue_make_request
helper.  Simplify this by passing the make_request pointer to
blk_alloc_queue, and while at it merge the _node variant into the main
helper by always passing a node_id, and remove the superfluous gfp_mask
parameter.  A lower-level __blk_alloc_queue is kept for the blk-mq case.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/m68k/emu/nfblock.c             |  3 +--
 arch/xtensa/platforms/iss/simdisk.c |  3 +--
 block/blk-cgroup.c                  |  2 +-
 block/blk-core.c                    | 39 +++++++++++++++++------------
 block/blk-mq.c                      |  8 ++----
 block/blk-settings.c                | 36 --------------------------
 block/blk.h                         |  2 ++
 drivers/block/brd.c                 |  4 +--
 drivers/block/drbd/drbd_main.c      |  3 +--
 drivers/block/null_blk_main.c       |  3 +--
 drivers/block/pktcdvd.c             |  3 +--
 drivers/block/ps3vram.c             |  3 +--
 drivers/block/rsxx/dev.c            |  3 +--
 drivers/block/umem.c                |  4 +--
 drivers/block/zram/zram_drv.c       |  4 +--
 drivers/lightnvm/core.c             |  3 +--
 drivers/md/bcache/super.c           |  3 +--
 drivers/md/dm.c                     |  9 +++----
 drivers/md/md.c                     |  3 +--
 drivers/nvdimm/blk.c                |  3 +--
 drivers/nvdimm/btt.c                |  3 +--
 drivers/nvdimm/pmem.c               |  3 +--
 drivers/nvme/host/multipath.c       |  3 +--
 drivers/s390/block/dcssblk.c        |  4 +--
 drivers/s390/block/xpram.c          |  4 +--
 include/linux/blkdev.h              |  4 +--
 26 files changed, 54 insertions(+), 108 deletions(-)

diff --git a/arch/m68k/emu/nfblock.c b/arch/m68k/emu/nfblock.c
index 40712e49381b..c3a630440512 100644
--- a/arch/m68k/emu/nfblock.c
+++ b/arch/m68k/emu/nfblock.c
@@ -118,12 +118,11 @@ static int __init nfhd_init_one(int id, u32 blocks, u32 bsize)
 	dev->bsize = bsize;
 	dev->bshift = ffs(bsize) - 10;
 
-	dev->queue = blk_alloc_queue(GFP_KERNEL);
+	dev->queue = blk_alloc_queue(nfhd_make_request, NUMA_NO_NODE);
 	if (dev->queue == NULL)
 		goto free_dev;
 
 	dev->queue->queuedata = dev;
-	blk_queue_make_request(dev->queue, nfhd_make_request);
 	blk_queue_logical_block_size(dev->queue, bsize);
 
 	dev->disk = alloc_disk(16);
diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c
index 833109880165..49322b66cda9 100644
--- a/arch/xtensa/platforms/iss/simdisk.c
+++ b/arch/xtensa/platforms/iss/simdisk.c
@@ -267,13 +267,12 @@ static int __init simdisk_setup(struct simdisk *dev, int which,
 	spin_lock_init(&dev->lock);
 	dev->users = 0;
 
-	dev->queue = blk_alloc_queue(GFP_KERNEL);
+	dev->queue = blk_alloc_queue(simdisk_make_request, NUMA_NO_NODE);
 	if (dev->queue == NULL) {
 		pr_err("blk_alloc_queue failed\n");
 		goto out_alloc_queue;
 	}
 
-	blk_queue_make_request(dev->queue, simdisk_make_request);
 	dev->queue->queuedata = dev;
 
 	dev->gd = alloc_disk(SIMDISK_MINORS);
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index a229b94d5390..c15a26096038 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1010,7 +1010,7 @@ blkcg_css_alloc(struct cgroup_subsys_state *parent_css)
  * blkcg_init_queue - initialize blkcg part of request queue
  * @q: request_queue to initialize
  *
- * Called from blk_alloc_queue_node(). Responsible for initializing blkcg
+ * Called from __blk_alloc_queue(). Responsible for initializing blkcg
  * part of new request_queue @q.
  *
  * RETURNS:
diff --git a/block/blk-core.c b/block/blk-core.c
index eaf6cb3887e6..18b8c09d093e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -388,12 +388,6 @@ void blk_cleanup_queue(struct request_queue *q)
 }
 EXPORT_SYMBOL(blk_cleanup_queue);
 
-struct request_queue *blk_alloc_queue(gfp_t gfp_mask)
-{
-	return blk_alloc_queue_node(gfp_mask, NUMA_NO_NODE);
-}
-EXPORT_SYMBOL(blk_alloc_queue);
-
 /**
  * blk_queue_enter() - try to increase q->q_usage_counter
  * @q: request queue pointer
@@ -470,24 +464,19 @@ static void blk_timeout_work(struct work_struct *work)
 {
 }
 
-/**
- * blk_alloc_queue_node - allocate a request queue
- * @gfp_mask: memory allocation flags
- * @node_id: NUMA node to allocate memory from
- */
-struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
+struct request_queue *__blk_alloc_queue(int node_id)
 {
 	struct request_queue *q;
 	int ret;
 
 	q = kmem_cache_alloc_node(blk_requestq_cachep,
-				gfp_mask | __GFP_ZERO, node_id);
+				GFP_KERNEL | __GFP_ZERO, node_id);
 	if (!q)
 		return NULL;
 
 	q->last_merge = NULL;
 
-	q->id = ida_simple_get(&blk_queue_ida, 0, 0, gfp_mask);
+	q->id = ida_simple_get(&blk_queue_ida, 0, 0, GFP_KERNEL);
 	if (q->id < 0)
 		goto fail_q;
 
@@ -495,7 +484,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 	if (ret)
 		goto fail_id;
 
-	q->backing_dev_info = bdi_alloc_node(gfp_mask, node_id);
+	q->backing_dev_info = bdi_alloc_node(GFP_KERNEL, node_id);
 	if (!q->backing_dev_info)
 		goto fail_split;
 
@@ -541,6 +530,9 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 	if (blkcg_init_queue(q))
 		goto fail_ref;
 
+	blk_queue_dma_alignment(q, 511);
+	blk_set_default_limits(&q->limits);
+
 	return q;
 
 fail_ref:
@@ -557,7 +549,22 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)
 	kmem_cache_free(blk_requestq_cachep, q);
 	return NULL;
 }
-EXPORT_SYMBOL(blk_alloc_queue_node);
+
+struct request_queue *blk_alloc_queue(make_request_fn make_request, int node_id)
+{
+	struct request_queue *q;
+
+	if (WARN_ON_ONCE(!make_request))
+		return -EINVAL;
+
+	q = __blk_alloc_queue(node_id);
+	if (!q)
+		return NULL;
+	q->make_request_fn = make_request;
+	q->nr_requests = BLKDEV_MAX_RQ;
+	return q;
+}
+EXPORT_SYMBOL(blk_alloc_queue);
 
 bool blk_get_queue(struct request_queue *q)
 {
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 216bf62e88b6..f6291ceedee4 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2729,7 +2729,7 @@ struct request_queue *blk_mq_init_queue_data(struct blk_mq_tag_set *set,
 {
 	struct request_queue *uninit_q, *q;
 
-	uninit_q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
+	uninit_q = __blk_alloc_queue(set->numa_node);
 	if (!uninit_q)
 		return ERR_PTR(-ENOMEM);
 	uninit_q->queuedata = queuedata;
@@ -2939,11 +2939,7 @@ 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);
 
-	blk_queue_make_request(q, blk_mq_make_request);
-
-	/*
-	 * Do this after blk_queue_make_request() overrides it...
-	 */
+	q->make_request_fn = blk_mq_make_request;
 	q->nr_requests = set->queue_depth;
 
 	/*
diff --git a/block/blk-settings.c b/block/blk-settings.c
index c8eda2e7b91e..126d216a2db6 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -86,42 +86,6 @@ void blk_set_stacking_limits(struct queue_limits *lim)
 }
 EXPORT_SYMBOL(blk_set_stacking_limits);
 
-/**
- * blk_queue_make_request - define an alternate make_request function for a device
- * @q:  the request queue for the device to be affected
- * @mfn: the alternate make_request function
- *
- * Description:
- *    The normal way for &struct bios to be passed to a device
- *    driver is for them to be collected into requests on a request
- *    queue, and then to allow the device driver to select requests
- *    off that queue when it is ready.  This works well for many block
- *    devices. However some block devices (typically virtual devices
- *    such as md or lvm) do not benefit from the processing on the
- *    request queue, and are served best by having the requests passed
- *    directly to them.  This can be achieved by providing a function
- *    to blk_queue_make_request().
- *
- * Caveat:
- *    The driver that does this *must* be able to deal appropriately
- *    with buffers in "highmemory". This can be accomplished by either calling
- *    kmap_atomic() to get a temporary kernel mapping, or by calling
- *    blk_queue_bounce() to create a buffer in normal memory.
- **/
-void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn)
-{
-	/*
-	 * set defaults
-	 */
-	q->nr_requests = BLKDEV_MAX_RQ;
-
-	q->make_request_fn = mfn;
-	blk_queue_dma_alignment(q, 511);
-
-	blk_set_default_limits(&q->limits);
-}
-EXPORT_SYMBOL(blk_queue_make_request);
-
 /**
  * blk_queue_bounce_limit - set bounce buffer limit for queue
  * @q: the request queue for the device
diff --git a/block/blk.h b/block/blk.h
index d9673164a145..491e52fc0aa6 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -482,4 +482,6 @@ static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
 #endif
 }
 
+struct request_queue *__blk_alloc_queue(int node_id);
+
 #endif /* BLK_INTERNAL_H */
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 220c5e18aba0..2fb25c348d53 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -381,12 +381,10 @@ static struct brd_device *brd_alloc(int i)
 	spin_lock_init(&brd->brd_lock);
 	INIT_RADIX_TREE(&brd->brd_pages, GFP_ATOMIC);
 
-	brd->brd_queue = blk_alloc_queue(GFP_KERNEL);
+	brd->brd_queue = blk_alloc_queue(brd_make_request, NUMA_NO_NODE);
 	if (!brd->brd_queue)
 		goto out_free_dev;
 
-	blk_queue_make_request(brd->brd_queue, brd_make_request);
-
 	/* This is so fdisk will align partitions on 4k, because of
 	 * direct_access API needing 4k alignment, returning a PFN
 	 * (This is only a problem on very small devices <= 4M,
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index a18155cdce41..72a7c3ea2ce3 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -2801,7 +2801,7 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig
 
 	drbd_init_set_defaults(device);
 
-	q = blk_alloc_queue_node(GFP_KERNEL, NUMA_NO_NODE);
+	q = blk_alloc_queue(drbd_make_request, NUMA_NO_NODE);
 	if (!q)
 		goto out_no_q;
 	device->rq_queue = q;
@@ -2828,7 +2828,6 @@ enum drbd_ret_code drbd_create_device(struct drbd_config_context *adm_ctx, unsig
 	q->backing_dev_info->congested_fn = drbd_congested;
 	q->backing_dev_info->congested_data = device;
 
-	blk_queue_make_request(q, drbd_make_request);
 	blk_queue_write_cache(q, true, true);
 	/* Setting the max_hw_sectors to an odd value of 8kibyte here
 	   This triggers a max_bio_size message upon first attach or connect */
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 2f864782550d..5f13793d35ee 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -1743,12 +1743,11 @@ static int null_add_dev(struct nullb_device *dev)
 			goto out_cleanup_tags;
 		}
 	} else if (dev->queue_mode == NULL_Q_BIO) {
-		nullb->q = blk_alloc_queue_node(GFP_KERNEL, dev->home_node);
+		nullb->q = blk_alloc_queue(null_queue_bio, dev->home_node);
 		if (!nullb->q) {
 			rv = -ENOMEM;
 			goto out_cleanup_queues;
 		}
-		blk_queue_make_request(nullb->q, null_queue_bio);
 		rv = init_driver_queues(nullb);
 		if (rv)
 			goto out_cleanup_blk_queue;
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 5f970a7d32c0..6af3b2e29c62 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2493,7 +2493,6 @@ static void pkt_init_queue(struct pktcdvd_device *pd)
 {
 	struct request_queue *q = pd->disk->queue;
 
-	blk_queue_make_request(q, pkt_make_request);
 	blk_queue_logical_block_size(q, CD_FRAMESIZE);
 	blk_queue_max_hw_sectors(q, PACKET_MAX_SECTORS);
 	q->queuedata = pd;
@@ -2750,7 +2749,7 @@ static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev)
 	strcpy(disk->disk_name, pd->name);
 	disk->devnode = pktcdvd_devnode;
 	disk->private_data = pd;
-	disk->queue = blk_alloc_queue(GFP_KERNEL);
+	disk->queue = blk_alloc_queue(pkt_make_request, NUMA_NO_NODE);
 	if (!disk->queue)
 		goto out_mem2;
 
diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index 4628e1a27a2b..821d4d8b1d76 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -737,7 +737,7 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
 
 	ps3vram_proc_init(dev);
 
-	queue = blk_alloc_queue(GFP_KERNEL);
+	queue = blk_alloc_queue(ps3vram_make_request, NUMA_NO_NODE);
 	if (!queue) {
 		dev_err(&dev->core, "blk_alloc_queue failed\n");
 		error = -ENOMEM;
@@ -746,7 +746,6 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
 
 	priv->queue = queue;
 	queue->queuedata = dev;
-	blk_queue_make_request(queue, ps3vram_make_request);
 	blk_queue_max_segments(queue, BLK_MAX_SEGMENTS);
 	blk_queue_max_segment_size(queue, BLK_MAX_SEGMENT_SIZE);
 	blk_queue_max_hw_sectors(queue, BLK_SAFE_MAX_SECTORS);
diff --git a/drivers/block/rsxx/dev.c b/drivers/block/rsxx/dev.c
index c47d28b2ce44..8ffa8260dcaf 100644
--- a/drivers/block/rsxx/dev.c
+++ b/drivers/block/rsxx/dev.c
@@ -248,7 +248,7 @@ int rsxx_setup_dev(struct rsxx_cardinfo *card)
 		return -ENOMEM;
 	}
 
-	card->queue = blk_alloc_queue(GFP_KERNEL);
+	card->queue = blk_alloc_queue(rsxx_make_request, NUMA_NO_NODE);
 	if (!card->queue) {
 		dev_err(CARD_TO_DEV(card), "Failed queue alloc\n");
 		unregister_blkdev(card->major, DRIVER_NAME);
@@ -269,7 +269,6 @@ int rsxx_setup_dev(struct rsxx_cardinfo *card)
 		blk_queue_logical_block_size(card->queue, blk_size);
 	}
 
-	blk_queue_make_request(card->queue, rsxx_make_request);
 	blk_queue_max_hw_sectors(card->queue, blkdev_max_hw_sectors);
 	blk_queue_physical_block_size(card->queue, RSXX_HW_BLK_SIZE);
 
diff --git a/drivers/block/umem.c b/drivers/block/umem.c
index 4eaf97d7a170..d84e8a878df2 100644
--- a/drivers/block/umem.c
+++ b/drivers/block/umem.c
@@ -885,11 +885,9 @@ static int mm_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	card->biotail = &card->bio;
 	spin_lock_init(&card->lock);
 
-	card->queue = blk_alloc_queue_node(GFP_KERNEL, NUMA_NO_NODE);
+	card->queue = blk_alloc_queue(mm_make_request, NUMA_NO_NODE);
 	if (!card->queue)
 		goto failed_alloc;
-
-	blk_queue_make_request(card->queue, mm_make_request);
 	card->queue->queuedata = card;
 
 	tasklet_init(&card->tasklet, process_page, (unsigned long)card);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 76e75097e9ab..ebb234f36909 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1895,7 +1895,7 @@ static int zram_add(void)
 #ifdef CONFIG_ZRAM_WRITEBACK
 	spin_lock_init(&zram->wb_limit_lock);
 #endif
-	queue = blk_alloc_queue(GFP_KERNEL);
+	queue = blk_alloc_queue(zram_make_request, NUMA_NO_NODE);
 	if (!queue) {
 		pr_err("Error allocating disk queue for device %d\n",
 			device_id);
@@ -1903,8 +1903,6 @@ static int zram_add(void)
 		goto out_free_idr;
 	}
 
-	blk_queue_make_request(queue, zram_make_request);
-
 	/* gendisk structure */
 	zram->disk = alloc_disk(1);
 	if (!zram->disk) {
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 7543e395a2c6..db38a68abb6c 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -380,12 +380,11 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
 		goto err_dev;
 	}
 
-	tqueue = blk_alloc_queue_node(GFP_KERNEL, dev->q->node);
+	tqueue = blk_alloc_queue(tt->make_rq, dev->q->node);
 	if (!tqueue) {
 		ret = -ENOMEM;
 		goto err_disk;
 	}
-	blk_queue_make_request(tqueue, tt->make_rq);
 
 	strlcpy(tdisk->disk_name, create->tgtname, sizeof(tdisk->disk_name));
 	tdisk->flags = GENHD_FL_EXT_DEVT;
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 5e38a167c85e..d98354fa28e3 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -866,11 +866,10 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 	d->disk->fops		= &bcache_ops;
 	d->disk->private_data	= d;
 
-	q = blk_alloc_queue(GFP_KERNEL);
+	q = blk_alloc_queue(make_request_fn, NUMA_NO_NODE);
 	if (!q)
 		return -ENOMEM;
 
-	blk_queue_make_request(q, make_request_fn);
 	d->disk->queue			= q;
 	q->queuedata			= d;
 	q->backing_dev_info->congested_data = d;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 0d881cfa160b..753302e83910 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1939,16 +1939,15 @@ static struct mapped_device *alloc_dev(int minor)
 	INIT_LIST_HEAD(&md->table_devices);
 	spin_lock_init(&md->uevent_lock);
 
-	md->queue = blk_alloc_queue_node(GFP_KERNEL, numa_node_id);
-	if (!md->queue)
-		goto bad;
-	md->queue->queuedata = md;
 	/*
 	 * default to bio-based required ->make_request_fn until DM
 	 * table is loaded and md->type established. If request-based
 	 * table is loaded: blk-mq will override accordingly.
 	 */
-	blk_queue_make_request(md->queue, dm_make_request);
+	md->queue = blk_alloc_queue(dm_make_request, numa_node_id);
+	if (!md->queue)
+		goto bad;
+	md->queue->queuedata = md;
 
 	md->disk = alloc_disk_node(1, md->numa_node_id);
 	if (!md->disk)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index f6cf3b53f6c1..cd1210a0d957 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5623,12 +5623,11 @@ static int md_alloc(dev_t dev, char *name)
 		mddev->hold_active = UNTIL_STOP;
 
 	error = -ENOMEM;
-	mddev->queue = blk_alloc_queue(GFP_KERNEL);
+	mddev->queue = blk_alloc_queue(md_make_request, NUMA_NO_NODE);
 	if (!mddev->queue)
 		goto abort;
 	mddev->queue->queuedata = mddev;
 
-	blk_queue_make_request(mddev->queue, md_make_request);
 	blk_set_stacking_limits(&mddev->queue->limits);
 
 	disk = alloc_disk(1 << shift);
diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
index 677d6f45b5c4..43751fab9d36 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -249,13 +249,12 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk)
 	internal_nlba = div_u64(nsblk->size, nsblk_internal_lbasize(nsblk));
 	available_disk_size = internal_nlba * nsblk_sector_size(nsblk);
 
-	q = blk_alloc_queue(GFP_KERNEL);
+	q = blk_alloc_queue(nd_blk_make_request, NUMA_NO_NODE);
 	if (!q)
 		return -ENOMEM;
 	if (devm_add_action_or_reset(dev, nd_blk_release_queue, q))
 		return -ENOMEM;
 
-	blk_queue_make_request(q, nd_blk_make_request);
 	blk_queue_max_hw_sectors(q, UINT_MAX);
 	blk_queue_logical_block_size(q, nsblk_sector_size(nsblk));
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 0d04ea3d9fd7..3b09419218d6 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1521,7 +1521,7 @@ static int btt_blk_init(struct btt *btt)
 	struct nd_namespace_common *ndns = nd_btt->ndns;
 
 	/* create a new disk and request queue for btt */
-	btt->btt_queue = blk_alloc_queue(GFP_KERNEL);
+	btt->btt_queue = blk_alloc_queue(btt_make_request, NUMA_NO_NODE);
 	if (!btt->btt_queue)
 		return -ENOMEM;
 
@@ -1540,7 +1540,6 @@ static int btt_blk_init(struct btt *btt)
 	btt->btt_disk->queue->backing_dev_info->capabilities |=
 			BDI_CAP_SYNCHRONOUS_IO;
 
-	blk_queue_make_request(btt->btt_queue, btt_make_request);
 	blk_queue_logical_block_size(btt->btt_queue, btt->sector_size);
 	blk_queue_max_hw_sectors(btt->btt_queue, UINT_MAX);
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, btt->btt_queue);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 4eae441f86c9..4ffc6f7ca131 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -395,7 +395,7 @@ static int pmem_attach_disk(struct device *dev,
 		return -EBUSY;
 	}
 
-	q = blk_alloc_queue_node(GFP_KERNEL, dev_to_node(dev));
+	q = blk_alloc_queue(pmem_make_request, dev_to_node(dev));
 	if (!q)
 		return -ENOMEM;
 
@@ -433,7 +433,6 @@ static int pmem_attach_disk(struct device *dev,
 	pmem->virt_addr = addr;
 
 	blk_queue_write_cache(q, true, fua);
-	blk_queue_make_request(q, pmem_make_request);
 	blk_queue_physical_block_size(q, PAGE_SIZE);
 	blk_queue_logical_block_size(q, pmem_sector_size(ndns));
 	blk_queue_max_hw_sectors(q, UINT_MAX);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a11900cf3a36..a38d7f196aba 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -377,11 +377,10 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
 	if (!(ctrl->subsys->cmic & (1 << 1)) || !multipath)
 		return 0;
 
-	q = blk_alloc_queue_node(GFP_KERNEL, ctrl->numa_node);
+	q = blk_alloc_queue(nvme_ns_head_make_request, ctrl->numa_node);
 	if (!q)
 		goto out;
 	q->queuedata = head;
-	blk_queue_make_request(q, nvme_ns_head_make_request);
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
 	/* set to a default value for 512 until disk is validated */
 	blk_queue_logical_block_size(q, 512);
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 63502ca537eb..80d22290f268 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -636,10 +636,10 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
 	}
 	dev_info->gd->major = dcssblk_major;
 	dev_info->gd->fops = &dcssblk_devops;
-	dev_info->dcssblk_queue = blk_alloc_queue(GFP_KERNEL);
+	dev_info->dcssblk_queue =
+		blk_alloc_queue(dcssblk_make_request, NUMA_NO_NODE);
 	dev_info->gd->queue = dev_info->dcssblk_queue;
 	dev_info->gd->private_data = dev_info;
-	blk_queue_make_request(dev_info->dcssblk_queue, dcssblk_make_request);
 	blk_queue_logical_block_size(dev_info->dcssblk_queue, 4096);
 	blk_queue_flag_set(QUEUE_FLAG_DAX, dev_info->dcssblk_queue);
 
diff --git a/drivers/s390/block/xpram.c b/drivers/s390/block/xpram.c
index 3df5d68d09f0..45a04daec89e 100644
--- a/drivers/s390/block/xpram.c
+++ b/drivers/s390/block/xpram.c
@@ -343,14 +343,14 @@ static int __init xpram_setup_blkdev(void)
 		xpram_disks[i] = alloc_disk(1);
 		if (!xpram_disks[i])
 			goto out;
-		xpram_queues[i] = blk_alloc_queue(GFP_KERNEL);
+		xpram_queues[i] = blk_alloc_queue(xpram_make_request,
+				NUMA_NO_NODE);
 		if (!xpram_queues[i]) {
 			put_disk(xpram_disks[i]);
 			goto out;
 		}
 		blk_queue_flag_set(QUEUE_FLAG_NONROT, xpram_queues[i]);
 		blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, xpram_queues[i]);
-		blk_queue_make_request(xpram_queues[i], xpram_make_request);
 		blk_queue_logical_block_size(xpram_queues[i], 4096);
 	}
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 53a1325efbc3..fdb517c33603 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1063,7 +1063,6 @@ extern void blk_abort_request(struct request *);
  * Access functions for manipulating queue properties
  */
 extern void blk_cleanup_queue(struct request_queue *);
-extern void blk_queue_make_request(struct request_queue *, make_request_fn *);
 extern void blk_queue_bounce_limit(struct request_queue *, u64);
 extern void blk_queue_max_hw_sectors(struct request_queue *, unsigned int);
 extern void blk_queue_chunk_sectors(struct request_queue *, unsigned int);
@@ -1140,8 +1139,7 @@ extern void blk_dump_rq_flags(struct request *, char *);
 extern long nr_blockdev_pages(void);
 
 bool __must_check blk_get_queue(struct request_queue *);
-struct request_queue *blk_alloc_queue(gfp_t);
-struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id);
+struct request_queue *blk_alloc_queue(make_request_fn make_request, int node_id);
 extern void blk_put_queue(struct request_queue *);
 extern void blk_set_queue_dying(struct request_queue *);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] Revert "blkdev: check for valid request queue before issuing flush"
  2020-03-27  8:30 simplify queue allocation Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-03-27  8:30 ` [PATCH 4/5] block: simplify queue allocation Christoph Hellwig
@ 2020-03-27  8:30 ` Christoph Hellwig
  2020-03-27 15:53 ` simplify queue allocation Jens Axboe
  5 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27  8:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-bcache, linux-block

This reverts commit f10d9f617a65905c556c3b37c9b9646ae7d04ed7.

We can't have queues without a make_request_fn any more (and the
loop device uses blk-mq these days anyway..).

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-flush.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/block/blk-flush.c b/block/blk-flush.c
index 843d25683691..c7f396e3d5e2 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -454,15 +454,6 @@ int blkdev_issue_flush(struct block_device *bdev, gfp_t gfp_mask,
 	if (!q)
 		return -ENXIO;
 
-	/*
-	 * some block devices may not have their queue correctly set up here
-	 * (e.g. loop device without a backing file) and so issuing a flush
-	 * here will panic. Ensure there is a request function before issuing
-	 * the flush.
-	 */
-	if (!q->make_request_fn)
-		return -ENXIO;
-
 	bio = bio_alloc(gfp_mask, 0);
 	bio_set_dev(bio, bdev);
 	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] block: add a blk_mq_init_queue_data helper
  2020-03-27  8:30 ` [PATCH 1/5] block: add a blk_mq_init_queue_data helper Christoph Hellwig
@ 2020-03-27 10:05   ` Johannes Thumshirn
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Thumshirn @ 2020-03-27 10:05 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-bcache, linux-block

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] null_blk: use blk_mq_init_queue_data
  2020-03-27  8:30 ` [PATCH 2/5] null_blk: use blk_mq_init_queue_data Christoph Hellwig
@ 2020-03-27 10:06   ` Johannes Thumshirn
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Thumshirn @ 2020-03-27 10:06 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-bcache, linux-block

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] block: simplify queue allocation
  2020-03-27  8:30 ` [PATCH 4/5] block: simplify queue allocation Christoph Hellwig
@ 2020-03-27 10:11   ` Johannes Thumshirn
  2020-03-27 11:21     ` Christoph Hellwig
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Thumshirn @ 2020-03-27 10:11 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-bcache, linux-block

On 27/03/2020 09:34, Christoph Hellwig wrote:
> -struct request_queue *blk_alloc_queue(gfp_t gfp_mask)
> -{
> -	return blk_alloc_queue_node(gfp_mask, NUMA_NO_NODE);
> -}
> -EXPORT_SYMBOL(blk_alloc_queue);

Why are you removing the non _node() variant? The memory allocation 
function for instance have this indirection as well and I don't think it 
simplifies a lot passing NUMA_NO_NODE in each block driver.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] block: simplify queue allocation
  2020-03-27 10:11   ` Johannes Thumshirn
@ 2020-03-27 11:21     ` Christoph Hellwig
  2020-03-27 16:24       ` Johannes Thumshirn
  0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27 11:21 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Christoph Hellwig, Jens Axboe, linux-bcache, linux-block

On Fri, Mar 27, 2020 at 10:11:08AM +0000, Johannes Thumshirn wrote:
> On 27/03/2020 09:34, Christoph Hellwig wrote:
> > -struct request_queue *blk_alloc_queue(gfp_t gfp_mask)
> > -{
> > -	return blk_alloc_queue_node(gfp_mask, NUMA_NO_NODE);
> > -}
> > -EXPORT_SYMBOL(blk_alloc_queue);
> 
> Why are you removing the non _node() variant? The memory allocation 
> function for instance have this indirection as well and I don't think it 
> simplifies a lot passing NUMA_NO_NODE in each block driver.

Because the two variants are rather pointless.  And this might get
more people to actually pass a useful node ID instead of copy and
pasting some old example code.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: simplify queue allocation
  2020-03-27  8:30 simplify queue allocation Christoph Hellwig
                   ` (4 preceding siblings ...)
  2020-03-27  8:30 ` [PATCH 5/5] Revert "blkdev: check for valid request queue before issuing flush" Christoph Hellwig
@ 2020-03-27 15:53 ` Jens Axboe
  2020-03-27 16:17   ` Christoph Hellwig
  5 siblings, 1 reply; 16+ messages in thread
From: Jens Axboe @ 2020-03-27 15:53 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-bcache, linux-block

On 3/27/20 2:30 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series ensures all allocated queues have a valid ->make_request_fn
> and also nicely consolidates the code for allocating queues.

This seems fine to me, but might be a good idea to shuffle 4/5 as the
last one, and do that one inside the merge window to avoid any potential
silly merge conflicts.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: simplify queue allocation
  2020-03-27 15:53 ` simplify queue allocation Jens Axboe
@ 2020-03-27 16:17   ` Christoph Hellwig
  2020-03-27 16:18     ` Jens Axboe
  0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2020-03-27 16:17 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, linux-bcache, linux-block

On Fri, Mar 27, 2020 at 09:53:16AM -0600, Jens Axboe wrote:
> On 3/27/20 2:30 AM, Christoph Hellwig wrote:
> > Hi Jens,
> > 
> > this series ensures all allocated queues have a valid ->make_request_fn
> > and also nicely consolidates the code for allocating queues.
> 
> This seems fine to me, but might be a good idea to shuffle 4/5 as the
> last one, and do that one inside the merge window to avoid any potential
> silly merge conflicts.

they should be trivial to reorder if you want to skip patch 4 for now.
But looking at current linux-next there isn't any conflict yet,
and I don't expect one as most block drivers go through the block
tree anyway.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: simplify queue allocation
  2020-03-27 16:17   ` Christoph Hellwig
@ 2020-03-27 16:18     ` Jens Axboe
  0 siblings, 0 replies; 16+ messages in thread
From: Jens Axboe @ 2020-03-27 16:18 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-bcache, linux-block

On 3/27/20 10:17 AM, Christoph Hellwig wrote:
> On Fri, Mar 27, 2020 at 09:53:16AM -0600, Jens Axboe wrote:
>> On 3/27/20 2:30 AM, Christoph Hellwig wrote:
>>> Hi Jens,
>>>
>>> this series ensures all allocated queues have a valid ->make_request_fn
>>> and also nicely consolidates the code for allocating queues.
>>
>> This seems fine to me, but might be a good idea to shuffle 4/5 as the
>> last one, and do that one inside the merge window to avoid any potential
>> silly merge conflicts.
> 
> they should be trivial to reorder if you want to skip patch 4 for now.
> But looking at current linux-next there isn't any conflict yet,
> and I don't expect one as most block drivers go through the block
> tree anyway.

Yeah I guess so, and any potential conflict would be trivial. I'll apply
it and hope for the best.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] block: simplify queue allocation
  2020-03-27 11:21     ` Christoph Hellwig
@ 2020-03-27 16:24       ` Johannes Thumshirn
  2020-03-27 16:26         ` Jens Axboe
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Thumshirn @ 2020-03-27 16:24 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-bcache, linux-block

On 27/03/2020 12:21, Christoph Hellwig wrote:
> Because the two variants are rather pointless.  And this might get
> more people to actually pass a useful node ID instead of copy and
> pasting some old example code. 

So then everybody will copy the NUMA_NO_NODE ones and in X years someone 
will come up with a wrapper passing in NUMA_NO_NODE to 
blk_alloc_queue_node(), because he/she didn't read the commit history.

But if I'm the only one objecting to it,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] block: simplify queue allocation
  2020-03-27 16:24       ` Johannes Thumshirn
@ 2020-03-27 16:26         ` Jens Axboe
  0 siblings, 0 replies; 16+ messages in thread
From: Jens Axboe @ 2020-03-27 16:26 UTC (permalink / raw)
  To: Johannes Thumshirn, Christoph Hellwig; +Cc: linux-bcache, linux-block

On 3/27/20 10:24 AM, Johannes Thumshirn wrote:
> On 27/03/2020 12:21, Christoph Hellwig wrote:
>> Because the two variants are rather pointless.  And this might get
>> more people to actually pass a useful node ID instead of copy and
>> pasting some old example code. 
> 
> So then everybody will copy the NUMA_NO_NODE ones and in X years someone 
> will come up with a wrapper passing in NUMA_NO_NODE to 
> blk_alloc_queue_node(), because he/she didn't read the commit history.

If you care about numa-node, then you are generally using it for other
allocations as well, so it should come naturally. If you don't, then
the copy/paste is fine. Better to expose it as the main API, so people
don't miss it.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] bcache: pass the make_request methods to blk_queue_make_request
  2020-03-27  8:30 ` [PATCH 3/5] bcache: pass the make_request methods to blk_queue_make_request Christoph Hellwig
@ 2020-03-30 11:15   ` Coly Li
  0 siblings, 0 replies; 16+ messages in thread
From: Coly Li @ 2020-03-30 11:15 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-bcache, linux-block

On 2020/3/27 4:30 下午, Christoph Hellwig wrote:
> bcache is the only driver not actually passing its make_request
> methods to blk_queue_make_request, but instead just sets them up
> manually a little later.  Make bcache follow the common way of
> setting up make_request based queues.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

For the bcache part, it is fine for me.

Reviewed-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li

> ---
>  drivers/md/bcache/request.c |  7 ++-----
>  drivers/md/bcache/request.h |  3 +++
>  drivers/md/bcache/super.c   | 10 ++++++----
>  3 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index 820d8402a1dc..71a90fbec314 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -1161,8 +1161,7 @@ static void quit_max_writeback_rate(struct cache_set *c,
>  
>  /* Cached devices - read & write stuff */
>  
> -static blk_qc_t cached_dev_make_request(struct request_queue *q,
> -					struct bio *bio)
> +blk_qc_t cached_dev_make_request(struct request_queue *q, struct bio *bio)
>  {
>  	struct search *s;
>  	struct bcache_device *d = bio->bi_disk->private_data;
> @@ -1266,7 +1265,6 @@ void bch_cached_dev_request_init(struct cached_dev *dc)
>  {
>  	struct gendisk *g = dc->disk.disk;
>  
> -	g->queue->make_request_fn		= cached_dev_make_request;
>  	g->queue->backing_dev_info->congested_fn = cached_dev_congested;
>  	dc->disk.cache_miss			= cached_dev_cache_miss;
>  	dc->disk.ioctl				= cached_dev_ioctl;
> @@ -1301,8 +1299,7 @@ static void flash_dev_nodata(struct closure *cl)
>  	continue_at(cl, search_free, NULL);
>  }
>  
> -static blk_qc_t flash_dev_make_request(struct request_queue *q,
> -					     struct bio *bio)
> +blk_qc_t flash_dev_make_request(struct request_queue *q, struct bio *bio)
>  {
>  	struct search *s;
>  	struct closure *cl;
> diff --git a/drivers/md/bcache/request.h b/drivers/md/bcache/request.h
> index c64dbd7a91aa..bb005c93dd72 100644
> --- a/drivers/md/bcache/request.h
> +++ b/drivers/md/bcache/request.h
> @@ -37,7 +37,10 @@ unsigned int bch_get_congested(const struct cache_set *c);
>  void bch_data_insert(struct closure *cl);
>  
>  void bch_cached_dev_request_init(struct cached_dev *dc);
> +blk_qc_t cached_dev_make_request(struct request_queue *q, struct bio *bio);
> +
>  void bch_flash_dev_request_init(struct bcache_device *d);
> +blk_qc_t flash_dev_make_request(struct request_queue *q, struct bio *bio);
>  
>  extern struct kmem_cache *bch_search_cache;
>  
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 0c3c5419c52b..5e38a167c85e 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -816,7 +816,7 @@ static void bcache_device_free(struct bcache_device *d)
>  }
>  
>  static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
> -			      sector_t sectors)
> +			      sector_t sectors, make_request_fn make_request_fn)
>  {
>  	struct request_queue *q;
>  	const size_t max_stripes = min_t(size_t, INT_MAX,
> @@ -870,7 +870,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
>  	if (!q)
>  		return -ENOMEM;
>  
> -	blk_queue_make_request(q, NULL);
> +	blk_queue_make_request(q, make_request_fn);
>  	d->disk->queue			= q;
>  	q->queuedata			= d;
>  	q->backing_dev_info->congested_data = d;
> @@ -1339,7 +1339,8 @@ static int cached_dev_init(struct cached_dev *dc, unsigned int block_size)
>  			q->limits.raid_partial_stripes_expensive;
>  
>  	ret = bcache_device_init(&dc->disk, block_size,
> -			 dc->bdev->bd_part->nr_sects - dc->sb.data_offset);
> +			 dc->bdev->bd_part->nr_sects - dc->sb.data_offset,
> +			 cached_dev_make_request);
>  	if (ret)
>  		return ret;
>  
> @@ -1451,7 +1452,8 @@ static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
>  
>  	kobject_init(&d->kobj, &bch_flash_dev_ktype);
>  
> -	if (bcache_device_init(d, block_bytes(c), u->sectors))
> +	if (bcache_device_init(d, block_bytes(c), u->sectors,
> +			flash_dev_make_request))
>  		goto err;
>  
>  	bcache_device_attach(d, c, u - c->uuids);
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-03-30 11:15 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27  8:30 simplify queue allocation Christoph Hellwig
2020-03-27  8:30 ` [PATCH 1/5] block: add a blk_mq_init_queue_data helper Christoph Hellwig
2020-03-27 10:05   ` Johannes Thumshirn
2020-03-27  8:30 ` [PATCH 2/5] null_blk: use blk_mq_init_queue_data Christoph Hellwig
2020-03-27 10:06   ` Johannes Thumshirn
2020-03-27  8:30 ` [PATCH 3/5] bcache: pass the make_request methods to blk_queue_make_request Christoph Hellwig
2020-03-30 11:15   ` Coly Li
2020-03-27  8:30 ` [PATCH 4/5] block: simplify queue allocation Christoph Hellwig
2020-03-27 10:11   ` Johannes Thumshirn
2020-03-27 11:21     ` Christoph Hellwig
2020-03-27 16:24       ` Johannes Thumshirn
2020-03-27 16:26         ` Jens Axboe
2020-03-27  8:30 ` [PATCH 5/5] Revert "blkdev: check for valid request queue before issuing flush" Christoph Hellwig
2020-03-27 15:53 ` simplify queue allocation Jens Axboe
2020-03-27 16:17   ` Christoph Hellwig
2020-03-27 16:18     ` 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.