| Message ID | 1284764008-19469-1-git-send-email-timur@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Sep 17, 2010 at 6:53 PM, Timur Tabi <timur@freescale.com> wrote: > Export the global variable 'ppc_tb_freq', so that modules (like the Book-E > watchdog driver) can use it. > > Signed-off-by: Timur Tabi <timur@freescale.com> > --- > > This export is necessary for the Book-E watchdog driver to be compiled as a > module. Since ppc_proc_freq is already exported, I figured it's okay for > ppc_tb_freq to be exported as well. > > arch/powerpc/kernel/time.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c > index 8533b3b..49aa130 100644 > --- a/arch/powerpc/kernel/time.c > +++ b/arch/powerpc/kernel/time.c > @@ -163,6 +163,7 @@ static long timezone_offset; > unsigned long ppc_proc_freq; > EXPORT_SYMBOL(ppc_proc_freq); > unsigned long ppc_tb_freq; > +EXPORT_SYMBOL(ppc_tb_freq); EXPORT_SYMBOL_GPL probably, no? josh
On Fri, Sep 17, 2010 at 7:38 PM, Josh Boyer <jwboyer@gmail.com> wrote: >> unsigned long ppc_proc_freq; >> EXPORT_SYMBOL(ppc_proc_freq); >> unsigned long ppc_tb_freq; >> +EXPORT_SYMBOL(ppc_tb_freq); > > EXPORT_SYMBOL_GPL probably, no? I don't see any reason to limit it to GPL drivers. Not only that, but then we'll have this: EXPORT_SYMBOL(ppc_proc_freq); EXPORT_SYMBOL_GPL(ppc_tb_freq); That just looks dumb.
On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote: > I don't see any reason to limit it to GPL drivers. Not only that, but > then we'll have this: I do > EXPORT_SYMBOL(ppc_proc_freq); > EXPORT_SYMBOL_GPL(ppc_tb_freq); > > That just looks dumb. Right, so send a patch to fix the first one too :-) Cheers, Ben.
On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> wrote: > On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote: >> I don't see any reason to limit it to GPL drivers. Not only that, but >> then we'll have this: > > I do Can you elaborate on that, or are you just going to pull rank on me? > >> EXPORT_SYMBOL(ppc_proc_freq); >> EXPORT_SYMBOL_GPL(ppc_tb_freq); >> >> That just looks dumb. > > Right, so send a patch to fix the first one too :-) Then why doesn't someone post a patch to change all EXPORT_SYMBOL to EXPORT_SYMBOL_GPL? And why do we consider EXPORT_SYMBOL to be "broken"? I'm not trying to be a troll, but I see a lot of inconsistency with respect to EXPORT_SYMBOL and EXPORT_SYMBOL_GPL. >
On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote: > On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> wrote: > >> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote: >>> I don't see any reason to limit it to GPL drivers. Not only that, but >>> then we'll have this: >> >> I do > > Can you elaborate on that, or are you just going to pull rank on me? > >> >>> EXPORT_SYMBOL(ppc_proc_freq); >>> EXPORT_SYMBOL_GPL(ppc_tb_freq); >>> >>> That just looks dumb. >> >> Right, so send a patch to fix the first one too :-) I don't think either of these should be EXPORT_SYMBOL_GPL. Why shouldn't a binary module be allowed to know these frequencies? My view is why preclude anyone from using this how they want. If they want to live in the gray area so be it. Who am I to say they shouldn't have that choice. > Then why doesn't someone post a patch to change all EXPORT_SYMBOL to EXPORT_SYMBOL_GPL? And why do we consider EXPORT_SYMBOL to be "broken"? > > I'm not trying to be a troll, but I see a lot of inconsistency with respect to EXPORT_SYMBOL and EXPORT_SYMBOL_GPL. I'm in agreement with the lack of clarity, it seems to be developer whim. - k
On Sat, Sep 18, 2010 at 5:34 PM, Kumar Gala <kumar.gala@freescale.com> wrote:
> I don't think either of these should be EXPORT_SYMBOL_GPL. Why shouldn't a binary module be allowed to know these frequencies? My view is why preclude anyone from using this how they want. If they want to live in the gray area so be it. Who am I to say they shouldn't have that choice.
W00t, binary modules again... No, please. No binary modules in kernelspace.
Thanks,
Vitaly
On Sat, Sep 18, 2010 at 11:34 AM, Kumar Gala <kumar.gala@freescale.com> wrote: > > On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote: > >> On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> wrote: >> >>> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote: >>>> I don't see any reason to limit it to GPL drivers. Not only that, but >>>> then we'll have this: >>> >>> I do >> >> Can you elaborate on that, or are you just going to pull rank on me? >> >>> >>>> EXPORT_SYMBOL(ppc_proc_freq); >>>> EXPORT_SYMBOL_GPL(ppc_tb_freq); >>>> >>>> That just looks dumb. >>> >>> Right, so send a patch to fix the first one too :-) > > I don't think either of these should be EXPORT_SYMBOL_GPL. Why shouldn't a binary module be allowed to know these frequencies? My view is why preclude anyone from using this how they want. If they want to live in the gray area so be it. Who am I to say they shouldn't have that choice. > It is not, in my opinion, about what is technically possible and what isn't. The kernel is licensed under the GPL. This is a Linux kernel only symbol. One would be hard pressed to claim they have a driver that wasn't written for Linux that happens to need that symbol. As a member of the Linux kernel community, I find it important to encourage the contribution of code back to the kernel, and this is one way to help that. This isn't BSD. Besides, a developer is free to export it however they wish in their own kernel tree. They can deviate from mainline if they so choose. josh
Josh Boyer wrote: > It is not, in my opinion, about what is technically possible and what > isn't. The kernel is licensed under the GPL. This is a Linux kernel > only symbol. One would be hard pressed to claim they have a driver > that wasn't written for Linux that happens to need that symbol. As a > member of the Linux kernel community, I find it important to encourage > the contribution of code back to the kernel, and this is one way to > help that. This isn't BSD. Fine, but this goes back to my original question -- if this is how the community feels, then why hasn't someone posted a patch that converts all EXPORT_SYMBOL into EXPORT_SYMBOL_GPL? Either we allow non-GPL drivers, or we don't. If we don't, then we need to eliminate EXPORT_SYMBOL once and for all. Otherwise, the message is hypocritical.
On Sat, Sep 18, 2010 at 1:36 PM, Tabi Timur-B04825 <B04825@freescale.com> wrote: > Josh Boyer wrote: >> It is not, in my opinion, about what is technically possible and what >> isn't. The kernel is licensed under the GPL. This is a Linux kernel >> only symbol. One would be hard pressed to claim they have a driver >> that wasn't written for Linux that happens to need that symbol. As a >> member of the Linux kernel community, I find it important to encourage >> the contribution of code back to the kernel, and this is one way to >> help that. This isn't BSD. > > Fine, but this goes back to my original question -- if this is how the > community feels, then why hasn't someone posted a patch that converts all > EXPORT_SYMBOL into EXPORT_SYMBOL_GPL? Because of history in a lot of cases, and like all communities, opinions vary. I did say this was my opinion, not a mandate of some sort. > Either we allow non-GPL drivers, or we don't. If we don't, then we need to > eliminate EXPORT_SYMBOL once and for all. Otherwise, the message is > hypocritical. I'd be all for it. I don't think it is as black and white as that though, as nothing rarely is. (we can't even get all the code to adhere to the < 80 column "rule" ). I also don't think it is necessarily hypocritical. This is a new symbol being exported, not one that has been exported for years. josh
Josh Boyer wrote: > This is a new symbol being exported, not > one that has been exported for years. Except that Ben says that I should change ppc_proc_freq from EXPORT_SYMBOL to EXPORT_SYMBOL_GPL as well. In a sense, we're in a catch-22. We have three choices: 1. We *arbitrarily* change ppc_proc_freq from EXPORT_SYMBOL to EXPORT_SYMBOL_GPL, so that the two symbols are exported the same way 2. We GPL-export only ppc_tb_freq and leave ppc_proc_freq as-is, but then it looks dumb. 3. We export ppc_tb_freq the same way we're exporting ppc_proc_freq, providing the most options and maintaining consistency. I just don't see how options #1 or #2 are better than #3, and so far the only explanations I've heard are along the lines of "we just like it that way". Obviously, Linus thinks it's okay to allow some non-GPL modules, otherwise he would have long ago changed all EXPORT_SYMBOLs to EXPORT_SYMBOL_GPL. I'm just capitalizing on that mindset.
On Sat, Sep 18, 2010 at 1:55 PM, Tabi Timur-B04825 <B04825@freescale.com> wrote: > Josh Boyer wrote: >> This is a new symbol being exported, not >> one that has been exported for years. > > Except that Ben says that I should change ppc_proc_freq from EXPORT_SYMBOL > to > EXPORT_SYMBOL_GPL as well. In a sense, we're in a catch-22. We have three > choices: > > 1. We *arbitrarily* change ppc_proc_freq from EXPORT_SYMBOL to > EXPORT_SYMBOL_GPL, so that the two symbols are exported the same way > > 2. We GPL-export only ppc_tb_freq and leave ppc_proc_freq as-is, but then it > looks dumb. I dunno. I don't think it looks dumb. It could mean nothing more than we were paying closer attention this time. > 3. We export ppc_tb_freq the same way we're exporting ppc_proc_freq, > providing the most options and maintaining consistency. > > I just don't see how options #1 or #2 are better than #3, and so far the > only > explanations I've heard are along the lines of "we just like it that way". Now I think I've been a bit more detailed than that. I at least explained why I prefer it that way. If you disagree, that's fine but don't make me sound like some kind of petulant child. > Obviously, Linus thinks it's okay to allow some non-GPL modules, otherwise > he > would have long ago changed all EXPORT_SYMBOLs to EXPORT_SYMBOL_GPL. I'm > just capitalizing on that mindset. Capitalizing? The patch you posted that uses this symbol is for a GPL driver so you gain or lose nothing by having this symbol be EXPORT_SYMBOL_GPL. Are you somehow advocating and getting some sort of gain by allowing non-GPL modules? If so, I find that unfortunate. If not, then I guess I don't understand what you mean by capitalizing. josh
On Sep 18, 2010, at 11:56 AM, Josh Boyer wrote: > On Sat, Sep 18, 2010 at 11:34 AM, Kumar Gala <kumar.gala@freescale.com> wrote: >> >> On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote: >> >>> On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> wrote: >>> >>>> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote: >>>>> I don't see any reason to limit it to GPL drivers. Not only that, but >>>>> then we'll have this: >>>> >>>> I do >>> >>> Can you elaborate on that, or are you just going to pull rank on me? >>> >>>> >>>>> EXPORT_SYMBOL(ppc_proc_freq); >>>>> EXPORT_SYMBOL_GPL(ppc_tb_freq); >>>>> >>>>> That just looks dumb. >>>> >>>> Right, so send a patch to fix the first one too :-) >> >> I don't think either of these should be EXPORT_SYMBOL_GPL. Why shouldn't a binary module be allowed to know these frequencies? My view is why preclude anyone from using this how they want. If they want to live in the gray area so be it. Who am I to say they shouldn't have that choice. >> > > It is not, in my opinion, about what is technically possible and what > isn't. The kernel is licensed under the GPL. This is a Linux kernel > only symbol. One would be hard pressed to claim they have a driver > that wasn't written for Linux that happens to need that symbol. As a > member of the Linux kernel community, I find it important to encourage > the contribution of code back to the kernel, and this is one way to > help that. This isn't BSD. > > Besides, a developer is free to export it however they wish in their > own kernel tree. They can deviate from mainline if they so choose. I'll buy this argument as a reason to make both EXPORT_SYMBOL_GPL. - k
On Sat, 2010-09-18 at 10:34 -0500, Kumar Gala wrote: > On Sep 18, 2010, at 9:36 AM, Tabi Timur-B04825 wrote: > > > On Sep 17, 2010, at 10:14 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> wrote: > > > >> On Fri, 2010-09-17 at 20:20 -0500, Timur Tabi wrote: > >>> I don't see any reason to limit it to GPL drivers. Not only that, but > >>> then we'll have this: > >> > >> I do > > > > Can you elaborate on that, or are you just going to pull rank on me? > > > >> > >>> EXPORT_SYMBOL(ppc_proc_freq); > >>> EXPORT_SYMBOL_GPL(ppc_tb_freq); > >>> > >>> That just looks dumb. > >> > >> Right, so send a patch to fix the first one too :-) > > I don't think either of these should be EXPORT_SYMBOL_GPL. Why > shouldn't a binary module be allowed to know these frequencies? My > view is why preclude anyone from using this how they want. If they > want to live in the gray area so be it. Who am I to say they > shouldn't have that choice. Well, I'm all for making binary modules life as hard as possible just for the sake of it :-) Cheers, Ben.
On Sat, 18 Sep 2010 14:22:12 -0400 Josh Boyer <jwboyer@gmail.com> wrote: > Capitalizing? The patch you posted that uses this symbol is for a GPL > driver so you gain or lose nothing by having this symbol be > EXPORT_SYMBOL_GPL. Are you somehow advocating and getting some sort > of gain by allowing non-GPL modules? If so, I find that unfortunate. > If not, then I guess I don't understand what you mean by capitalizing. One can dislike DRM (even a very weak form such as this) without having a particular desire to go outside the bounds of what it allows. I thought EXPORT_SYMBOL_GPL was originally meant to indicate the symbols whose use is likely to be indicitave of code that is, in some copyright-meaningful way, derived from GPL code? I have a hard time seeing that being the case here. If every symbol is made GPL-only, then that just gives the binary-only people[1] more incentive to circumvent the entire mechanism. It loses its meaning. Documentation/DocBook/kernel-hacking.tmpl says, "It implies that the function is considered an internal implementation issue, and not really an interface." -Scott [1] Plus anyone who might want to make a kernel module out of code which is open source, but not under a license the GPLv2 is compatible with.
Hi Scott, > On Sat, 18 Sep 2010 14:22:12 -0400 > Josh Boyer <jwboyer@gmail.com> wrote: > >> Capitalizing? The patch you posted that uses this symbol is for a GPL >> driver so you gain or lose nothing by having this symbol be >> EXPORT_SYMBOL_GPL. Are you somehow advocating and getting some sort >> of gain by allowing non-GPL modules? If so, I find that unfortunate. >> If not, then I guess I don't understand what you mean by capitalizing. > > One can dislike DRM (even a very weak form such as this) without having > a particular desire to go outside the bounds of what it allows. > > I thought EXPORT_SYMBOL_GPL was originally meant to indicate the > symbols whose use is likely to be indicitave of code that is, in some > copyright-meaningful way, derived from GPL code? Google finds this, which coincides with what I remmber[1]: EXPORT_SYMBOL_GPL Some kernel developers are unhappy with providing external interfaces to their code, only to see those interfaces being used by binary only modules. They view it as their work being appropriated. Whether you agree with that view or not is completely irrelevant, the person who owns the copyright decides how their work can be used. EXPORT_SYMBOL_GPL() allows for new interfaces to be marked as only available to modules with a GPL compatible license. This is independent of the kernel tainting, but obviously takes advantage of MODULE_LICENSE() strings. EXPORT_SYMBOL_GPL() may only be used for new exported symbols, Linus has spoken. I believe the phrase involved killer penguins with chainsaws for anybody who changed existing exported interfaces. Cheers Detlev [1] http://lkml.indiana.edu/hypermail/linux/kernel/0110.2/0369.html -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu@denx.de
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 8533b3b..49aa130 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -163,6 +163,7 @@ static long timezone_offset; unsigned long ppc_proc_freq; EXPORT_SYMBOL(ppc_proc_freq); unsigned long ppc_tb_freq; +EXPORT_SYMBOL(ppc_tb_freq); static DEFINE_PER_CPU(u64, last_jiffy);
Export the global variable 'ppc_tb_freq', so that modules (like the Book-E watchdog driver) can use it. Signed-off-by: Timur Tabi <timur@freescale.com> --- This export is necessary for the Book-E watchdog driver to be compiled as a module. Since ppc_proc_freq is already exported, I figured it's okay for ppc_tb_freq to be exported as well. arch/powerpc/kernel/time.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)