Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mohamed Khalfella <mkhalfella@purestorage.com>
To: Sagi Grimberg <sagi@grimberg.me>
Cc: Daniel Wagner <dwagner@suse.de>, Daniel Wagner <wagi@kernel.org>,
	Christoph Hellwig <hch@lst.de>, Keith Busch <kbusch@kernel.org>,
	Hannes Reinecke <hare@suse.de>,
	John Meneghini <jmeneghi@redhat.com>,
	randyj@purestorage.com, linux-nvme@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC 3/3] nvme: delay failover by command quiesce timeout
Date: Wed, 16 Apr 2025 15:59:13 -0700	[thread overview]
Message-ID: <20250416225913.GA2476975-mkhalfella@purestorage.com> (raw)
In-Reply-To: <df6cd26e-551a-4bc1-bdc6-9c715e502aa8@grimberg.me>

On 2025-04-17 01:21:08 +0300, Sagi Grimberg wrote:
> 
> 
> On 16/04/2025 16:53, Mohamed Khalfella wrote:
> > On 2025-04-16 10:30:11 +0200, Daniel Wagner wrote:
> >> On Tue, Apr 15, 2025 at 05:40:16PM -0700, Mohamed Khalfella wrote:
> >>> On 2025-04-15 14:17:48 +0200, Daniel Wagner wrote:
> >>>> Pasthrough commands should fail immediately. Userland is in charge here,
> >>>> not the kernel. At least this what should happen here.
> >>> I see your point. Unless I am missing something these requests should be
> >>> held equally to bio requests from multipath layer. Let us say app
> >>> submitted write a request that got canceled immediately, how does the app
> >>> know when it is safe to retry the write request?
> >> Good question, but nothing new as far I can tell. If the kernel doesn't
> >> start to retry passthru IO commands, we have to figure out how to pass
> >> additional information to the userland.
> >>
> > nvme multipath does not retry passthru commands. That is said, there is
> > nothing prevents userspace from retrying canceled command immediately
> > resulting in the unwanted behavior these very patches try to address.
> 
> userspace can read the controller cqt and implement the retry logic on 
> its own.
> If it doesn't/can't, it should use normal fs io. the driver does not 
> handle passthru retries.

passthru requests are not very different from normal IO. If the driver
holds normal IO requests to prevent corruption, it should hold passthru
requests too, for the same reason, no?

IMO, keeping the request holding logic in the driver makes more sense
than implementing it in userspace. One reason is that CCR can help
release requests held requests faster.

> 
> >
> >>> Holding requests like write until it is safe to be retried is the whole
> >>> point of this work, right?
> >> My first goal was to address the IO commands submitted via the block
> >> layer. I didn't had the IO passthru interface on my radar. I agree,
> >> getting the IO passthru path correct is also good idea.
> > Okay. This will be addressed in the next revision, right?
> 
> I don't think it should. passthru IO requires the issuer to understand 
> the nvme
> device, and CQT falls under this definition.


  reply	other threads:[~2025-04-16 23:01 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-24 12:07 [PATCH RFC 0/3] nvme: add support for command quiesce timeout Daniel Wagner
2025-03-24 12:07 ` [PATCH RFC 1/3] nvmet: add command quiesce time Daniel Wagner
2025-04-01  9:33   ` Hannes Reinecke
2025-04-10  9:00   ` Mohamed Khalfella
2025-04-16 11:37     ` Daniel Wagner
2025-03-24 12:07 ` [PATCH RFC 2/3] nvme: store cqt value into nvme ctrl object Daniel Wagner
2025-04-01  9:34   ` Hannes Reinecke
2025-03-24 12:07 ` [PATCH RFC 3/3] nvme: delay failover by command quiesce timeout Daniel Wagner
2025-04-01  9:37   ` Hannes Reinecke
2025-04-15 12:00     ` Daniel Wagner
2025-04-01 13:32   ` Nilay Shroff
2025-04-15 12:05     ` Daniel Wagner
2025-04-10  8:51   ` Mohamed Khalfella
2025-04-14 22:28     ` Sagi Grimberg
2025-04-15 12:11       ` Daniel Wagner
2025-04-15 21:07         ` Sagi Grimberg
2025-04-15 23:02           ` Randy Jennings
2025-04-15 23:35             ` Sagi Grimberg
2025-04-15 23:57               ` Randy Jennings
2025-04-16 22:15                 ` Sagi Grimberg
2025-04-17  0:47                   ` Randy Jennings
2025-04-15 12:17     ` Daniel Wagner
2025-04-15 22:56       ` Randy Jennings
2025-04-16  6:39         ` Daniel Wagner
2025-04-16  0:17       ` Mohamed Khalfella
2025-04-16  6:57         ` Daniel Wagner
2025-04-16 13:39           ` Mohamed Khalfella
2025-04-16  0:40       ` Mohamed Khalfella
2025-04-16  8:30         ` Daniel Wagner
2025-04-16 13:53           ` Mohamed Khalfella
2025-04-16 22:21             ` Sagi Grimberg
2025-04-16 22:59               ` Mohamed Khalfella [this message]
2025-04-17  7:28                 ` Hannes Reinecke
2025-04-10 16:07   ` Jiewei Ke
2025-04-10 17:13   ` Jiewei Ke
2025-04-13 22:03   ` Sagi Grimberg
2025-04-16  8:51     ` Daniel Wagner
2025-04-16  0:23   ` Mohamed Khalfella
2025-04-16 11:33     ` Daniel Wagner
     [not found] <8F2489FD-1663-4A52-A50B-F15046AC2878@163.com>
2025-04-15 12:34 ` Daniel Wagner
2025-04-15 15:08   ` Jiewei Ke

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=20250416225913.GA2476975-mkhalfella@purestorage.com \
    --to=mkhalfella@purestorage.com \
    --cc=dwagner@suse.de \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=jmeneghi@redhat.com \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=randyj@purestorage.com \
    --cc=sagi@grimberg.me \
    --cc=wagi@kernel.org \
    /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