[PATCH] blk-mq: enable IO poll if .nr_queues of type poll > 0

[PATCH] blk-mq: enable IO poll if .nr_queues of type poll > 0

[Ming Lei]

The queue mapping of type poll only exists when set->map[HCTX_TYPE_POLL].nr_queues
is bigger than zero, so enhance the constraint by checking .nr_queues of type poll
before enabling IO poll.

Otherwise IO race & timeout can be observed when running block/007.

Cc: Jeff Moyer <jmoyer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c    | 3 ++-
 block/blk-sysfs.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 6847f014606b..e45400da4180 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2833,7 +2833,8 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 	q->tag_set = set;
 
 	q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
-	if (set->nr_maps > HCTX_TYPE_POLL)
+	if (set->nr_maps > HCTX_TYPE_POLL &&
+	    set->map[HCTX_TYPE_POLL].nr_queues)
 		blk_queue_flag_set(QUEUE_FLAG_POLL, q);
 
 	if (!(set->flags & BLK_MQ_F_SG_MERGE))
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index bb7c642ffefa..0619c8922893 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -402,7 +402,8 @@ static ssize_t queue_poll_store(struct request_queue *q, const char *page,
 	unsigned long poll_on;
 	ssize_t ret;
 
-	if (!q->tag_set || q->tag_set->nr_maps <= HCTX_TYPE_POLL)
+	if (!q->tag_set || q->tag_set->nr_maps <= HCTX_TYPE_POLL ||
+	    !q->tag_set->map[HCTX_TYPE_POLL].nr_queues)
 		return -EINVAL;
 
 	ret = queue_var_store(&poll_on, page, count);
-- 
2.9.5

Re: [PATCH] blk-mq: enable IO poll if .nr_queues of type poll > 0

[Jens Axboe]

On 12/17/18 9:15 PM, Ming Lei wrote:
> The queue mapping of type poll only exists when set->map[HCTX_TYPE_POLL].nr_queues
> is bigger than zero, so enhance the constraint by checking .nr_queues of type poll
> before enabling IO poll.
> 
> Otherwise IO race & timeout can be observed when running block/007.

Looks good to me, but we might consider having a helper for this, since
we have this check in 3 places now. But for now this is good, thanks
Ming.

-- 
Jens Axboe

Re: [PATCH] blk-mq: enable IO poll if .nr_queues of type poll > 0

[Christoph Hellwig]

On Mon, Dec 17, 2018 at 09:34:55PM -0700, Jens Axboe wrote:
> On 12/17/18 9:15 PM, Ming Lei wrote:
> > The queue mapping of type poll only exists when set->map[HCTX_TYPE_POLL].nr_queues
> > is bigger than zero, so enhance the constraint by checking .nr_queues of type poll
> > before enabling IO poll.
> > 
> > Otherwise IO race & timeout can be observed when running block/007.
> 
> Looks good to me, but we might consider having a helper for this, since
> we have this check in 3 places now. But for now this is good, thanks
> Ming.

Jens, if you want this I can drop the nvme patch to set nr_maps to 2
as it wouldn't be needed.  Not sure which one is more elegant, but
I guess Mings version is a little more fool proof.

Re: [PATCH] blk-mq: enable IO poll if .nr_queues of type poll > 0

[Ming Lei]

On Tue, Dec 18, 2018 at 07:47:39AM +0100, Christoph Hellwig wrote:
> On Mon, Dec 17, 2018 at 09:34:55PM -0700, Jens Axboe wrote:
> > On 12/17/18 9:15 PM, Ming Lei wrote:
> > > The queue mapping of type poll only exists when set->map[HCTX_TYPE_POLL].nr_queues
> > > is bigger than zero, so enhance the constraint by checking .nr_queues of type poll
> > > before enabling IO poll.
> > > 
> > > Otherwise IO race & timeout can be observed when running block/007.
> > 
> > Looks good to me, but we might consider having a helper for this, since
> > we have this check in 3 places now. But for now this is good, thanks
> > Ming.
> 
> Jens, if you want this I can drop the nvme patch to set nr_maps to 2
> as it wouldn't be needed.  Not sure which one is more elegant, but
> I guess Mings version is a little more fool proof.

For READ queue type, its .nr_maps can be 3 and .nr_queues is zero, so
if check on the two queue type is kept as consistent, we may get good
code readability, IMO.

As Jens mentioned, we may introduce blk_mq_queue_type_supported() helper
to cover both.

Thanks,
Ming

Re: [PATCH] blk-mq: enable IO poll if .nr_queues of type poll > 0

[Jens Axboe]

On 12/18/18 1:09 AM, Ming Lei wrote:
> On Tue, Dec 18, 2018 at 07:47:39AM +0100, Christoph Hellwig wrote:
>> On Mon, Dec 17, 2018 at 09:34:55PM -0700, Jens Axboe wrote:
>>> On 12/17/18 9:15 PM, Ming Lei wrote:
>>>> The queue mapping of type poll only exists when set->map[HCTX_TYPE_POLL].nr_queues
>>>> is bigger than zero, so enhance the constraint by checking .nr_queues of type poll
>>>> before enabling IO poll.
>>>>
>>>> Otherwise IO race & timeout can be observed when running block/007.
>>>
>>> Looks good to me, but we might consider having a helper for this, since
>>> we have this check in 3 places now. But for now this is good, thanks
>>> Ming.
>>
>> Jens, if you want this I can drop the nvme patch to set nr_maps to 2
>> as it wouldn't be needed.  Not sure which one is more elegant, but
>> I guess Mings version is a little more fool proof.
> 
> For READ queue type, its .nr_maps can be 3 and .nr_queues is zero, so
> if check on the two queue type is kept as consistent, we may get good
> code readability, IMO.
> 
> As Jens mentioned, we may introduce blk_mq_queue_type_supported() helper
> to cover both.

I think that would be preferable.

-- 
Jens Axboe