From: ming.lei@redhat.com (Ming Lei)
Subject: [PATCH 2/2] nvme pci: try to allocate multiple irq vectors again in case of -EINVAL
Date: Fri, 28 Dec 2018 06:16:33 +0800 [thread overview]
Message-ID: <20181227221631.GA22073@ming.t460p> (raw)
In-Reply-To: <20181227130834.GA22967@lst.de>
On Thu, Dec 27, 2018@02:08:34PM +0100, Christoph Hellwig wrote:
> On Thu, Dec 27, 2018@04:21:38PM +0800, Ming Lei wrote:
> > On Wed, Dec 26, 2018@07:20:27PM +0100, Christoph Hellwig wrote:
> > > On Wed, Dec 26, 2018@06:37:55PM +0800, Ming Lei wrote:
> > > > It is observed on QEMU that pci_alloc_irq_vectors_affinity() may
> > > > returns -EINVAL when the requested number is too big(such as 64).
> > >
> > > Which is not how this API is supposed to work and documented to work.
> > >
> > > We need to fix pci_alloc_irq_vectors_affinity to not return a spurious
> > > error and just return the allocated number of vectors instead of
> > > hacking around that in drivers.
> >
> > Yeah, you are right.
> >
> > The issue is that QEMU nvme-pci is MSIX-capable only, and hasn't MSI
> > capability.
> >
> > __pci_enable_msix_range() actually returns -ENOSPC, but __pci_enable_msi_range()
> > returns -EINVAL because dev->msi_cap is zero.
> >
> > Maybe we need the following fix?
>
> Should it matter? We still get a negative vecs back, and still fall
> back to the next option. Unless ther are no irqs available at all
> for the selected types pci_alloc_irq_vectors_affinity should never
> return an error.
The patch in last email does fix this issue.
In this case, the number of NVMe PCI's MSI-X table entries is 64, so
__pci_enable_msix_range() return -ENOSPC when we ask for 65.
However, the following __pci_enable_msi_range() returns -EINVAL because
the NVMe PCI isn't capable of MSI, then this error is returned from
pci_alloc_irq_vectors_affinity() finally to NVMe driver.
Of course, -EINVAL makes a difference because the current code only
tries to assign one irq vector in this case, and it shouldn't be
returned from pci_alloc_irq_vectors_affinity(), given there is enough
msix entries for fallback, right?
Thanks,
Ming
WARNING: multiple messages have this Message-ID (diff)
From: Ming Lei <ming.lei@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@fb.com>,
linux-pci@vger.kernel.org, linux-nvme@lists.infradead.org,
Keith Busch <keith.busch@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [PATCH 2/2] nvme pci: try to allocate multiple irq vectors again in case of -EINVAL
Date: Fri, 28 Dec 2018 06:16:33 +0800 [thread overview]
Message-ID: <20181227221631.GA22073@ming.t460p> (raw)
In-Reply-To: <20181227130834.GA22967@lst.de>
On Thu, Dec 27, 2018 at 02:08:34PM +0100, Christoph Hellwig wrote:
> On Thu, Dec 27, 2018 at 04:21:38PM +0800, Ming Lei wrote:
> > On Wed, Dec 26, 2018 at 07:20:27PM +0100, Christoph Hellwig wrote:
> > > On Wed, Dec 26, 2018 at 06:37:55PM +0800, Ming Lei wrote:
> > > > It is observed on QEMU that pci_alloc_irq_vectors_affinity() may
> > > > returns -EINVAL when the requested number is too big(such as 64).
> > >
> > > Which is not how this API is supposed to work and documented to work.
> > >
> > > We need to fix pci_alloc_irq_vectors_affinity to not return a spurious
> > > error and just return the allocated number of vectors instead of
> > > hacking around that in drivers.
> >
> > Yeah, you are right.
> >
> > The issue is that QEMU nvme-pci is MSIX-capable only, and hasn't MSI
> > capability.
> >
> > __pci_enable_msix_range() actually returns -ENOSPC, but __pci_enable_msi_range()
> > returns -EINVAL because dev->msi_cap is zero.
> >
> > Maybe we need the following fix?
>
> Should it matter? We still get a negative vecs back, and still fall
> back to the next option. Unless ther are no irqs available at all
> for the selected types pci_alloc_irq_vectors_affinity should never
> return an error.
The patch in last email does fix this issue.
In this case, the number of NVMe PCI's MSI-X table entries is 64, so
__pci_enable_msix_range() return -ENOSPC when we ask for 65.
However, the following __pci_enable_msi_range() returns -EINVAL because
the NVMe PCI isn't capable of MSI, then this error is returned from
pci_alloc_irq_vectors_affinity() finally to NVMe driver.
Of course, -EINVAL makes a difference because the current code only
tries to assign one irq vector in this case, and it shouldn't be
returned from pci_alloc_irq_vectors_affinity(), given there is enough
msix entries for fallback, right?
Thanks,
Ming
next prev parent reply other threads:[~2018-12-27 22:16 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-26 10:37 [PATCH 0/2] nvme pci: two fixes on nvme_setup_irqs Ming Lei
2018-12-26 10:37 ` [PATCH 1/2] nvme pci: fix nvme_setup_irqs() Ming Lei
2018-12-26 10:37 ` [PATCH 2/2] nvme pci: try to allocate multiple irq vectors again in case of -EINVAL Ming Lei
2018-12-26 18:20 ` Christoph Hellwig
2018-12-27 8:21 ` Ming Lei
2018-12-27 8:21 ` Ming Lei
2018-12-27 13:08 ` Christoph Hellwig
2018-12-27 13:08 ` Christoph Hellwig
2018-12-27 22:16 ` Ming Lei [this message]
2018-12-27 22:16 ` Ming Lei
2018-12-31 21:51 ` Bjorn Helgaas
2018-12-31 21:51 ` Bjorn Helgaas
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=20181227221631.GA22073@ming.t460p \
--to=ming.lei@redhat.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.