From: hch@lst.de (Christoph Hellwig)
Subject: [PATCH 1/2] PCI: Provide sensible irq vector alloc/free routines
Date: Thu, 12 May 2016 14:19:33 +0200 [thread overview]
Message-ID: <20160512121933.GA11084@lst.de> (raw)
In-Reply-To: <20160512121155.GB8681@agordeev.lab.eng.brq.redhat.com>
On Thu, May 12, 2016@02:11:57PM +0200, Alexander Gordeev wrote:
> Not this way. MSI vectors could be a scarce resource in a platform. So
> even though devices could support more MSI-Xs than MSIs the underlying
> platform might fail to fulfil a device request.
I can go back to just calling the existing exported functions like
the first version did for now, that'll handle it for now.
> > > Another thing, pci_alloc_irq_vectors() tries to allocate vectors in a
> > > range from 1 to nr_vecs now. So this function implicitly falls into
> > > the other two range functions family and therefore:
> > >
> > > (a) pci_alloc_irq_vectors() name is not perfec;
> >
> > What would you call it instead?
>
> I do not know, really :( I would expect "range" within the name since
> a range requested indeed, but I am just hinting here.
We're requesting multiple vectors, so I think the naming should be fine.
> > > (b) why not introduce 'minvec' minimal number of interrupts then?
> > > We could have a handy pci_enable_irq_range() as result;
> >
> > That seems pretty pointless, when the caller can simply treat a too
> > small number as failure and use the existing failure path for that.
>
> There was a huge discussion on this few years ago, when the range
> functions were introduced. Actually, the prototypes of these two is
> the outcome of that discussion. I almost sure your point was expressed
> by many at the time ;)
A quick audit shows that there are indeed a few users of this
interface. I can add pci_alloc_irq_vectors_range that allows passing
a minvec, and the old pci_alloc_irq_vectors interface for everyone
who wants to keep it simple.
> > pci_enable_msi_range still has a horrible API that forces the caller
> > to deal with the irq numbers differently than the MSI-X case, so it
> > should also go away in the long run.
>
> Well, if we introduce pci_enable_irq_range() (or something) and
> pci_get_dev_irq(int vector) (or something) that covers MSI-X, MSI and
> legacy IRQs then we can get it done now. Your pci_alloc_irq_vectors()
> is just few steps from there, huh?
Or we can just look at pdev->irqs as in my patch. That'll work without
much overhead for the single IRQ case as it can just point to ->irq,
and gives a neat interface for MSI-X and the rare multi-MSI case.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Alexander Gordeev <agordeev@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>,
helgaas@kernel.org, pjw@netapp.com, axboe@fb.com,
keith.busch@intel.com, linux-pci@vger.kernel.org,
linux-nvme@lists.infradead.org
Subject: Re: [PATCH 1/2] PCI: Provide sensible irq vector alloc/free routines
Date: Thu, 12 May 2016 14:19:33 +0200 [thread overview]
Message-ID: <20160512121933.GA11084@lst.de> (raw)
In-Reply-To: <20160512121155.GB8681@agordeev.lab.eng.brq.redhat.com>
On Thu, May 12, 2016 at 02:11:57PM +0200, Alexander Gordeev wrote:
> Not this way. MSI vectors could be a scarce resource in a platform. So
> even though devices could support more MSI-Xs than MSIs the underlying
> platform might fail to fulfil a device request.
I can go back to just calling the existing exported functions like
the first version did for now, that'll handle it for now.
> > > Another thing, pci_alloc_irq_vectors() tries to allocate vectors in a
> > > range from 1 to nr_vecs now. So this function implicitly falls into
> > > the other two range functions family and therefore:
> > >
> > > (a) pci_alloc_irq_vectors() name is not perfec;
> >
> > What would you call it instead?
>
> I do not know, really :( I would expect "range" within the name since
> a range requested indeed, but I am just hinting here.
We're requesting multiple vectors, so I think the naming should be fine.
> > > (b) why not introduce 'minvec' minimal number of interrupts then?
> > > We could have a handy pci_enable_irq_range() as result;
> >
> > That seems pretty pointless, when the caller can simply treat a too
> > small number as failure and use the existing failure path for that.
>
> There was a huge discussion on this few years ago, when the range
> functions were introduced. Actually, the prototypes of these two is
> the outcome of that discussion. I almost sure your point was expressed
> by many at the time ;)
A quick audit shows that there are indeed a few users of this
interface. I can add pci_alloc_irq_vectors_range that allows passing
a minvec, and the old pci_alloc_irq_vectors interface for everyone
who wants to keep it simple.
> > pci_enable_msi_range still has a horrible API that forces the caller
> > to deal with the irq numbers differently than the MSI-X case, so it
> > should also go away in the long run.
>
> Well, if we introduce pci_enable_irq_range() (or something) and
> pci_get_dev_irq(int vector) (or something) that covers MSI-X, MSI and
> legacy IRQs then we can get it done now. Your pci_alloc_irq_vectors()
> is just few steps from there, huh?
Or we can just look at pdev->irqs as in my patch. That'll work without
much overhead for the single IRQ case as it can just point to ->irq,
and gives a neat interface for MSI-X and the rare multi-MSI case.
next prev parent reply other threads:[~2016-05-12 12:19 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-05 14:04 PCI: Provide sensible irq vector alloc/free routines Christoph Hellwig
2016-05-05 14:04 ` Christoph Hellwig
2016-05-05 14:04 ` [PATCH 1/2] " Christoph Hellwig
2016-05-05 14:04 ` Christoph Hellwig
2016-05-06 16:04 ` Bjorn Helgaas
2016-05-06 16:04 ` Bjorn Helgaas
2016-05-06 16:28 ` Christoph Hellwig
2016-05-06 16:28 ` Christoph Hellwig
2016-05-06 16:35 ` Bjorn Helgaas
2016-05-06 16:35 ` Bjorn Helgaas
2016-05-08 9:05 ` Christoph Hellwig
2016-05-08 9:05 ` Christoph Hellwig
2016-05-09 22:46 ` Bjorn Helgaas
2016-05-09 22:46 ` Bjorn Helgaas
2016-05-11 8:52 ` Christoph Hellwig
2016-05-11 8:52 ` Christoph Hellwig
2016-05-16 19:39 ` Bjorn Helgaas
2016-05-16 19:39 ` Bjorn Helgaas
2016-05-17 16:45 ` Christoph Hellwig
2016-05-17 16:45 ` Christoph Hellwig
2016-05-11 7:45 ` Alexander Gordeev
2016-05-11 7:45 ` Alexander Gordeev
2016-05-11 8:50 ` Christoph Hellwig
2016-05-11 8:50 ` Christoph Hellwig
2016-05-11 9:44 ` Alexander Gordeev
2016-05-11 9:44 ` Alexander Gordeev
2016-05-12 7:35 ` Christoph Hellwig
2016-05-12 7:35 ` Christoph Hellwig
2016-05-12 9:44 ` Alexander Gordeev
2016-05-12 9:44 ` Alexander Gordeev
2016-05-12 11:03 ` Christoph Hellwig
2016-05-12 11:03 ` Christoph Hellwig
2016-05-12 12:11 ` Alexander Gordeev
2016-05-12 12:11 ` Alexander Gordeev
2016-05-12 12:19 ` Christoph Hellwig [this message]
2016-05-12 12:19 ` Christoph Hellwig
2016-05-12 14:29 ` Christoph Hellwig
2016-05-12 14:29 ` Christoph Hellwig
2016-05-13 8:29 ` Alexander Gordeev
2016-05-13 8:29 ` Alexander Gordeev
2016-06-11 1:14 ` Bjorn Helgaas
2016-06-11 1:14 ` Bjorn Helgaas
2016-06-13 15:15 ` Christoph Hellwig
2016-06-13 15:15 ` Christoph Hellwig
2016-06-21 14:33 ` Christoph Hellwig
2016-06-21 14:33 ` Christoph Hellwig
2016-05-12 11:04 ` Alexander Gordeev
2016-05-12 11:04 ` Alexander Gordeev
2016-05-12 11:04 ` Christoph Hellwig
2016-05-12 11:04 ` Christoph Hellwig
2016-05-05 14:04 ` [PATCH 2/2] nvme: switch to use pci_alloc_irq_vectors Christoph Hellwig
2016-05-05 14:04 ` Christoph Hellwig
2016-05-10 15:27 ` Keith Busch
2016-05-10 15:27 ` 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=20160512121933.GA11084@lst.de \
--to=hch@lst.de \
/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.