From: mr.nuke.me@gmail.com (Alex G.)
Subject: [PATCH] nvme/pci: Sync controller reset for AER slot_reset
Date: Thu, 10 May 2018 13:56:56 -0500 [thread overview]
Message-ID: <93d528ce-043a-5118-02b3-986d151b37cf@gmail.com> (raw)
In-Reply-To: <20180510160113.4432-1-keith.busch@intel.com>
On 05/10/2018 11:01 AM, Keith Busch wrote:
> AER handling expects a successful return from slot_reset means the
> driver made the device functional again. The nvme driver had been using
> an asynchronous reset to recover the device, so the device
> may still be initializing after control is returned to the
> AER handler. This creates problems for subsequent event handling,
> causing the initializion to fail.
>
> This patch fixes that by syncing the controller reset before returning
> to the AER driver, and reporting the true state of the reset.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199657
> Reported-by: Alex Gagniuc <mr.nuke.me at gmail.com>
Tested-by: Alex Gagniuc <mr.nuke.me at gmail.com>
Sponsored-by: DellEMC
You know I had to add that plug somewhere :p
> Cc: Sinan Kaya <okaya at codeaurora.org>
> Cc: Bjorn Helgaas <bhelgaas at google.com>
> Cc: <stable at vger.kernel.org>
> Signed-off-by: Keith Busch <keith.busch at intel.com>
> ---
> drivers/nvme/host/pci.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index b542dce45927..2e221796257a 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2681,8 +2681,15 @@ 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);
> - nvme_reset_ctrl(&dev->ctrl);
> - return PCI_ERS_RESULT_RECOVERED;
> + nvme_reset_ctrl_sync(&dev->ctrl);
This does wonders when nvme_reset_ctrl_sync() returns in a timely
manner. I was also able to get the nvme drive in a state where
nvme_reset_ctrl_sync() does not return. Then we end up with the device
lock in report_slot_reset, which, as you may imagine, is not a great thing.
I think this step is a move in the better direction, but we still have
problems.
Alex
> + switch (dev->ctrl.state) {
> + case NVME_CTRL_LIVE:
> + case NVME_CTRL_ADMIN_ONLY:
> + return PCI_ERS_RESULT_RECOVERED;
> + default:
> + return PCI_ERS_RESULT_DISCONNECT;
> + }
> }
>
> static void nvme_error_resume(struct pci_dev *pdev)
>
WARNING: multiple messages have this Message-ID (diff)
From: "Alex G." <mr.nuke.me@gmail.com>
To: Keith Busch <keith.busch@intel.com>,
linux-nvme@lists.infradead.org, Christoph Hellwig <hch@lst.de>,
Sagi Grimberg <sagi@grimberg.me>
Cc: Sinan Kaya <okaya@codeaurora.org>,
Bjorn Helgaas <bhelgaas@google.com>,
stable@vger.kernel.org, austin_bolen@dell.com,
Alex_Gagniuc@Dellteam.com, shyam_iyer@dell.com
Subject: Re: [PATCH] nvme/pci: Sync controller reset for AER slot_reset
Date: Thu, 10 May 2018 13:56:56 -0500 [thread overview]
Message-ID: <93d528ce-043a-5118-02b3-986d151b37cf@gmail.com> (raw)
In-Reply-To: <20180510160113.4432-1-keith.busch@intel.com>
On 05/10/2018 11:01 AM, Keith Busch wrote:
> AER handling expects a successful return from slot_reset means the
> driver made the device functional again. The nvme driver had been using
> an asynchronous reset to recover the device, so the device
> may still be initializing after control is returned to the
> AER handler. This creates problems for subsequent event handling,
> causing the initializion to fail.
>
> This patch fixes that by syncing the controller reset before returning
> to the AER driver, and reporting the true state of the reset.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199657
> Reported-by: Alex Gagniuc <mr.nuke.me@gmail.com>
Tested-by: Alex Gagniuc <mr.nuke.me@gmail.com>
Sponsored-by: DellEMC
You know I had to add that plug somewhere :p
> Cc: Sinan Kaya <okaya@codeaurora.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> drivers/nvme/host/pci.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index b542dce45927..2e221796257a 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2681,8 +2681,15 @@ 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);
> - nvme_reset_ctrl(&dev->ctrl);
> - return PCI_ERS_RESULT_RECOVERED;
> + nvme_reset_ctrl_sync(&dev->ctrl);
This does wonders when nvme_reset_ctrl_sync() returns in a timely
manner. I was also able to get the nvme drive in a state where
nvme_reset_ctrl_sync() does not return. Then we end up with the device
lock in report_slot_reset, which, as you may imagine, is not a great thing.
I think this step is a move in the better direction, but we still have
problems.
Alex
> + switch (dev->ctrl.state) {
> + case NVME_CTRL_LIVE:
> + case NVME_CTRL_ADMIN_ONLY:
> + return PCI_ERS_RESULT_RECOVERED;
> + default:
> + return PCI_ERS_RESULT_DISCONNECT;
> + }
> }
>
> static void nvme_error_resume(struct pci_dev *pdev)
>
next prev parent reply other threads:[~2018-05-10 18:56 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-10 16:01 [PATCH] nvme/pci: Sync controller reset for AER slot_reset Keith Busch
2018-05-10 16:01 ` Keith Busch
2018-05-10 18:56 ` Alex G. [this message]
2018-05-10 18:56 ` Alex G.
2018-05-10 19:14 ` Keith Busch
2018-05-10 19:14 ` Keith Busch
2018-05-10 19:20 ` Alex G.
2018-05-10 19:20 ` Alex G.
2018-05-11 14:18 ` Alex G.
2018-05-11 14:18 ` Alex G.
2018-05-12 9:27 ` Ming Lei
2018-05-12 9:27 ` Ming Lei
2018-05-11 6:38 ` Christoph Hellwig
2018-05-11 6:38 ` Christoph Hellwig
2018-05-11 20:54 ` Martin K. Petersen
2018-05-11 20:54 ` Martin K. Petersen
2018-05-11 21:09 ` Keith Busch
2018-05-11 21:09 ` Keith Busch
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=93d528ce-043a-5118-02b3-986d151b37cf@gmail.com \
--to=mr.nuke.me@gmail.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.