Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: axboe@kernel.dk (Jens Axboe)
Subject: [PATCH] nvme-pci: only set nr_maps to 2 if poll queues are supported
Date: Fri, 14 Dec 2018 12:36:02 -0700	[thread overview]
Message-ID: <dad8c698-cac2-8523-de9f-bfee87ff166b@kernel.dk> (raw)
In-Reply-To: <a136f3b0-0be4-ef22-d224-3ca3ae22cf54@grimberg.me>

On 12/14/18 12:01 PM, Sagi Grimberg wrote:
> 
>>>> The block layer now enables polling support on a queue if nr_maps
>>>> includes the poll map, so we should only set that if we actually
>>>> support poll queues.
>>>>
>>>> Fixes:  6544d229bf ("block: enable polling by default if a poll map is initalized")
>>>
>>> What does it fix? is there a bug when setting 3 queue maps?
>>
>> After the recent changes, blk_mq_init_allocated_queue() does this:
>>
>> if (set->nr_maps > HCTX_TYPE_POLL)
>> 	blk_queue_flag_set(QUEUE_FLAG_POLL, q);
>>
>> since the core doesn't have any notion of how many queues are used for
>> what. We only support polling if you have explicit poll queues now,
>> hence the above is wrong if nvme says nr_maps is 3 and it doesn't have
>> any poll queues.
> 
> Yea, but its not broken because the map shares the default queue map
> right? Just wanted to understand if something is broken rather than just
> wrong.

It's broken since we say we support polling by setting the QUEUE_FLAG_POLL
flag, but we do not. It's not broken in terms of maps on the nvme side.
As you mention, the map sharing means it's perfectly legit to not have
special queues for each map. Polling is just special since we absolutely
do require special poll queues without interrupts. It's not like reads
vs writes, where we can share and nobody cares.

-- 
Jens Axboe

  reply	other threads:[~2018-12-14 19:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-14 16:20 [PATCH] nvme-pci: only set nr_maps to 2 if poll queues are supported Christoph Hellwig
2018-12-14 16:47 ` Jens Axboe
2018-12-14 16:49   ` Christoph Hellwig
2018-12-14 16:49     ` Jens Axboe
2018-12-14 18:43 ` Sagi Grimberg
2018-12-14 18:50   ` Jens Axboe
2018-12-14 19:01     ` Sagi Grimberg
2018-12-14 19:36       ` Jens Axboe [this message]
2018-12-14 19:49         ` Sagi Grimberg
2018-12-15  1:14 ` Sagi Grimberg
  -- strict thread matches above, loose matches on Subject: below --
2018-12-14 15:42 Christoph Hellwig
2018-12-14 15:45 ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dad8c698-cac2-8523-de9f-bfee87ff166b@kernel.dk \
    --to=axboe@kernel.dk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox