All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Maurizio Lombardi" <mlombard@arkamax.eu>
To: "Mohamed Khalfella" <mkhalfella@purestorage.com>,
	"Maurizio Lombardi" <mlombard@redhat.com>
Cc: <kbusch@kernel.org>, <mheyne@amazon.de>, <emilne@redhat.com>,
	<jmeneghi@redhat.com>, <linux-nvme@lists.infradead.org>,
	<dwagner@suse.de>, <mlombard@arkamax.eu>, <chaitanyak@nvidia.com>,
	<hare@kernel.org>, <hch@lst.de>
Subject: Re: [PATCH V5 3/7] nvme: add sysfs attribute to change admin timeout per nvme controller
Date: Fri, 15 May 2026 07:54:03 +0200	[thread overview]
Message-ID: <DIJ0N00IF49P.2C9YI5U1FE9FG@arkamax.eu> (raw)
In-Reply-To: <20260514165929.GJ10532-mkhalfella@purestorage.com>

On Thu May 14, 2026 at 6:59 PM CEST, Mohamed Khalfella wrote:
> On Thu 2026-05-14 10:32:51 +0200, Maurizio Lombardi wrote:
>> Currently, there is no method to adjust the timeout values on a
>> per-controller basis with nvme admin queues.
>> Add an admin_timeout attribute to nvme so that different nvme controllers
>> which may have different timeout requirements can have custom admin
>> timeouts set.
>> 
>> The admin timeout is also applied to the fabrics queue (fabrics_q).
>> The fabrics queue is utilized for fabric-specific administrative and
>> control operations, such as Connect and Property Get/Set commands.
>> 
>> Reviewed-by: Daniel Wagner <dwagner@suse.de>
>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
>
> Reviewed-by: Mohamed Khalfella <mkhalfella@purestorage.com>

Thanks for the review.

>
>> ---
>>  drivers/nvme/host/core.c  |  1 +
>>  drivers/nvme/host/nvme.h  |  1 +
>>  drivers/nvme/host/pci.c   |  2 +-
>>  drivers/nvme/host/sysfs.c | 41 +++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 44 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 84f295e3bf08..fe6dcd19cecb 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -5144,6 +5144,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
>>  	memset(&ctrl->ka_cmd, 0, sizeof(ctrl->ka_cmd));
>>  	ctrl->ka_cmd.common.opcode = nvme_admin_keep_alive;
>>  	ctrl->ka_last_check_time = jiffies;
>> +	ctrl->admin_timeout = NVME_ADMIN_TIMEOUT;
>>  
>>  	BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >
>>  			PAGE_SIZE);
>
> nvme_alloc_admin_tag_set() uses NVME_ADMIN_TIMEOUT. Do we want to
> replace that with ctrl->admin_timeout?
>

That shouldn't be necessary. nvme_alloc_admin_tag_set() is only called
once during initial controller setup. Since the user cannot modify the
admin_timeout sysfs attribute until the controller reaches the LIVE state,
ctrl->admin_timeout will still just be the default NVME_ADMIN_TIMEOUT
at this stage.

Maurizio


  reply	other threads:[~2026-05-15  5:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-14  8:32 [PATCH V5 0/7] nvme: Refactor and expose per-controller timeout configuration Maurizio Lombardi
2026-05-14  8:32 ` [PATCH V5 1/7] nvme: Let the blocklayer set timeouts for requests Maurizio Lombardi
2026-05-14  8:32 ` [PATCH V5 2/7] nvme: remove redundant timeout argument from nvme_wait_freeze_timeout Maurizio Lombardi
2026-05-15  4:30   ` Christoph Hellwig
2026-05-14  8:32 ` [PATCH V5 3/7] nvme: add sysfs attribute to change admin timeout per nvme controller Maurizio Lombardi
2026-05-14 16:59   ` Mohamed Khalfella
2026-05-15  5:54     ` Maurizio Lombardi [this message]
2026-05-15  4:30   ` Christoph Hellwig
2026-05-14  8:32 ` [PATCH V5 4/7] nvme: add sysfs attribute to change IO timeout per controller Maurizio Lombardi
2026-05-15  4:31   ` Christoph Hellwig
2026-05-14  8:32 ` [PATCH V5 5/7] nvme-core: align fabrics_q teardown with admin_q in nvme_free_ctrl Maurizio Lombardi
2026-05-14  8:32 ` [PATCH V5 6/7] nvmet-loop: do not alloc admin tag set during reset Maurizio Lombardi
2026-05-14  8:32 ` [PATCH V5 7/7] nvme-core: warn on allocating admin tag set with existing queue Maurizio Lombardi

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=DIJ0N00IF49P.2C9YI5U1FE9FG@arkamax.eu \
    --to=mlombard@arkamax.eu \
    --cc=chaitanyak@nvidia.com \
    --cc=dwagner@suse.de \
    --cc=emilne@redhat.com \
    --cc=hare@kernel.org \
    --cc=hch@lst.de \
    --cc=jmeneghi@redhat.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=mheyne@amazon.de \
    --cc=mkhalfella@purestorage.com \
    --cc=mlombard@redhat.com \
    /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 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.