From: "Heyne, Maximilian" <mheyne@amazon.de>
To: Keith Busch <kbusch@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
"Sagi Grimberg" <sagi@grimberg.me>,
"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] nvme: Let the blocklayer set timeouts for requests
Date: Tue, 2 Dec 2025 17:51:36 +0000 [thread overview]
Message-ID: <20251202-78-chops-11b0312d@mheyne-amazon> (raw)
In-Reply-To: <aS8kP-bfOmZ-3W9E@kbusch-mbp>
On Tue, Dec 02, 2025 at 10:39:11AM -0700, Keith Busch wrote:
> On Tue, Dec 02, 2025 at 01:58:19PM +0000, Heyne, Maximilian wrote:
> > When initializing an nvme request which is about to be send to the block
> > layer, we do not need to initialize its timeout. If it's left
> > uninitialized at 0 the block layer will use the request queue's timeout
> > in blk_add_timer (via nvme_start_request which is called from
> > nvme_*_queue_rq). These timeouts are setup to either NVME_IO_TIMEOUT or
> > NVME_ADMIN_TIMEOUT when the request queues were created.
> >
> > Because the io_timeout of the IO queues can actually be modified via
> > sysfs, the following situation can occur:
> >
> > 1) NVME_IO_TIMEOUT = 30 (default module parameter)
> > 2) nvme1n1 is probed. IO queues default timeout is 30 s
> > 3) manually change the IO timeout to 90 s
> > echo 90000 > /sys/class/nvme/nvme1/nvme1n1/queue/io_timeout
> > 4) nvme zns report-zones /dev/nvme1n1
> > This command issues IO commands with timeout 30 s instead of the
> > wanted 90 s which might be more suitable for this device.
>
> Does this example really use 30s, though? User space commands should be
> going through nvme_submit_user_cmd(), which overrides the timeout set
> from the nvme_init_request with whatever the user requested (usually 0).
You're right. I actually worked on multiple (older) kernel versions and
forgot about this case. It was actually commit 470e900c8036ff ("nvme:
refactor nvme_alloc_request") which subtly changed the behavior but only
for the ioctl case. So ioctl's are fine then but, for example,
everything which goes via nvme_submit_sync_cmd shows the issue. So we
need to update the commit message accordingly. Sorry for that. I'll give
it a day or two for further comments on this patch and then send it with
a more correct message.
Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Christof Hellmis
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
prev parent reply other threads:[~2025-12-02 17:52 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-02 13:58 Heyne, Maximilian
2025-12-02 17:39 ` Keith Busch
2025-12-02 17:51 ` Heyne, Maximilian [this message]
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=20251202-78-chops-11b0312d@mheyne-amazon \
--to=mheyne@amazon.de \
--cc=axboe@kernel.dk \
--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
all inboxes | Powered by JetHome®