From: Michael Ellerman <michael@ellerman.id.au>
To: Alexander Gordeev <agordeev@redhat.com>
Cc: linuxppc-dev@lists.ozlabs.org, Joerg Roedel <joro@8bytes.org>,
"x86@kernel.org" <x86@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
Jan Beulich <JBeulich@suse.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
Tejun Heo <tj@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface
Date: Wed, 2 Oct 2013 12:43:24 +1000 [thread overview]
Message-ID: <20131002024324.GC22748@concordia> (raw)
In-Reply-To: <20131001103526.GA5053@dhcp-26-207.brq.redhat.com>
On Tue, Oct 01, 2013 at 12:35:27PM +0200, Alexander Gordeev wrote:
> On Tue, Oct 01, 2013 at 05:51:33PM +1000, Michael Ellerman wrote:
> > The disadvantage is that any restriction imposed on us above the quota
> > can only be reported as an error from pci_enable_msix().
> >
> > The quota code, called from pci_get_msix_limit(), can only do so much to
> > interogate firmware about the limitations. The ultimate way to check if
> > firmware will give us enough MSIs is to try and allocate them. But we
> > can't do that from pci_get_msix_limit() because the driver is not asking
> > us to enable MSIs, just query them.
>
> If things are this way then pci_enable_msix() already exposed to this
> problem internally on pSeries.
>
> I see that even successful quota checks in rtas_msi_check_device() and
> rtas_setup_msi_irqs() do not guarantee (as you say) that firmware will
> give enough MSIs. Hence, pci_enable_msix() might fail even though the
> its quota checks succeeded.
Yes, but it can report that failure to the caller, which can then retry.
> Therefore, nothing will really change if we make pci_get_msix_limit() check
> quota and hope the follow-up call to pci_enable_msix() succeeded.
No that's not equivalent. Under your scheme if pci_enable_msix() fails
then the caller just bails, it will never try again with a lower number.
> (Of course, we could allocate-deallocate MSIs at check time, but I think it
> is an overkill).
It's not only overkill, it's messing with the device behind the drivers
back, which is definitely a no-no in my opinion.
> > You'll also need to add another arch hook, for the quota check, and
> > we'll have to add it to our per-platform indirection as well.
>
> Already, in a branch, hidden from Bjorn & Tejun eyes ;)
>
> > All a lot of bother for no real gain IMHO.
>
> Well, I do not have a strong opinion here. I leave it to the ones who have :)
> But few drivers have became clearer as result of this change (and messy ones
> are still messy).
Amen.
cheers
WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <michael@ellerman.id.au>
To: Alexander Gordeev <agordeev@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Tejun Heo <tj@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"x86@kernel.org" <x86@kernel.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
Ingo Molnar <mingo@kernel.org>, Joerg Roedel <joro@8bytes.org>,
Jan Beulich <JBeulich@suse.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface
Date: Wed, 2 Oct 2013 12:43:24 +1000 [thread overview]
Message-ID: <20131002024324.GC22748@concordia> (raw)
In-Reply-To: <20131001103526.GA5053@dhcp-26-207.brq.redhat.com>
On Tue, Oct 01, 2013 at 12:35:27PM +0200, Alexander Gordeev wrote:
> On Tue, Oct 01, 2013 at 05:51:33PM +1000, Michael Ellerman wrote:
> > The disadvantage is that any restriction imposed on us above the quota
> > can only be reported as an error from pci_enable_msix().
> >
> > The quota code, called from pci_get_msix_limit(), can only do so much to
> > interogate firmware about the limitations. The ultimate way to check if
> > firmware will give us enough MSIs is to try and allocate them. But we
> > can't do that from pci_get_msix_limit() because the driver is not asking
> > us to enable MSIs, just query them.
>
> If things are this way then pci_enable_msix() already exposed to this
> problem internally on pSeries.
>
> I see that even successful quota checks in rtas_msi_check_device() and
> rtas_setup_msi_irqs() do not guarantee (as you say) that firmware will
> give enough MSIs. Hence, pci_enable_msix() might fail even though the
> its quota checks succeeded.
Yes, but it can report that failure to the caller, which can then retry.
> Therefore, nothing will really change if we make pci_get_msix_limit() check
> quota and hope the follow-up call to pci_enable_msix() succeeded.
No that's not equivalent. Under your scheme if pci_enable_msix() fails
then the caller just bails, it will never try again with a lower number.
> (Of course, we could allocate-deallocate MSIs at check time, but I think it
> is an overkill).
It's not only overkill, it's messing with the device behind the drivers
back, which is definitely a no-no in my opinion.
> > You'll also need to add another arch hook, for the quota check, and
> > we'll have to add it to our per-platform indirection as well.
>
> Already, in a branch, hidden from Bjorn & Tejun eyes ;)
>
> > All a lot of bother for no real gain IMHO.
>
> Well, I do not have a strong opinion here. I leave it to the ones who have :)
> But few drivers have became clearer as result of this change (and messy ones
> are still messy).
Amen.
cheers
next prev parent reply other threads:[~2013-10-02 2:43 UTC|newest]
Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-05 12:51 [PATCH v2 0/6] AHCI: Conserve interrupts with pci_enable_msi_block_part() interface Alexander Gordeev
2013-09-05 12:52 ` [PATCH v2 1/6] PCI/MSI: Introduce " Alexander Gordeev
2013-09-05 12:52 ` [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface Alexander Gordeev
2013-09-05 13:09 ` Tejun Heo
2013-09-05 15:03 ` Alexander Gordeev
2013-09-05 15:04 ` Tejun Heo
2013-09-05 15:40 ` Alexander Gordeev
2013-09-05 15:44 ` Tejun Heo
2013-09-05 18:54 ` Alexander Gordeev
2013-09-05 20:06 ` Tejun Heo
2013-09-06 16:01 ` Bjorn Helgaas
2013-09-06 16:06 ` Tejun Heo
2013-09-06 23:32 ` Bjorn Helgaas
2013-09-09 15:20 ` Alexander Gordeev
2013-09-09 15:22 ` [PATCH 1/9] PCI/MSI/PPC: Fix wrong RTAS error code reporting Alexander Gordeev
2013-09-09 15:22 ` [PATCH 2/9] PCI/MSI/PPC: Make return values only 0/-errno when MSIs allocated Alexander Gordeev
2013-09-09 15:24 ` [PATCH 3/9] PCI/MSI/x86: " Alexander Gordeev
2013-09-09 15:24 ` [PATCH 4/9] PCI/MSI/MIPS: " Alexander Gordeev
2013-09-09 15:25 ` [PATCH 2/9] PCI/MSI/PPC: Make return values only 0/-errno when MSIs allocated[PATCH 5/9] PCI/MSI/s390: " Alexander Gordeev
2013-09-09 15:38 ` scrap this one Alexander Gordeev
2013-09-09 15:26 ` [PATCH 5/9] PCI/MSI/s390: Make return values only 0/-errno when MSIs allocated Alexander Gordeev
2013-09-10 12:42 ` Sergei Shtylyov
2013-09-10 13:09 ` Alexander Gordeev
2013-09-09 15:27 ` [PATCH 6/9] PCI/MSI/s390: Remove superfluous check of MSI type Alexander Gordeev
2013-09-09 15:28 ` [PATCH 7/9] PCI/MSI/s390: Make return values only 0/-errno when MSIs allocated Alexander Gordeev
2013-09-09 15:29 ` [PATCH 8/9] PCI/MSI: Fix return value when populate_msi_sysfs() failed Alexander Gordeev
2013-09-09 15:29 ` [PATCH 9/9] PCI/MSI: Make return values only 0/-errno when MSIs allocated Alexander Gordeev
2013-09-09 15:37 ` [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface Tejun Heo
2013-09-09 15:45 ` Alexander Gordeev
2013-09-16 10:22 ` Alexander Gordeev
2013-09-16 10:22 ` Alexander Gordeev
2013-09-17 14:30 ` Michael Ellerman
2013-09-17 14:30 ` Michael Ellerman
2013-09-18 9:48 ` Alexander Gordeev
2013-09-18 9:48 ` Alexander Gordeev
2013-09-18 14:22 ` Tejun Heo
2013-09-18 14:22 ` Tejun Heo
2013-09-18 16:50 ` Alexander Gordeev
2013-09-18 16:50 ` Alexander Gordeev
2013-09-20 8:24 ` Alexander Gordeev
2013-09-20 8:24 ` Alexander Gordeev
2013-09-20 12:27 ` Tejun Heo
2013-09-20 12:27 ` Tejun Heo
2013-09-25 18:02 ` Bjorn Helgaas
2013-09-25 18:02 ` Bjorn Helgaas
2013-09-25 20:58 ` Alexander Gordeev
2013-09-25 20:58 ` Alexander Gordeev
2013-09-25 21:00 ` Tejun Heo
2013-09-25 21:00 ` Tejun Heo
2013-09-26 7:46 ` Alexander Gordeev
2013-09-26 7:46 ` Alexander Gordeev
2013-09-26 8:58 ` David Laight
2013-09-26 8:58 ` David Laight
2013-09-26 8:58 ` David Laight
2013-09-26 10:45 ` Alexander Gordeev
2013-09-26 10:45 ` Alexander Gordeev
2013-09-26 11:34 ` David Laight
2013-09-26 11:34 ` David Laight
2013-09-26 11:34 ` David Laight
2013-09-26 12:13 ` Alexander Gordeev
2013-09-26 12:13 ` Alexander Gordeev
2013-09-26 13:11 ` Tejun Heo
2013-09-26 13:11 ` Tejun Heo
2013-09-26 14:39 ` Alexander Gordeev
2013-09-26 14:39 ` Alexander Gordeev
2013-09-26 14:42 ` Tejun Heo
2013-09-26 14:42 ` Tejun Heo
2013-10-01 7:19 ` Michael Ellerman
2013-10-01 7:19 ` Michael Ellerman
2013-09-20 12:26 ` Tejun Heo
2013-09-20 12:26 ` Tejun Heo
2013-10-01 7:26 ` Michael Ellerman
2013-10-01 7:26 ` Michael Ellerman
2013-10-01 7:35 ` Michael Ellerman
2013-10-01 7:35 ` Michael Ellerman
2013-10-01 11:55 ` Tejun Heo
2013-10-01 11:55 ` Tejun Heo
2013-10-02 2:33 ` Michael Ellerman
2013-10-02 2:33 ` Michael Ellerman
2013-10-02 3:23 ` Tejun Heo
2013-10-02 3:23 ` Tejun Heo
2013-09-26 12:32 ` Mark Lord
2013-09-26 12:32 ` Mark Lord
2013-09-26 13:03 ` Alexander Gordeev
2013-09-26 13:03 ` Alexander Gordeev
2013-10-02 2:46 ` Mark Lord
2013-10-02 2:46 ` Mark Lord
2013-10-02 7:26 ` Alexander Gordeev
2013-10-02 7:26 ` Alexander Gordeev
2013-12-18 18:26 ` Bjorn Helgaas
2013-12-18 18:26 ` Bjorn Helgaas
2013-10-01 7:51 ` Michael Ellerman
2013-10-01 7:51 ` Michael Ellerman
2013-10-01 10:35 ` Alexander Gordeev
2013-10-01 10:35 ` Alexander Gordeev
2013-10-02 2:43 ` Michael Ellerman [this message]
2013-10-02 2:43 ` Michael Ellerman
2013-10-02 7:10 ` Alexander Gordeev
2013-10-02 7:10 ` Alexander Gordeev
2013-09-06 13:17 ` Alexander Gordeev
2013-09-05 12:53 ` [PATCH v2 3/6] MSI/x86: Support pci_enable_msi_block_part() interface Alexander Gordeev
2013-09-05 12:53 ` [PATCH v2 4/6] AHCI: Conserve interrupts with " Alexander Gordeev
2013-09-05 13:10 ` Tejun Heo
2013-09-05 15:23 ` Alexander Gordeev
2013-09-05 12:54 ` [PATCH v2 5/6] AHCI: Check MRSM bit when multiple MSIs enabled Alexander Gordeev
2013-09-05 13:11 ` Tejun Heo
2013-09-05 15:25 ` Alexander Gordeev
2013-09-05 14:32 ` Sergei Shtylyov
2013-09-05 12:54 ` [PATCH v2 6/6] PCI/MSI: Get rid of pci_enable_msi_block_auto() interface Alexander Gordeev
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=20131002024324.GC22748@concordia \
--to=michael@ellerman.id.au \
--cc=JBeulich@suse.com \
--cc=agordeev@redhat.com \
--cc=bhelgaas@google.com \
--cc=joro@8bytes.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@kernel.org \
--cc=tj@kernel.org \
--cc=x86@kernel.org \
/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.