From patchwork Wed Oct 2 02:33:38 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 2972931 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id F11839F88A for ; Wed, 2 Oct 2013 02:34:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0DC44203DF for ; Wed, 2 Oct 2013 02:34:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 07F9D203C4 for ; Wed, 2 Oct 2013 02:34:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752440Ab3JBCdp (ORCPT ); Tue, 1 Oct 2013 22:33:45 -0400 Received: from ozlabs.org ([203.10.76.45]:37109 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752331Ab3JBCdo (ORCPT ); Tue, 1 Oct 2013 22:33:44 -0400 Received: from concordia (ibmaus65.lnk.telstra.net [165.228.126.9]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPSA id 6B0F22C0097; Wed, 2 Oct 2013 12:33:40 +1000 (EST) Date: Wed, 2 Oct 2013 12:33:38 +1000 From: Michael Ellerman To: Tejun Heo Cc: Alexander Gordeev , Benjamin Herrenschmidt , "linux-kernel@vger.kernel.org" , "x86@kernel.org" , "linux-pci@vger.kernel.org" , "linux-ide@vger.kernel.org" , Ingo Molnar , Joerg Roedel , Jan Beulich , Bjorn Helgaas , linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface Message-ID: <20131002023337.GB22748@concordia> References: <20130906160621.GF22763@mtj.dyndns.org> <20130906233205.GF12956@google.com> <20130909152044.GA24962@dhcp-26-207.brq.redhat.com> <20130916102210.GA14102@dhcp-26-207.brq.redhat.com> <20130917143022.GA7707@concordia> <20130918094759.GA2353@dhcp-26-207.brq.redhat.com> <20130918142231.GA21650@mtj.dyndns.org> <20131001073548.GI17966@concordia> <20131001115503.GA23722@mtj.dyndns.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20131001115503.GA23722@mtj.dyndns.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Oct 01, 2013 at 07:55:03AM -0400, Tejun Heo wrote: > Hello, > > On Tue, Oct 01, 2013 at 05:35:48PM +1000, Michael Ellerman wrote: > > > > Roughly third of the drivers just do not care and bail out once > > > > pci_enable_msix() has not succeeded. Not sure how many of these are > > > > mandated by the hardware. > > > > > > Yeah, I mean, this type of interface is a trap. People have to > > > actively resist to avoid doing silly stuff which is a lot to ask. > > > > I really think you're overstating the complexity here. > > > > Functions typically return a boolean -> nothing to see here > > This function returns a tristate value -> brain explosion! > > It is an interface which forces the driver writers to write > complicated fallback code which won't usually be excercised. It does not force anyone to do anything. That's just bull. Code which is unwilling or unable to cope with the extra complexity can simply do: if (pci_enable_msix(..)) goto fail; It's as simple as that. > > > * Determine the number of MSIs the controller wants. Don't worry > > > about quotas or limits or anything. Just determine the number > > > necessary to enable enhanced interrupt handling. > > > > > > * Try allocating that number of MSIs. If it fails, then just revert > > > to single interrupt mode. It's not the end of the world and mostly > > > guaranteed to work. Let's please not even try to do partial > > > multiple interrupts. I really don't think it's worth the risk or > > > complexity. > > > > It will potentially break existing setups on our hardware. > > I think it'd be much better to have a separate interface for the > drivers which actually require it *in practice* rather than forcing > everyone to go "oh this interface supports that, I don't know if I > need it but let's implement fallback logic which I won't and have no > means of testing". Sure, that's easy: > Are we talking about some limited number of device drivers here? I don't have a list, but yeah there are certain drivers that folks care about. > Also, is the quota still necessary for machines in production today? As far as I know yes. The number of MSIs is growing on modern systems, but so is the number of cpus and devices. cheers --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index d5f90d6..48d0252 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -988,6 +988,18 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec) } EXPORT_SYMBOL(pci_enable_msix); +int pci_enable_msix_or_fail(struct pci_dev *dev, struct msix_entry *entries, + int nvec) +{ + int rc; + + rc = pci_enable_msix(dev, entries, nvec); + if (rc > 0) + rc = -ENOSPC; + + return rc; +} + void pci_msix_shutdown(struct pci_dev *dev) { struct msi_desc *entry;