Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1189746 > unrolled thread

Re: [PATCH v9 0/8] pci: add pci_iomap_wc() and pci_ioremap_wc_bar()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v9 0/8] pci: add pci_iomap_wc() and pci_ioremap_wc_bar() Ingo Molnar <mingo@kernel.org> - 2015-07-22 10:40 +0200
    Re: [PATCH v9 0/8] pci: add pci_iomap_wc() and pci_ioremap_wc_bar() Bjorn Helgaas <bhelgaas@google.com> - 2015-07-22 15:50 +0200

#1189746 — Re: [PATCH v9 0/8] pci: add pci_iomap_wc() and pci_ioremap_wc_bar()

* Bjorn Helgaas <bhelgaas@google.com> wrote:

> > > > Let me know if these are OK or if there are any questions.
> > > > 
> > > > [0] http://lkml.kernel.org/r/20150625204703.GC4898@pd.tnic
> > > > [1] http://lkml.kernel.org/r/20150707095012.GQ7021@wotan.suse.de
> > > 
> > > Ingo,
> > > 
> > > Just a friendly reminder. Let me know if there are any issues or questions.
> > 
> > It would be nice to get an Acked-by from Bjorn for the PCI API bits.
> 
> I think the actual code of pci_ioremap_wc() and pci_ioremap_wc_bar() is fine 
> (although I might have named it pci_ioremap_bar_wc() for consistency).
> 
> I declined to merge or ack them myself because they're obvious extensions of 
> pci_ioremap() and pci_ioremap_bar(), and I would prefer that they be exported 
> the same way, i.e., with EXPORT_SYMBOL(), not EXPORT_SYMBOL_GPL().

Huh? AFAICS pci_ioremap_bar() has been a _GPL export for a long time:

1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       126)#ifdef CONFIG_HAS_IOMEM
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       127)void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       128){
1f7bf3bfb5d60   (Bjorn Helgaas  2015-03-12 12:30:11 -0500       129)    struct resource *res = &pdev->resource[bar];
1f7bf3bfb5d60   (Bjorn Helgaas  2015-03-12 12:30:11 -0500       130)
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       131)    /*
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       132)     * Make sure the BAR is actually a memory resource, not an IO resource
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       133)     */
646c0282df042   (Bjorn Helgaas  2015-03-12 12:30:15 -0500       134)    if (res->flags & IORESOURCE_UNSET || !(res->flags & IORESOURCE_MEM)) {
1f7bf3bfb5d60   (Bjorn Helgaas  2015-03-12 12:30:11 -0500       135)            dev_warn(&pdev->dev, "can't ioremap BAR %d: %pR\n", bar, res);
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       136)            return NULL;
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       137)    }
1f7bf3bfb5d60   (Bjorn Helgaas  2015-03-12 12:30:11 -0500       138)    return ioremap_nocache(res->start, resource_size(res));
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       139)}
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       140)EXPORT_SYMBOL_GPL(pci_ioremap_bar);
1684f5ddd4c0c   (Andrew Morton  2008-12-01 14:30:30 -0800       141)#endif

commit 1684f5ddd4c0c happened in 2008, well before you became PCI maintainer in 
2012.

and I'd prefer keeping the same EXPORT_SYMBOL_GPL() pattern for the new APIs.

(ioremap_wc() is EXPORT_SYMBOL() mostly by accident, it's the odd one out.)

Also, FWIIW: I personally got essentially zero feedback and help from proprietary 
binary kernel module vendors in the past couple of years as x86 maintainer, 
despite a fair chunk of kernel crashes reported on distro kernels occuring in 
them...

Based on that very negative experience, when we introduce something as complex and 
as critical as new caching APIs, the last thing I want is to have obscure bugs in 
binary modules I cannot fix in any reasonable fashion. So even if the parent APIs 
of new APIs weren't already _GPL exports (as in this case), I'd export them as 
_GPL in this case.

> I think using EXPORT_SYMBOL_GPL to express individual political aims rather than 
> as a hint about what might be derived work makes us look like zealots, and 
> that's not my style.

As far as I'm concerned it's a pure technological choice: I don't want to export 
certain types of hard to fix and critical functionality to drivers that I cannot 
then fix.

But I also applied EXPORT_SYMBOL() patches in the past, so I'm not one-sided about 
it.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1189955

Hi Ingo,

On Wed, Jul 22, 2015 at 10:38:45AM +0200, Ingo Molnar wrote:
> 
> * Bjorn Helgaas <bhelgaas@google.com> wrote:
> 
> > > > > Let me know if these are OK or if there are any questions.
> > > > > 
> > > > > [0] http://lkml.kernel.org/r/20150625204703.GC4898@pd.tnic
> > > > > [1] http://lkml.kernel.org/r/20150707095012.GQ7021@wotan.suse.de
> > > > 
> > > > Ingo,
> > > > 
> > > > Just a friendly reminder. Let me know if there are any issues or questions.
> > > 
> > > It would be nice to get an Acked-by from Bjorn for the PCI API bits.
> > 
> > I think the actual code of pci_ioremap_wc() and pci_ioremap_wc_bar() is fine 
> > (although I might have named it pci_ioremap_bar_wc() for consistency).
> > 
> > I declined to merge or ack them myself because they're obvious extensions of 
> > pci_ioremap() and pci_ioremap_bar(), and I would prefer that they be exported 
> > the same way, i.e., with EXPORT_SYMBOL(), not EXPORT_SYMBOL_GPL().
> 
> Huh? AFAICS pci_ioremap_bar() has been a _GPL export for a long time:
> ...
> (ioremap_wc() is EXPORT_SYMBOL() mostly by accident, it's the odd one out.)

You're right, I was mistaken about pci_ioremap_bar().  But I'm not
convinced yet that ioremap_wc() is the odd one out.  All the interfaces I
found, with the exception of ioremap_uc() on x86 and pci_ioremap_bar(), are
EXPORT_SYMBOL(), even the _wc and _wt flavors.

> Also, FWIIW: I personally got essentially zero feedback and help from proprietary 
> binary kernel module vendors in the past couple of years as x86 maintainer, 
> despite a fair chunk of kernel crashes reported on distro kernels occuring in 
> them...
> 
> Based on that very negative experience, when we introduce something as complex and 
> as critical as new caching APIs, the last thing I want is to have obscure bugs in 
> binary modules I cannot fix in any reasonable fashion. So even if the parent APIs 
> of new APIs weren't already _GPL exports (as in this case), I'd export them as 
> _GPL in this case.
> 
> > I think using EXPORT_SYMBOL_GPL to express individual political aims rather than 
> > as a hint about what might be derived work makes us look like zealots, and 
> > that's not my style.
> 
> As far as I'm concerned it's a pure technological choice: I don't want to export 
> certain types of hard to fix and critical functionality to drivers that I cannot 
> then fix.

That's a good argument that I hadn't heard before (or possibly it was there
and I missed it).  It would be stronger still if we could change the parent
APIs similarly.  If a proprietary driver can't use pci_ioremap_wc() because
it's exported _GPL, it's trivial to use ioremap_wc() directly.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web