From: Bjorn Helgaas <helgaas@kernel.org>
To: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
Cc: linux-pci@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux@yadro.com, Keith Busch <keith.busch@intel.com>,
Jens Axboe <axboe@fb.com>, Christoph Hellwig <hch@lst.de>,
Sagi Grimberg <sagi@grimberg.me>,
linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC v4 08/21] nvme-pci: Handle movable BARs
Date: Tue, 26 Mar 2019 15:20:55 -0500 [thread overview]
Message-ID: <20190326202055.GP24180@google.com> (raw)
In-Reply-To: <20190311133122.11417-9-s.miroshnichenko@yadro.com>
[+cc Keith, Jens, Christoph, Sagi, linux-nvme, LKML]
On Mon, Mar 11, 2019 at 04:31:09PM +0300, Sergey Miroshnichenko wrote:
> Hotplugged devices can affect the existing ones by moving their BARs.
> PCI subsystem will inform the NVME driver about this by invoking
> reset_prepare()+reset_done(), then iounmap()+ioremap() must be called.
Do you mean the PCI core will invoke ->rescan_prepare() and
->rescan_done() (as opposed to *reset*)?
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
> drivers/nvme/host/pci.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 92bad1c810ac..ccea3033a67a 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -106,6 +106,7 @@ struct nvme_dev {
> unsigned int num_vecs;
> int q_depth;
> u32 db_stride;
> + resource_size_t current_phys_bar;
> void __iomem *bar;
> unsigned long bar_mapped_size;
> struct work_struct remove_work;
> @@ -1672,13 +1673,16 @@ static int nvme_remap_bar(struct nvme_dev *dev, unsigned long size)
> {
> struct pci_dev *pdev = to_pci_dev(dev->dev);
>
> - if (size <= dev->bar_mapped_size)
> + if (dev->bar &&
> + dev->current_phys_bar == pci_resource_start(pdev, 0) &&
> + size <= dev->bar_mapped_size)
> return 0;
> if (size > pci_resource_len(pdev, 0))
> return -ENOMEM;
> if (dev->bar)
> iounmap(dev->bar);
> - dev->bar = ioremap(pci_resource_start(pdev, 0), size);
> + dev->current_phys_bar = pci_resource_start(pdev, 0);
> + dev->bar = ioremap(dev->current_phys_bar, size);
dev->current_phys_bar is different from pci_resource_start() in the
case where the PCI core has moved the nvme BAR, but nvme has not yet
remapped it.
I'm not sure it's worth keeping track of current_phys_bar, as opposed
to always unmapping and remapping. Is this a performance path? I
think there are advantages to always exercising the same code path,
regardless of whether the BAR happened to be moved, e.g., if there's a
bug in the "BAR moved" path, it may be a heisenbug because whether we
exercise that path depends on the current configuration.
If you do need to cache current_phys_bar, maybe this, so it's a little
easier to see that you're not changing the ioremap() itself:
dev->bar = ioremap(pci_resource_start(pdev, 0), size);
dev->current_phys_bar = pci_resource_start(pdev, 0);
> if (!dev->bar) {
> dev->bar_mapped_size = 0;
> return -ENOMEM;
> @@ -2504,6 +2508,8 @@ static void nvme_reset_work(struct work_struct *work)
> if (WARN_ON(dev->ctrl.state != NVME_CTRL_RESETTING))
> goto out;
>
> + nvme_remap_bar(dev, db_bar_size(dev, 0));
How is this change connected to rescan? This looks reset-related.
> /*
> * If we're called to reset a live controller first shut it down before
> * moving on.
> @@ -2910,6 +2916,23 @@ static void nvme_error_resume(struct pci_dev *pdev)
> flush_work(&dev->ctrl.reset_work);
> }
>
> +void nvme_rescan_prepare(struct pci_dev *pdev)
> +{
> + struct nvme_dev *dev = pci_get_drvdata(pdev);
> +
> + nvme_dev_disable(dev, false);
> + nvme_dev_unmap(dev);
> + dev->bar = NULL;
> +}
> +
> +void nvme_rescan_done(struct pci_dev *pdev)
> +{
> + struct nvme_dev *dev = pci_get_drvdata(pdev);
> +
> + nvme_dev_map(dev);
> + nvme_reset_ctrl_sync(&dev->ctrl);
> +}
> +
> static const struct pci_error_handlers nvme_err_handler = {
> .error_detected = nvme_error_detected,
> .slot_reset = nvme_slot_reset,
> @@ -2974,6 +2997,8 @@ static struct pci_driver nvme_driver = {
> },
> .sriov_configure = pci_sriov_configure_simple,
> .err_handler = &nvme_err_handler,
> + .rescan_prepare = nvme_rescan_prepare,
> + .rescan_done = nvme_rescan_done,
> };
>
> static int __init nvme_init(void)
> --
> 2.20.1
>
next prev parent reply other threads:[~2019-03-26 20:20 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-11 13:31 [PATCH RFC v4 00/21] PCI: Allow BAR movement during hotplug Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 01/21] PCI: Fix writing invalid BARs during pci_restore_state() Sergey Miroshnichenko
2019-03-26 14:02 ` Bjorn Helgaas
2019-03-11 13:31 ` [PATCH RFC v4 02/21] PCI: Fix race condition in pci_enable/disable_device() Sergey Miroshnichenko
2019-03-26 19:00 ` Bjorn Helgaas
2019-03-27 17:11 ` Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 03/21] PCI: Enable bridge's I/O and MEM access for hotplugged devices Sergey Miroshnichenko
2019-03-26 19:13 ` Bjorn Helgaas
2019-03-27 17:13 ` Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 04/21] PCI: Define PCI-specific version of the release_child_resources() Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 05/21] PCI: hotplug: Add a flag for the movable BARs feature Sergey Miroshnichenko
2019-03-26 19:24 ` Bjorn Helgaas
2019-03-27 17:16 ` Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 06/21] PCI: Pause the devices with movable BARs during rescan Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 07/21] PCI: Wake up bridges during rescan when movable BARs enabled Sergey Miroshnichenko
2019-03-26 19:28 ` Bjorn Helgaas
2019-03-11 13:31 ` [PATCH RFC v4 08/21] nvme-pci: Handle movable BARs Sergey Miroshnichenko
2019-03-26 20:20 ` Bjorn Helgaas [this message]
2019-03-27 17:30 ` Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 09/21] PCI: Mark immovable BARs with PCI_FIXED Sergey Miroshnichenko
2019-03-26 20:28 ` Bjorn Helgaas
2019-03-27 17:03 ` David Laight
2019-03-27 17:39 ` Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 10/21] PCI: Fix assigning of fixed prefetchable resources Sergey Miroshnichenko
2019-03-26 20:37 ` Bjorn Helgaas
2019-03-11 13:31 ` [PATCH RFC v4 11/21] PCI: Release and reassign the root bridge resources during rescan Sergey Miroshnichenko
2019-03-26 20:41 ` Bjorn Helgaas
2019-03-27 17:40 ` Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 12/21] PCI: Don't allow hotplugged devices to steal resources Sergey Miroshnichenko
2019-03-26 20:55 ` Bjorn Helgaas
2019-03-27 18:02 ` Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 13/21] PCI: Include fixed BARs into the bus size calculating Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 14/21] PCI: Don't reserve memory for hotplug when enabled movable BARs Sergey Miroshnichenko
2019-03-26 20:57 ` Bjorn Helgaas
2019-03-11 13:31 ` [PATCH RFC v4 15/21] PCI: Allow the failed resources to be reassigned later Sergey Miroshnichenko
2019-03-26 20:58 ` Bjorn Helgaas
2019-03-11 13:31 ` [PATCH RFC v4 16/21] PCI: Calculate fixed areas of bridge windows based on fixed BARs Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 17/21] PCI: Calculate boundaries for bridge windows Sergey Miroshnichenko
2019-03-26 21:01 ` Bjorn Helgaas
2019-03-11 13:31 ` [PATCH RFC v4 18/21] PCI: Make sure bridge windows include their fixed BARs Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 19/21] PCI: Prioritize fixed BAR assigning over the movable ones Sergey Miroshnichenko
2019-03-11 13:31 ` [PATCH RFC v4 20/21] PCI: pciehp: Add support for the movable BARs feature Sergey Miroshnichenko
2019-03-26 21:11 ` Bjorn Helgaas
2019-03-11 13:31 ` [PATCH RFC v4 21/21] powerpc/pci: Fix crash with enabled movable BARs Sergey Miroshnichenko
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=20190326202055.GP24180@google.com \
--to=helgaas@kernel.org \
--cc=axboe@fb.com \
--cc=hch@lst.de \
--cc=keith.busch@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux@yadro.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=s.miroshnichenko@yadro.com \
--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;
as well as URLs for NNTP newsgroup(s).