[PATCH] nvme-pci: fix queue unquiesce check on slot_reset

[PATCH] nvme-pci: fix queue unquiesce check on slot_reset

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

A zero return means the reset was successfully scheduled. We don't want
to unquiesce the queues while the reset_work is pending, as that will
just flush out requeued requests to a failed completion.

Fixes: 71a5bb153be104 ("nvme: ensure disabling pairs with unquiesce")
Reported-by: Dhankaran Singh Ajravat <dhankaran@meta.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b178d52eac1b7..c9e2a5450bc0f 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3575,7 +3575,7 @@ static pci_ers_result_t nvme_slot_reset(struct pci_dev *pdev)
 
 	dev_info(dev->ctrl.device, "restart after slot reset\n");
 	pci_restore_state(pdev);
-	if (!nvme_try_sched_reset(&dev->ctrl))
+	if (nvme_try_sched_reset(&dev->ctrl))
 		nvme_unquiesce_io_queues(&dev->ctrl);
 	return PCI_ERS_RESULT_RECOVERED;
 }
-- 
2.47.1

Re: [PATCH] nvme-pci: fix queue unquiesce check on slot_reset

[Christoph Hellwig]

On Thu, Apr 24, 2025 at 10:18:01AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> A zero return means the reset was successfully scheduled. We don't want
> to unquiesce the queues while the reset_work is pending, as that will
> just flush out requeued requests to a failed completion.
> 
> Fixes: 71a5bb153be104 ("nvme: ensure disabling pairs with unquiesce")

Sounds like this code path isn't get teste all the much if this stuck
around for so long..

Re: [PATCH] nvme-pci: fix queue unquiesce check on slot_reset

[Keith Busch]

On Fri, Apr 25, 2025 at 03:19:35PM +0200, Christoph Hellwig wrote:
> On Thu, Apr 24, 2025 at 10:18:01AM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > A zero return means the reset was successfully scheduled. We don't want
> > to unquiesce the queues while the reset_work is pending, as that will
> > just flush out requeued requests to a failed completion.
> > 
> > Fixes: 71a5bb153be104 ("nvme: ensure disabling pairs with unquiesce")
> 
> Sounds like this code path isn't get teste all the much if this stuck
> around for so long..

The conditions that trigger pcie errors were the primary concern. Of
course you'll get IO errors, right? The pcie connection is flakey! But
we are supposed retry IO's after recovery, which we weren't doing, and
that was a secondary concern I embaressingly overlooked for many
reports.

Re: [PATCH] nvme-pci: fix queue unquiesce check on slot_reset

[Keith Busch]

On Fri, Apr 25, 2025 at 08:33:28AM -0600, Keith Busch wrote:
> On Fri, Apr 25, 2025 at 03:19:35PM +0200, Christoph Hellwig wrote:
> > On Thu, Apr 24, 2025 at 10:18:01AM -0700, Keith Busch wrote:
> > > From: Keith Busch <kbusch@kernel.org>
> > > 
> > > A zero return means the reset was successfully scheduled. We don't want
> > > to unquiesce the queues while the reset_work is pending, as that will
> > > just flush out requeued requests to a failed completion.
> > > 
> > > Fixes: 71a5bb153be104 ("nvme: ensure disabling pairs with unquiesce")
> > 
> > Sounds like this code path isn't get teste all the much if this stuck
> > around for so long..
> 
> The conditions that trigger pcie errors were the primary concern. Of
> course you'll get IO errors, right? The pcie connection is flakey! But
> we are supposed retry IO's after recovery, which we weren't doing, and
> that was a secondary concern I embaressingly overlooked for many
> reports.

We can still take this right? It is a good fix, even if we misunderstood
why IO was failing for over a year there.

Re: [PATCH] nvme-pci: fix queue unquiesce check on slot_reset

[Christoph Hellwig]

On Mon, Apr 28, 2025 at 09:22:55AM -0600, Keith Busch wrote:
> > > Sounds like this code path isn't get teste all the much if this stuck
> > > around for so long..
> > 
> > The conditions that trigger pcie errors were the primary concern. Of
> > course you'll get IO errors, right? The pcie connection is flakey! But
> > we are supposed retry IO's after recovery, which we weren't doing, and
> > that was a secondary concern I embaressingly overlooked for many
> > reports.
> 
> We can still take this right?

Absolutely.  Just wondering how this managed to slip through.

Re: [PATCH] nvme-pci: fix queue unquiesce check on slot_reset

[Christoph Hellwig]

Thanks,

added to nvme-6.15.