linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] null_blk: poll queue support
@ 2021-04-17 15:29 Jens Axboe
  2021-04-18  4:48 ` Chaitanya Kulkarni
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Jens Axboe @ 2021-04-17 15:29 UTC (permalink / raw)
  To: linux-block

There's currently no way to experiment with polled IO with null_blk,
which seems like an oversight. This patch adds support for polled IO.
We keep a list of issued IOs on submit, and then process that list
when mq_ops->poll() is invoked.

A new parameter is added, poll_queues. It defaults to 1 like the
submit queues, meaning we'll have 1 poll queue available.

Signed-off-by: Jens Axboe <axboe@kernel.dk>

---

Just a quick hack, works for me though in testing.

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 51bfd7737552..0a0d1c262701 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -92,6 +92,10 @@ static int g_submit_queues = 1;
 module_param_named(submit_queues, g_submit_queues, int, 0444);
 MODULE_PARM_DESC(submit_queues, "Number of submission queues");
 
+static int g_poll_queues = 1;
+module_param_named(poll_queues, g_poll_queues, int, 0444);
+MODULE_PARM_DESC(poll_queues, "Number of IOPOLL submission queues");
+
 static int g_home_node = NUMA_NO_NODE;
 module_param_named(home_node, g_home_node, int, 0444);
 MODULE_PARM_DESC(home_node, "Home node for the device");
@@ -347,6 +351,7 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
 NULLB_DEVICE_ATTR(size, ulong, NULL);
 NULLB_DEVICE_ATTR(completion_nsec, ulong, NULL);
 NULLB_DEVICE_ATTR(submit_queues, uint, nullb_apply_submit_queues);
+NULLB_DEVICE_ATTR(poll_queues, uint, nullb_apply_submit_queues);
 NULLB_DEVICE_ATTR(home_node, uint, NULL);
 NULLB_DEVICE_ATTR(queue_mode, uint, NULL);
 NULLB_DEVICE_ATTR(blocksize, uint, NULL);
@@ -465,6 +470,7 @@ static struct configfs_attribute *nullb_device_attrs[] = {
 	&nullb_device_attr_size,
 	&nullb_device_attr_completion_nsec,
 	&nullb_device_attr_submit_queues,
+	&nullb_device_attr_poll_queues,
 	&nullb_device_attr_home_node,
 	&nullb_device_attr_queue_mode,
 	&nullb_device_attr_blocksize,
@@ -591,6 +597,7 @@ static struct nullb_device *null_alloc_dev(void)
 	dev->size = g_gb * 1024;
 	dev->completion_nsec = g_completion_nsec;
 	dev->submit_queues = g_submit_queues;
+	dev->poll_queues = g_poll_queues;
 	dev->home_node = g_home_node;
 	dev->queue_mode = g_queue_mode;
 	dev->blocksize = g_bs;
@@ -1452,12 +1459,74 @@ static bool should_requeue_request(struct request *rq)
 	return false;
 }
 
+static int null_map_queues(struct blk_mq_tag_set *set)
+{
+	struct nullb *nullb = set->driver_data;
+	int i, qoff;
+
+	for (i = 0, qoff = 0; i < set->nr_maps; i++) {
+		struct blk_mq_queue_map *map = &set->map[i];
+
+		switch (i) {
+		case HCTX_TYPE_DEFAULT:
+			map->nr_queues = nullb->dev->submit_queues;
+			break;
+		case HCTX_TYPE_READ:
+			map->nr_queues = 0;
+			continue;
+		case HCTX_TYPE_POLL:
+			map->nr_queues = nullb->dev->poll_queues;
+			break;
+		}
+		map->queue_offset = qoff;
+		qoff += map->nr_queues;
+		blk_mq_map_queues(map);
+	}
+
+	return 0;
+}
+
+static int null_poll(struct blk_mq_hw_ctx *hctx)
+{
+	struct nullb_queue *nq = hctx->driver_data;
+	LIST_HEAD(list);
+	int nr = 0;
+
+	spin_lock(&nq->poll_lock);
+	list_splice_init(&nq->poll_list, &list);
+	spin_unlock(&nq->poll_lock);
+
+	while (!list_empty(&list)) {
+		struct nullb_cmd *cmd;
+		struct request *req;
+
+		req = list_first_entry(&list, struct request, queuelist);
+		list_del_init(&req->queuelist);
+		cmd = blk_mq_rq_to_pdu(req);
+		cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
+						blk_rq_sectors(req));
+		nullb_complete_cmd(cmd);
+		nr++;
+	}
+
+	return nr;
+}
+
 static enum blk_eh_timer_return null_timeout_rq(struct request *rq, bool res)
 {
+	struct blk_mq_hw_ctx *hctx = rq->mq_hctx;
 	struct nullb_cmd *cmd = blk_mq_rq_to_pdu(rq);
 
 	pr_info("rq %p timed out\n", rq);
 
+	if (hctx->type == HCTX_TYPE_POLL) {
+		struct nullb_queue *nq = hctx->driver_data;
+
+		spin_lock(&nq->poll_lock);
+		list_del_init(&rq->queuelist);
+		spin_unlock(&nq->poll_lock);
+	}
+
 	/*
 	 * If the device is marked as blocking (i.e. memory backed or zoned
 	 * device), the submission path may be blocked waiting for resources
@@ -1478,10 +1547,11 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
 	struct nullb_queue *nq = hctx->driver_data;
 	sector_t nr_sectors = blk_rq_sectors(bd->rq);
 	sector_t sector = blk_rq_pos(bd->rq);
+	const bool is_poll = hctx->type == HCTX_TYPE_POLL;
 
 	might_sleep_if(hctx->flags & BLK_MQ_F_BLOCKING);
 
-	if (nq->dev->irqmode == NULL_IRQ_TIMER) {
+	if (!is_poll && nq->dev->irqmode == NULL_IRQ_TIMER) {
 		hrtimer_init(&cmd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 		cmd->timer.function = null_cmd_timer_expired;
 	}
@@ -1505,6 +1575,13 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
 			return BLK_STS_OK;
 		}
 	}
+
+	if (is_poll) {
+		spin_lock(&nq->poll_lock);
+		list_add_tail(&bd->rq->queuelist, &nq->poll_list);
+		spin_unlock(&nq->poll_lock);
+		return BLK_STS_OK;
+	}
 	if (cmd->fake_timeout)
 		return BLK_STS_OK;
 
@@ -1540,6 +1617,8 @@ static void null_init_queue(struct nullb *nullb, struct nullb_queue *nq)
 	init_waitqueue_head(&nq->wait);
 	nq->queue_depth = nullb->queue_depth;
 	nq->dev = nullb->dev;
+	INIT_LIST_HEAD(&nq->poll_list);
+	spin_lock_init(&nq->poll_lock);
 }
 
 static int null_init_hctx(struct blk_mq_hw_ctx *hctx, void *driver_data,
@@ -1565,6 +1644,8 @@ static const struct blk_mq_ops null_mq_ops = {
 	.queue_rq       = null_queue_rq,
 	.complete	= null_complete_rq,
 	.timeout	= null_timeout_rq,
+	.poll		= null_poll,
+	.map_queues	= null_map_queues,
 	.init_hctx	= null_init_hctx,
 	.exit_hctx	= null_exit_hctx,
 };
@@ -1662,13 +1743,17 @@ static int setup_commands(struct nullb_queue *nq)
 
 static int setup_queues(struct nullb *nullb)
 {
-	nullb->queues = kcalloc(nr_cpu_ids, sizeof(struct nullb_queue),
+	int nqueues = nr_cpu_ids;
+
+	if (g_poll_queues)
+		nqueues *= 2;
+
+	nullb->queues = kcalloc(nqueues, sizeof(struct nullb_queue),
 				GFP_KERNEL);
 	if (!nullb->queues)
 		return -ENOMEM;
 
 	nullb->queue_depth = nullb->dev->hw_queue_depth;
-
 	return 0;
 }
 
@@ -1724,9 +1809,14 @@ static int null_gendisk_register(struct nullb *nullb)
 
 static int null_init_tag_set(struct nullb *nullb, struct blk_mq_tag_set *set)
 {
+	int poll_queues;
+
 	set->ops = &null_mq_ops;
 	set->nr_hw_queues = nullb ? nullb->dev->submit_queues :
 						g_submit_queues;
+	poll_queues = nullb ? nullb->dev->poll_queues : g_poll_queues;
+	if (poll_queues)
+		set->nr_hw_queues *= 2;
 	set->queue_depth = nullb ? nullb->dev->hw_queue_depth :
 						g_hw_queue_depth;
 	set->numa_node = nullb ? nullb->dev->home_node : g_home_node;
@@ -1736,7 +1826,11 @@ static int null_init_tag_set(struct nullb *nullb, struct blk_mq_tag_set *set)
 		set->flags |= BLK_MQ_F_NO_SCHED;
 	if (g_shared_tag_bitmap)
 		set->flags |= BLK_MQ_F_TAG_HCTX_SHARED;
-	set->driver_data = NULL;
+	set->driver_data = nullb;
+	if (g_poll_queues)
+		set->nr_maps = 3;
+	else
+		set->nr_maps = 1;
 
 	if ((nullb && nullb->dev->blocking) || g_blocking)
 		set->flags |= BLK_MQ_F_BLOCKING;
diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index 4876d5adb12d..ca16468176aa 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h
@@ -32,6 +32,9 @@ struct nullb_queue {
 	struct nullb_device *dev;
 	unsigned int requeue_selection;
 
+	struct list_head poll_list;
+	spinlock_t poll_lock;
+
 	struct nullb_cmd *cmds;
 };
 
@@ -83,6 +86,7 @@ struct nullb_device {
 	unsigned int zone_max_open; /* max number of open zones */
 	unsigned int zone_max_active; /* max number of active zones */
 	unsigned int submit_queues; /* number of submission queues */
+	unsigned int poll_queues; /* number of IOPOLL submission queues */
 	unsigned int home_node; /* home node for the device */
 	unsigned int queue_mode; /* block interface */
 	unsigned int blocksize; /* block size */

-- 
Jens Axboe


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

* Re: [PATCH] null_blk: poll queue support
  2021-04-17 15:29 [PATCH] null_blk: poll queue support Jens Axboe
@ 2021-04-18  4:48 ` Chaitanya Kulkarni
  2021-04-18  4:49 ` Chaitanya Kulkarni
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Chaitanya Kulkarni @ 2021-04-18  4:48 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

On 4/17/21 08:30, Jens Axboe wrote:
> There's currently no way to experiment with polled IO with null_blk,
> which seems like an oversight. This patch adds support for polled IO.
> We keep a list of issued IOs on submit, and then process that list
> when mq_ops->poll() is invoked.
>
> A new parameter is added, poll_queues. It defaults to 1 like the
> submit queues, meaning we'll have 1 poll queue available.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>


Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>




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

* Re: [PATCH] null_blk: poll queue support
  2021-04-17 15:29 [PATCH] null_blk: poll queue support Jens Axboe
  2021-04-18  4:48 ` Chaitanya Kulkarni
@ 2021-04-18  4:49 ` Chaitanya Kulkarni
  2021-04-19 19:48   ` Jens Axboe
  2021-05-03 15:47 ` Bart Van Assche
  2021-09-14 21:38 ` Pavel Begunkov
  3 siblings, 1 reply; 12+ messages in thread
From: Chaitanya Kulkarni @ 2021-04-18  4:49 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

On 4/17/21 08:30, Jens Axboe wrote:
> +		cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
> +						blk_rq_sectors(req));

How about following on the top of this patch ?

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 8efaf21cc053..4c27e37ccc51 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1496,6 +1496,7 @@ static int null_map_queues(struct blk_mq_tag_set *set)
 static int null_poll(struct blk_mq_hw_ctx *hctx)
 {
        struct nullb_queue *nq = hctx->driver_data;
+       blk_status_t sts;
        LIST_HEAD(list);
        int nr = 0;
 
@@ -1510,8 +1511,16 @@ static int null_poll(struct blk_mq_hw_ctx *hctx)
                req = list_first_entry(&list, struct request, queuelist);
                list_del_init(&req->queuelist);
                cmd = blk_mq_rq_to_pdu(req);
-               cmd->error = null_process_cmd(cmd, req_op(req),
blk_rq_pos(req),
-                                               blk_rq_sectors(req));
+               if (cmd->nq->dev->zoned)
+                       sts = null_process_zoned_cmd(cmd, req_op(req),
+                                                    blk_rq_pos(req),
+                                                    blk_rq_sectors(req));
+               else
+                       sts = null_process_cmd(cmd, req_op(req),
blk_rq_pos(req),
+                                              blk_rq_sectors(req));
+
+               cmd->error = sts;
+
                nullb_complete_cmd(cmd);
                nr++;
        }

If you are okay I can send a well tested patch with little bit code
cleanup once this is in the tree.



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

* Re: [PATCH] null_blk: poll queue support
  2021-04-18  4:49 ` Chaitanya Kulkarni
@ 2021-04-19 19:48   ` Jens Axboe
  2021-04-19 22:19     ` Chaitanya Kulkarni
  0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2021-04-19 19:48 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-block

On 4/17/21 10:49 PM, Chaitanya Kulkarni wrote:
> On 4/17/21 08:30, Jens Axboe wrote:
>> +		cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
>> +						blk_rq_sectors(req));
> 
> How about following on the top of this patch ?
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 8efaf21cc053..4c27e37ccc51 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1496,6 +1496,7 @@ static int null_map_queues(struct blk_mq_tag_set *set)
>  static int null_poll(struct blk_mq_hw_ctx *hctx)
>  {
>         struct nullb_queue *nq = hctx->driver_data;
> +       blk_status_t sts;
>         LIST_HEAD(list);
>         int nr = 0;
>  
> @@ -1510,8 +1511,16 @@ static int null_poll(struct blk_mq_hw_ctx *hctx)
>                 req = list_first_entry(&list, struct request, queuelist);
>                 list_del_init(&req->queuelist);
>                 cmd = blk_mq_rq_to_pdu(req);
> -               cmd->error = null_process_cmd(cmd, req_op(req),
> blk_rq_pos(req),
> -                                               blk_rq_sectors(req));
> +               if (cmd->nq->dev->zoned)
> +                       sts = null_process_zoned_cmd(cmd, req_op(req),
> +                                                    blk_rq_pos(req),
> +                                                    blk_rq_sectors(req));
> +               else
> +                       sts = null_process_cmd(cmd, req_op(req),
> blk_rq_pos(req),
> +                                              blk_rq_sectors(req));
> +
> +               cmd->error = sts;
> +
>                 nullb_complete_cmd(cmd);
>                 nr++;
>         }
> 
> If you are okay I can send a well tested patch with little bit code
> cleanup once this is in the tree.

Yes, that might be a good idea. I'll just fold it in, I've got it
sitting separately so far. Just let me know when you've tested it.

-- 
Jens Axboe


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

* Re: [PATCH] null_blk: poll queue support
  2021-04-19 19:48   ` Jens Axboe
@ 2021-04-19 22:19     ` Chaitanya Kulkarni
  2021-04-19 22:55       ` Jens Axboe
  0 siblings, 1 reply; 12+ messages in thread
From: Chaitanya Kulkarni @ 2021-04-19 22:19 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

Jens,

On 4/19/21 12:48, Jens Axboe wrote:
>> +
>> +               cmd->error = sts;
>> +
>>                 nullb_complete_cmd(cmd);
>>                 nr++;
>>         }
>>
>> If you are okay I can send a well tested patch with little bit code
>> cleanup once this is in the tree.
> Yes, that might be a good idea. I'll just fold it in, I've got it
> sitting separately so far. Just let me know when you've tested it.
>
> -- Jens Axboe

I'm OOO, won't be able to do the testing for couple of days.

Meanwhile if you apply your patch, I'll send ZBD patch with testing
in couple of days.



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

* Re: [PATCH] null_blk: poll queue support
  2021-04-19 22:19     ` Chaitanya Kulkarni
@ 2021-04-19 22:55       ` Jens Axboe
  0 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2021-04-19 22:55 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-block

On 4/19/21 4:19 PM, Chaitanya Kulkarni wrote:
> Jens,
> 
> On 4/19/21 12:48, Jens Axboe wrote:
>>> +
>>> +               cmd->error = sts;
>>> +
>>>                 nullb_complete_cmd(cmd);
>>>                 nr++;
>>>         }
>>>
>>> If you are okay I can send a well tested patch with little bit code
>>> cleanup once this is in the tree.
>> Yes, that might be a good idea. I'll just fold it in, I've got it
>> sitting separately so far. Just let me know when you've tested it.
>>
>> -- Jens Axboe
> 
> I'm OOO, won't be able to do the testing for couple of days.
> 
> Meanwhile if you apply your patch, I'll send ZBD patch with testing
> in couple of days.

OK that's fine, we can just do a separate patch, or fold in as needed.

-- 
Jens Axboe


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

* Re: [PATCH] null_blk: poll queue support
  2021-04-17 15:29 [PATCH] null_blk: poll queue support Jens Axboe
  2021-04-18  4:48 ` Chaitanya Kulkarni
  2021-04-18  4:49 ` Chaitanya Kulkarni
@ 2021-05-03 15:47 ` Bart Van Assche
  2021-05-03 17:42   ` Jens Axboe
  2021-09-14 21:38 ` Pavel Begunkov
  3 siblings, 1 reply; 12+ messages in thread
From: Bart Van Assche @ 2021-05-03 15:47 UTC (permalink / raw)
  To: Jens Axboe, linux-block

On 4/17/21 8:29 AM, Jens Axboe wrote:
> There's currently no way to experiment with polled IO with null_blk,
> which seems like an oversight. This patch adds support for polled IO.
> We keep a list of issued IOs on submit, and then process that list
> when mq_ops->poll() is invoked.
> 
> A new parameter is added, poll_queues. It defaults to 1 like the
> submit queues, meaning we'll have 1 poll queue available.

Has anyone run blktests against blk-for-next since this patch got
queued? The following appears in the kernel log if I run blktests:

root[5563]: run blktests block/010
null_blk: module loaded
==================================================================
BUG: KASAN: null-ptr-deref in null_map_queues+0x131/0x1a0 [null_blk]
Read of size 8 at addr 0000000000000000 by task modprobe/5612

CPU: 7 PID: 5612 Comm: modprobe Not tainted 5.12.0-dbg+ #33
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a-rebuilt.opensuse.org 04/01/2014
Call Trace:
 show_stack+0x52/0x58
 dump_stack+0x9d/0xcf
 kasan_report.cold+0x4b/0x50
 __asan_load8+0x69/0x90
 null_map_queues+0x131/0x1a0 [null_blk]
 blk_mq_update_queue_map+0x122/0x1a0
 blk_mq_alloc_tag_set+0x1c8/0x480
 null_init_tag_set+0x19c/0x220 [null_blk]
 null_init+0x1ac/0x1000 [null_blk]
 do_one_initcall+0xd3/0x460
 do_init_module+0x10a/0x400
 load_module+0xbe2/0xc50
 __do_sys_finit_module+0x131/0x1c0
 __x64_sys_finit_module+0x43/0x50
 do_syscall_64+0x3a/0xb0
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f51f7fd367d
Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d bb f7 0c 00 f7 d8 64 89 01 48
RSP: 002b:00007ffd4a668728 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
RAX: ffffffffffffffda RBX: 00005609718ddbe0 RCX: 00007f51f7fd367d
RDX: 0000000000000000 RSI: 00005609718ddfe0 RDI: 0000000000000003
RBP: 0000000000040000 R08: 0000000000000000 R09: 000000000000003a
R10: 0000000000000003 R11: 0000000000000246 R12: 00005609718ddfe0
R13: 0000000000000000 R14: 00005609718ddd70 R15: 00005609718ddbe0
==================================================================

Thanks,

Bart.

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

* Re: [PATCH] null_blk: poll queue support
  2021-05-03 15:47 ` Bart Van Assche
@ 2021-05-03 17:42   ` Jens Axboe
  2021-05-04  0:32     ` Bart Van Assche
  0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2021-05-03 17:42 UTC (permalink / raw)
  To: Bart Van Assche, linux-block

On 5/3/21 9:47 AM, Bart Van Assche wrote:
> On 4/17/21 8:29 AM, Jens Axboe wrote:
>> There's currently no way to experiment with polled IO with null_blk,
>> which seems like an oversight. This patch adds support for polled IO.
>> We keep a list of issued IOs on submit, and then process that list
>> when mq_ops->poll() is invoked.
>>
>> A new parameter is added, poll_queues. It defaults to 1 like the
>> submit queues, meaning we'll have 1 poll queue available.
> 
> Has anyone run blktests against blk-for-next since this patch got
> queued? The following appears in the kernel log if I run blktests:

Do you know what parameters the module was loaded with? I'll drop this
one from the 5.13 queue for now.

-- 
Jens Axboe


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

* Re: [PATCH] null_blk: poll queue support
  2021-05-03 17:42   ` Jens Axboe
@ 2021-05-04  0:32     ` Bart Van Assche
  0 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2021-05-04  0:32 UTC (permalink / raw)
  To: Jens Axboe, linux-block

On 5/3/21 10:42 AM, Jens Axboe wrote:
> On 5/3/21 9:47 AM, Bart Van Assche wrote:
>> On 4/17/21 8:29 AM, Jens Axboe wrote:
>>> There's currently no way to experiment with polled IO with null_blk,
>>> which seems like an oversight. This patch adds support for polled IO.
>>> We keep a list of issued IOs on submit, and then process that list
>>> when mq_ops->poll() is invoked.
>>>
>>> A new parameter is added, poll_queues. It defaults to 1 like the
>>> submit queues, meaning we'll have 1 poll queue available.
>>
>> Has anyone run blktests against blk-for-next since this patch got
>> queued? The following appears in the kernel log if I run blktests:
> 
> Do you know what parameters the module was loaded with? I'll drop this
> one from the 5.13 queue for now.

Hi Jens,

The call trace was reported about 35 seconds after the message
indicating the start of the test so I assume that the null_blk driver
was loaded by the following shell code from tests/block/010:

	if ! _init_null_blk queue_mode=2 submit_queues=16 nr_devices=32 shared_tags=1; then
		return 1
	fi

Bart.



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

* Re: [PATCH] null_blk: poll queue support
  2021-04-17 15:29 [PATCH] null_blk: poll queue support Jens Axboe
                   ` (2 preceding siblings ...)
  2021-05-03 15:47 ` Bart Van Assche
@ 2021-09-14 21:38 ` Pavel Begunkov
  2021-09-16  1:57   ` Jens Axboe
  3 siblings, 1 reply; 12+ messages in thread
From: Pavel Begunkov @ 2021-09-14 21:38 UTC (permalink / raw)
  To: Jens Axboe, linux-block

On 4/17/21 4:29 PM, Jens Axboe wrote:
> There's currently no way to experiment with polled IO with null_blk,
> which seems like an oversight. This patch adds support for polled IO.
> We keep a list of issued IOs on submit, and then process that list
> when mq_ops->poll() is invoked.

That would be pretty useful to have.

to fold in:

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 5914fed8fa56..eb5cfe189e90 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1508,7 +1508,7 @@ static int null_poll(struct blk_mq_hw_ctx *hctx)
 		cmd = blk_mq_rq_to_pdu(req);
 		cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
 						blk_rq_sectors(req));
-		nullb_complete_cmd(cmd);
+		end_cmd(cmd);
 		nr++;
 	}
 
> 
> A new parameter is added, poll_queues. It defaults to 1 like the
> submit queues, meaning we'll have 1 poll queue available.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> ---

-- 
Pavel Begunkov

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

* Re: [PATCH] null_blk: poll queue support
  2021-09-14 21:38 ` Pavel Begunkov
@ 2021-09-16  1:57   ` Jens Axboe
  2021-09-20 11:40     ` Pavel Begunkov
  0 siblings, 1 reply; 12+ messages in thread
From: Jens Axboe @ 2021-09-16  1:57 UTC (permalink / raw)
  To: Pavel Begunkov, linux-block

On 9/14/21 3:38 PM, Pavel Begunkov wrote:
> On 4/17/21 4:29 PM, Jens Axboe wrote:
>> There's currently no way to experiment with polled IO with null_blk,
>> which seems like an oversight. This patch adds support for polled IO.
>> We keep a list of issued IOs on submit, and then process that list
>> when mq_ops->poll() is invoked.
> 
> That would be pretty useful to have.
> 
> to fold in:
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 5914fed8fa56..eb5cfe189e90 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1508,7 +1508,7 @@ static int null_poll(struct blk_mq_hw_ctx *hctx)
>  		cmd = blk_mq_rq_to_pdu(req);
>  		cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
>  						blk_rq_sectors(req));
> -		nullb_complete_cmd(cmd);
> +		end_cmd(cmd);
>  		nr++;
>  	}

Let's try again with that...

-- 
Jens Axboe


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

* Re: [PATCH] null_blk: poll queue support
  2021-09-16  1:57   ` Jens Axboe
@ 2021-09-20 11:40     ` Pavel Begunkov
  0 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-09-20 11:40 UTC (permalink / raw)
  To: Jens Axboe, linux-block

On 9/16/21 2:57 AM, Jens Axboe wrote:
> On 9/14/21 3:38 PM, Pavel Begunkov wrote:
>> On 4/17/21 4:29 PM, Jens Axboe wrote:
>>> There's currently no way to experiment with polled IO with null_blk,
>>> which seems like an oversight. This patch adds support for polled IO.
>>> We keep a list of issued IOs on submit, and then process that list
>>> when mq_ops->poll() is invoked.
>>
>> That would be pretty useful to have.
>>
>> to fold in:
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>> index 5914fed8fa56..eb5cfe189e90 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -1508,7 +1508,7 @@ static int null_poll(struct blk_mq_hw_ctx *hctx)
>>  		cmd = blk_mq_rq_to_pdu(req);
>>  		cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
>>  						blk_rq_sectors(req));
>> -		nullb_complete_cmd(cmd);
>> +		end_cmd(cmd);
>>  		nr++;
>>  	}
> 
> Let's try again with that...

I'm not so sure it fixes the problem Bart mentioned, but at least it
doesn't crash for me right away (timer mode) and io_uring tests complete
well.

-- 
Pavel Begunkov

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

end of thread, other threads:[~2021-09-20 11:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-17 15:29 [PATCH] null_blk: poll queue support Jens Axboe
2021-04-18  4:48 ` Chaitanya Kulkarni
2021-04-18  4:49 ` Chaitanya Kulkarni
2021-04-19 19:48   ` Jens Axboe
2021-04-19 22:19     ` Chaitanya Kulkarni
2021-04-19 22:55       ` Jens Axboe
2021-05-03 15:47 ` Bart Van Assche
2021-05-03 17:42   ` Jens Axboe
2021-05-04  0:32     ` Bart Van Assche
2021-09-14 21:38 ` Pavel Begunkov
2021-09-16  1:57   ` Jens Axboe
2021-09-20 11:40     ` Pavel Begunkov

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).