linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Damien Le Moal <dlemoal@kernel.org>
To: Kamaljit Singh <Kamaljit.Singh1@wdc.com>,
	"kbusch@kernel.org" <kbusch@kernel.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>, hch <hch@lst.de>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "cassel@kernel.org" <cassel@kernel.org>
Subject: Re: [PATCH v2 3/3] nvme: prevent ioq creation for discovery controllers
Date: Thu, 3 Jul 2025 08:59:47 +0900	[thread overview]
Message-ID: <63310a08-e01f-464d-9f46-54bb817f2121@kernel.org> (raw)
In-Reply-To: <MN2PR04MB68626BDAC087B4CC8CC1A1A9BC40A@MN2PR04MB6862.namprd04.prod.outlook.com>

On 7/3/25 3:09 AM, Kamaljit Singh wrote:
> Hi Damien,
>  
> On 7/1/25 19:18, Damien Le Moal wrote:
>>>   /*
>>> - * An admin controller has one admin queue, but no I/O queues.
>>> + * An admin or discovery controller has one admin queue, but no I/O queues.
>>>    * Override queue_count so it only creates an admin queue.
>>>    */
>>>   void nvme_override_prohibited_io_queues(struct nvme_ctrl *ctrl)
>>>   {
>>> -     if (nvme_admin_ctrl(ctrl))
>>> +     if (nvme_admin_ctrl(ctrl) || nvme_discovery_ctrl(ctrl))
>>>                ctrl->queue_count = 1;
>>>   }
>>>   EXPORT_SYMBOL_GPL(nvme_override_prohibited_io_queues);
>>
>> Repeating comment on patch 1. Can't we do this in nvme_init_subsystem() or may
>> be better, in nvme_set_queue_count() or nvme_init_ctrl_finish() ?
> nvme_set_queue_count() won’t even be called in this case, as its only used to
> configure IO queues, which are not being configured for an admin controller.
>  
> If we move nvme_override_prohibited_io_queues() up the stack into either of your
> suggested core.c functions, it will affect apple & fc drivers, which I don’t have any
> means of testing. Any suggestions?

Why would the controller vendor or its fabric matter ?
If the controller type is admin or discovery, you want queue_count set to 1. So
I do not think there is any problem.


-- 
Damien Le Moal
Western Digital Research


  reply	other threads:[~2025-07-03  0:02 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-02  0:58 [PATCH v2 0/3] Support for Administrative Controllers Kamaljit Singh
2025-07-02  0:58 ` [PATCH v2 1/3] nvme: add capability to connect to an admin controller Kamaljit Singh
2025-07-02  2:11   ` Damien Le Moal
2025-07-02 18:39     ` Kamaljit Singh
2025-07-03  9:04       ` hch
2025-07-03  8:55   ` Hannes Reinecke
2025-07-16 21:27     ` Kamaljit Singh
2025-07-02  0:58 ` [PATCH v2 2/3] nvme: prevent admin controller from smart log fetch (LID 2) Kamaljit Singh
2025-07-02  2:13   ` Damien Le Moal
2025-07-02 18:21     ` Kamaljit Singh
2025-07-03  8:56   ` Hannes Reinecke
2025-07-03  9:05   ` Christoph Hellwig
2025-07-02  0:58 ` [PATCH v2 3/3] nvme: prevent ioq creation for discovery controllers Kamaljit Singh
2025-07-02  2:18   ` Damien Le Moal
2025-07-02 18:09     ` Kamaljit Singh
2025-07-02 23:59       ` Damien Le Moal [this message]
2025-07-03  9:05         ` hch
2025-07-03  9:42   ` Hannes Reinecke

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=63310a08-e01f-464d-9f46-54bb817f2121@kernel.org \
    --to=dlemoal@kernel.org \
    --cc=Kamaljit.Singh1@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=cassel@kernel.org \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /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;
as well as URLs for NNTP newsgroup(s).