Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 04:14:56PM +0100, Arjan van de Ven wrote: > Maybe I'm too pessimistic in my assumption that external open module > writers don't actually follow mainline closely; and maybe part of me > would love for them to follow it closer, so close that they would even > consider submitting their driver for it. But sadly from experience... > warnings dont' work. Only when things break hard people notice. We've both done the same job, so it's unsurprising that we share the same view on this. A number of times I've had out of tree module authors complaining to _me_ because "Fedora fucked up my module, it works fine in $otherdistro" (ignoring the fact that $otherdistro is a point release or more behind Fedora). There's definitly a culture-gap between some out-of-tree module authors and people who intend from day 1 to get their code merged into mainline. The former don't know what happens in mainline until several distros start moving onto newer kernels, so no amount of warnings is going to help these people. Dave -- http://www.codemonkey.org.uk - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 04:14:56PM +0100, Arjan van de Ven wrote: Maybe I'm too pessimistic in my assumption that external open module writers don't actually follow mainline closely; and maybe part of me would love for them to follow it closer, so close that they would even consider submitting their driver for it. But sadly from experience... warnings dont' work. Only when things break hard people notice. We've both done the same job, so it's unsurprising that we share the same view on this. A number of times I've had out of tree module authors complaining to _me_ because Fedora fucked up my module, it works fine in $otherdistro (ignoring the fact that $otherdistro is a point release or more behind Fedora). There's definitly a culture-gap between some out-of-tree module authors and people who intend from day 1 to get their code merged into mainline. The former don't know what happens in mainline until several distros start moving onto newer kernels, so no amount of warnings is going to help these people. Dave -- http://www.codemonkey.org.uk - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 05:18:38AM -0700, David Miller wrote:
> From: Christoph Hellwig <[EMAIL PROTECTED]>
> Date: Sun, 9 Sep 2007 21:39:20 +0100
>
> > On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
> > > sys_{open,read} can finally be unexported.
> >
> > Andrew, can you please put this in? Having these exports for syscalls
> > around
> > hsa been a long-time annoyance that can finally be fixed now.
>
> I'm happy to see this go too, but please add the necessary
> export to arch/sparc64/kernel/sparc64_ksyms.S so that the
> solaris system call table reference in
> arch/sparc64/solaris/systbls.S can be satisfied.
>...
This module calls neither sys_open() nor sys_read() directly.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 05:18:38AM -0700, David Miller wrote:
From: Christoph Hellwig [EMAIL PROTECTED]
Date: Sun, 9 Sep 2007 21:39:20 +0100
On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
sys_{open,read} can finally be unexported.
Andrew, can you please put this in? Having these exports for syscalls
around
hsa been a long-time annoyance that can finally be fixed now.
I'm happy to see this go too, but please add the necessary
export to arch/sparc64/kernel/sparc64_ksyms.S so that the
solaris system call table reference in
arch/sparc64/solaris/systbls.S can be satisfied.
...
This module calls neither sys_open() nor sys_read() directly.
cu
Adrian
--
Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
Only a promise, Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 10:54:10AM -0700, Andrew Morton wrote: > > I once made the mistake of putting a "please tell [EMAIL PROTECTED]" printk > in 3c59x.c. My inbox nearly died. Then there's that damned "PCI bus hidden > behind transparent bus" printk which I've actually removed from -mm because > so many people keep reporting it and we don't do anything about it. This has now been fixed in the main tree too. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 10:54:10AM -0700, Andrew Morton wrote: I once made the mistake of putting a please tell [EMAIL PROTECTED] printk in 3c59x.c. My inbox nearly died. Then there's that damned PCI bus hidden behind transparent bus printk which I've actually removed from -mm because so many people keep reporting it and we don't do anything about it. This has now been fixed in the main tree too. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On 09/11/2007 12:41 AM, Adrian Bunk wrote: On Tue, Sep 11, 2007 at 12:15:56AM +0200, Rene Herman wrote: On 09/11/2007 12:18 AM, Adrian Bunk wrote: On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote: There is no benefit in making some rigid set of rules. Is is considered beneficial to provide API stability for external modules or not? If I may... Yes, it is. Just not at any significant cost and Andrew is saying that he considers the _UNUSED() thing not significant. But there is no API stability for external modules this way. I agree that doing things only half is semi-regularly worse than doing them not at all, and this specific case might be the worst example of all, as I read that using sys_open/read is actively harmful, so, well... I read the thread since I tend to keep lots of external crap around. Not in any way that would mean I'd have any grounds for complaining about anything; mostly just driver stuff in various states of completeness that I never seem to get around to cleaning up enough to submit to anyone. But as such, I can comment on the fact that I'm much more likely to notice the warning than I am to notice a thread on LKML, say. How much more likely I'd be to then also actually do anything about it before it just breaks anyway is another matter, but again, well... It simply doesn't make sense to give the few sys_open() abusers even more grace period while changes to the IRQ API affecting nearly everyone are allowed without any requirements of ensuring API stability. I'm not a fan of API stability for external modules, but if API stability was considered important it should be done consequently and not only for some patches that have the bad fate of having to go through Andrew to Linus. In this case I believe it makes sense to just rip it out, but generally it doesn't need to be such a fully robotic yes/no decision, I'd say. Rene. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Tue, Sep 11, 2007 at 12:15:56AM +0200, Rene Herman wrote: > On 09/11/2007 12:18 AM, Adrian Bunk wrote: > >> On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote: > >>> There is no benefit in making some rigid set of rules. >> Is is considered beneficial to provide API stability for external modules >> or not? > > If I may... > > Yes, it is. Just not at any significant cost and Andrew is saying that he > considers the _UNUSED() thing not significant. But there is no API stability for external modules this way. It simply doesn't make sense to give the few sys_open() abusers even more grace period while changes to the IRQ API affecting nearly everyone are allowed without any requirements of ensuring API stability. I'm not a fan of API stability for external modules, but if API stability was considered important it should be done consequently and not only for some patches that have the bad fate of having to go through Andrew to Linus. > Rene. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On 09/11/2007 12:18 AM, Adrian Bunk wrote: On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote: There is no benefit in making some rigid set of rules. Is is considered beneficial to provide API stability for external modules or not? If I may... Yes, it is. Just not at any significant cost and Andrew is saying that he considers the _UNUSED() thing not significant. Rene. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote:
> On Mon, 10 Sep 2007 21:58:21 +0200 Adrian Bunk <[EMAIL PROTECTED]> wrote:
>
> > On Mon, Sep 10, 2007 at 10:25:56AM -0700, Andrew Morton wrote:
> > >...
> > > Also, Adrian goes on and on with weird theories about how I'm picking on
> > > him. But other patches (such as 7d12e780e003f93433d49ce78c) DO OTHER
> > > STUFF. Like simplify the code, and make it smaller, faster or more
> > > maintainable or more reliable.
> >
> > The unexport of sys_{open,read} actually makes the kernel smaller...
> >
> > > So the tradeoff is quite different from a
> > > one-liner which does nothing but kill an export. And, contrary to his
> > > claims, we _do_ put temporary back-compat wrappers in there when we
> > > change interfaces on those relatively rare occasions when it is possible,
> > > and when we remember to do it.
> >
> > Your tradeoff misses the impact on external modules.
> >
> > The unexport of sys_open will not break many modules, while
> > commit 7d12e780e003f93433d49ce78c most likely broke the majority of
> > external modules.
> >
> > Do we guarantee some API stability to module authors or do we not
> > guarantee this?
>
> Neither. We look at each change and make sensible decisions based upon a
> number of factors.
In my experience, the only factor is whether a patch has to go through
you or not...
> > Emphasizing on API stability in the cases that don't matter much while
> > breaking the API in cases that affect most modules doesn't make any
> > sense at all.
> >
> > And your "remember to do it" is an important point. As an example, every
> > change to a struct that is part of the signature of one or exportted
> > functions does change the API of all of these functions. If we offer any
> > API stability for external modules we need to review all patches that
> > touch include/ because many of them contain changes to the modules API
> > that might otherwise get missed.
> >
> > Let's either continue to state that their is no stable API for external
> > modules or define some API stability rules and do whatever is required
> > for implementing them.
>
> There is no benefit in making some rigid set of rules.
Is is considered beneficial to provide API stability for external
modules or not?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 21:58:21 +0200 Adrian Bunk <[EMAIL PROTECTED]> wrote:
> On Mon, Sep 10, 2007 at 10:25:56AM -0700, Andrew Morton wrote:
> >...
> > Also, Adrian goes on and on with weird theories about how I'm picking on
> > him. But other patches (such as 7d12e780e003f93433d49ce78c) DO OTHER
> > STUFF. Like simplify the code, and make it smaller, faster or more
> > maintainable or more reliable.
>
> The unexport of sys_{open,read} actually makes the kernel smaller...
>
> > So the tradeoff is quite different from a
> > one-liner which does nothing but kill an export. And, contrary to his
> > claims, we _do_ put temporary back-compat wrappers in there when we
> > change interfaces on those relatively rare occasions when it is possible,
> > and when we remember to do it.
>
> Your tradeoff misses the impact on external modules.
>
> The unexport of sys_open will not break many modules, while
> commit 7d12e780e003f93433d49ce78c most likely broke the majority of
> external modules.
>
> Do we guarantee some API stability to module authors or do we not
> guarantee this?
Neither. We look at each change and make sensible decisions based upon a
number of factors.
> Emphasizing on API stability in the cases that don't matter much while
> breaking the API in cases that affect most modules doesn't make any
> sense at all.
>
> And your "remember to do it" is an important point. As an example, every
> change to a struct that is part of the signature of one or exportted
> functions does change the API of all of these functions. If we offer any
> API stability for external modules we need to review all patches that
> touch include/ because many of them contain changes to the modules API
> that might otherwise get missed.
>
> Let's either continue to state that their is no stable API for external
> modules or define some API stability rules and do whatever is required
> for implementing them.
There is no benefit in making some rigid set of rules.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 10:25:56AM -0700, Andrew Morton wrote:
>...
> Also, Adrian goes on and on with weird theories about how I'm picking on
> him. But other patches (such as 7d12e780e003f93433d49ce78c) DO OTHER
> STUFF. Like simplify the code, and make it smaller, faster or more
> maintainable or more reliable.
The unexport of sys_{open,read} actually makes the kernel smaller...
> So the tradeoff is quite different from a
> one-liner which does nothing but kill an export. And, contrary to his
> claims, we _do_ put temporary back-compat wrappers in there when we
> change interfaces on those relatively rare occasions when it is possible,
> and when we remember to do it.
Your tradeoff misses the impact on external modules.
The unexport of sys_open will not break many modules, while
commit 7d12e780e003f93433d49ce78c most likely broke the majority of
external modules.
Do we guarantee some API stability to module authors or do we not
guarantee this?
Emphasizing on API stability in the cases that don't matter much while
breaking the API in cases that affect most modules doesn't make any
sense at all.
And your "remember to do it" is an important point. As an example, every
change to a struct that is part of the signature of one or exportted
functions does change the API of all of these functions. If we offer any
API stability for external modules we need to review all patches that
touch include/ because many of them contain changes to the modules API
that might otherwise get missed.
Let's either continue to state that their is no stable API for external
modules or define some API stability rules and do whatever is required
for implementing them.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 18:44:54 +0100 Alan Cox <[EMAIL PROTECTED]> wrote: > > A single kernel release seems sufficient. It gives the maintainers of such > > code time to hear about the breakage and time to fix it. > > Users don't report warnings generally. They won't even see modprobe > warnings or anything in dmesg. Short of using their sound card to scream > "Next release you are screwed" they won't notice (and if you the sound > card trick they'll think they got rooted) > I once made the mistake of putting a "please tell [EMAIL PROTECTED]" printk in 3c59x.c. My inbox nearly died. Then there's that damned "PCI bus hidden behind transparent bus" printk which I've actually removed from -mm because so many people keep reporting it and we don't do anything about it. All it takes is a couple of people to report the problem to the maintainer over a few-weeks period. It doesn't seem unreasonable to expect this to happen. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
> A single kernel release seems sufficient. It gives the maintainers of such > code time to hear about the breakage and time to fix it. Users don't report warnings generally. They won't even see modprobe warnings or anything in dmesg. Short of using their sound card to scream "Next release you are screwed" they won't notice (and if you the sound card trick they'll think they got rooted) Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 13:43:58 +0100 Al Viro <[EMAIL PROTECTED]> wrote: > On Mon, Sep 10, 2007 at 02:23:24AM -0700, Andrew Morton wrote: > > > And I think almost everyone disagrees with you. We just carry too much > > > crap around because of your subborness in this issue, and it gets really > > > annoying to have some high up on the food chain fighting his longly flight > > > against the other people. > > > > I would like to see "everyone" explain what we lose by giving developers a > > bit of warning before we break their stuff. > > > > > > And it would need to be a good explanation, Christoph. A measured one > > which recognises and then weighs the tradeoffs involved, and the costs. An > > inebriated-sounding rant will not impress, sorry. > > Fine, then. Just how much of a warning is needed in this particular case? A single kernel release seems sufficient. It gives the maintainers of such code time to hear about the breakage and time to fix it. > Normally I'm the last one to support Adrian's exportectomy binges, but in > case of sys_open() it's _way_ overdue. We have two ways of warning people about upcoming changes: __deprecated_for_modules will cause a build-time warning and EXPORT_UNUSED_SYMBOL() will generate a modprobe-time warning. A year or so ago we decided that EXPORT_UNUSED_SYMBOL() should be associated with a comment which reminds us when to kill the export. This is all pretty lightweight. > There had been warnings for quite > a while; moreover, fixing the breakage in any case that is not already > badly broken is going to take a few minutes. People who had not found > spare ten minutes during the last few years have my sincere condolences, > but they bloody can do that when it hits the fan and I see no reason to > believe that a warning in build time or modprobe time would do more than > multiple warnings on maillists. You guys keep on talking about developers. It's not to do with them. Fact is, people use external modules. To get their machines working correctly, to get their work done, to do stuff they want done. Many of these people are non-programmers. So when they download a new kernel and find that the module which they use doesn't work because of something which we've done, they get pissed off, and we lose a tester. This has happened many times. Furthermore I have on at least two occasions been working a bug with a reporter who was unable (or unwilling, I forget) to run the latest kernel because we'd broken some out-of-tree code which that tester uses. So, very occasionally the breakage interferes with our ability to fix our bugs. Now, the above two problems are minor ones, but it is very very easy for us to lessen them. We do two things: a) Adrian's patch does -EXPORT_SYMBOL(foo); +EXPORT_SYMBOL_UNUSED(foo); /* Remove in 2.6.25 */ b) Each release we do a grep for EXPORT_SYMBOL_UNUSED and clean up the dead ones. Total cost of this effort: maybe ten developer minutes per release, and a few tens of additional bytes in the released vmlinux. I think that for a few additional testers and a few less-pissed-off users (nothing to do with developers), this cost is justified. That's all. Also, Adrian goes on and on with weird theories about how I'm picking on him. But other patches (such as 7d12e780e003f93433d49ce78c) DO OTHER STUFF. Like simplify the code, and make it smaller, faster or more maintainable or more reliable. So the tradeoff is quite different from a one-liner which does nothing but kill an export. And, contrary to his claims, we _do_ put temporary back-compat wrappers in there when we change interfaces on those relatively rare occasions when it is possible, and when we remember to do it. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 02:23:24 -0700 Andrew Morton <[EMAIL PROTECTED]> wrote: > I would like to see "everyone" explain what we lose by giving > developers a bit of warning before we break their stuff. here's the skinny: 99% of the external module developers won't notice until they're gone, even with the warning. And that's an optimistic estimate. Warnings don't work for this. All this kind of thing buys us is an excuse so that we can say "but we warned you for 3 months". It doesn't actually change anything else. Maybe I'm too pessimistic in my assumption that external open module writers don't actually follow mainline closely; and maybe part of me would love for them to follow it closer, so close that they would even consider submitting their driver for it. But sadly from experience... warnings dont' work. Only when things break hard people notice. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 02:23:24AM -0700, Andrew Morton wrote: > > And I think almost everyone disagrees with you. We just carry too much > > crap around because of your subborness in this issue, and it gets really > > annoying to have some high up on the food chain fighting his longly flight > > against the other people. > > I would like to see "everyone" explain what we lose by giving developers a > bit of warning before we break their stuff. > > > And it would need to be a good explanation, Christoph. A measured one > which recognises and then weighs the tradeoffs involved, and the costs. An > inebriated-sounding rant will not impress, sorry. Fine, then. Just how much of a warning is needed in this particular case? Normally I'm the last one to support Adrian's exportectomy binges, but in case of sys_open() it's _way_ overdue. There had been warnings for quite a while; moreover, fixing the breakage in any case that is not already badly broken is going to take a few minutes. People who had not found spare ten minutes during the last few years have my sincere condolences, but they bloody can do that when it hits the fan and I see no reason to believe that a warning in build time or modprobe time would do more than multiple warnings on maillists. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
> I would like to see "everyone" explain what we lose by giving developers a > bit of warning before we break their stuff. We lose the ability to get anything done on a timescale that makes it happen. Instead we have this continual cycle of remove, akpm says no, submitter forgets, 3 month pause, remove, akpm says no, ... Adrian does now and then try and remove stuff its stupid to remove. People NAK those quite promptly. We've been trying to deprecate sys_open and the other related exports since 1.3.x or thereabouts. There is almost no correct sane way to use these exports either, because your fs struct may be shared so horrible things happen. I'm (minus the language selection) with Christoph "Effing" Hellwig on this one - for these symbols at least. When we break stuff people moan and we can put them back, providing they go into the Linus tree fairly early in the -rc sequence. In the cases we've inadvertently broken stuff before people have moaned fairly quickly too - eg when the tty layer took some inlines into _GPL exports by accident. Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 05:18:38AM -0700, David Miller wrote: > I'm happy to see this go too, but please add the necessary > export to arch/sparc64/kernel/sparc64_ksyms.S so that the > solaris system call table reference in > arch/sparc64/solaris/systbls.S can be satisfied. > > This keeps coming up, and people keep missing the reference > in the solaris compatibility module. Solaris emulation has it's own solaris_open which chains through to sparc32_open, not sys_open. Similarly for read it uses the CHAIN macro which seems to go directly to the syscall table instead of using the sys_read symbol. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
From: Christoph Hellwig <[EMAIL PROTECTED]>
Date: Sun, 9 Sep 2007 21:39:20 +0100
> On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
> > sys_{open,read} can finally be unexported.
>
> Andrew, can you please put this in? Having these exports for syscalls around
> hsa been a long-time annoyance that can finally be fixed now.
I'm happy to see this go too, but please add the necessary
export to arch/sparc64/kernel/sparc64_ksyms.S so that the
solaris system call table reference in
arch/sparc64/solaris/systbls.S can be satisfied.
This keeps coming up, and people keep missing the reference
in the solaris compatibility module.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 02:23:24AM -0700, Andrew Morton wrote: > On Mon, 10 Sep 2007 10:08:08 +0100 Christoph Hellwig <[EMAIL PROTECTED]> > wrote: >... > > > Adrian knows this, yet he habitually sends zero-warning export-removal > > > patches and I habitually ignore them. I guess we must both enjoy this or > > > something. > > > > And I think almost everyone disagrees with you. We just carry too much > > crap around because of your subborness in this issue, and it gets really > > annoying to have some high up on the food chain fighting his longly flight > > against the other people. > > I would like to see "everyone" explain what we lose by giving developers a > bit of warning before we break their stuff. >... If it is really your intention that "everyone" warns external module authors in advance, the interesting part of the process are patches like commit 7d12e780e003f93433d49ce78cfedf4b4c52adc5 that break the API for virtually every module. EXPORT_UNUSED_SYMBOL can't handle such API changes, you'll need to add other parts to the process. In this case it would most likely require adding a new IRQ API plus new APIs in several subsystems where structs changed, mantaining both in parallel for some time, and then dropping the older ones after some time. What we lose is a bit of flexibility when changing kernel code plus a bit of time when adding replacement APIs and maintaining both in parallel. cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 10:08:08 +0100 Christoph Hellwig <[EMAIL PROTECTED]> wrote: > On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: > > Sure. But I think it is better to give people some warning when we're > > planning on breaking out-of-tree things. I do occasionally receive reports > > of "hey, the X driver which I get from Y doesn't work any more". Often > > it's open-source stuff, too. I see no point in irritating our users more > > than > > we need to. > > There is no fucking reason they should have used it in the first place. fucking except fucking we fucking exported fucking it fucking so fucking they fucking used fucking it. fucking. Hey, this would be easier with a sed script! And it so improves the communication! > Until we have something like a coherent export API instead of random crap > exported because someone once needed it to do a dumb thing it'll happen. > Only way to avoid it is to merge the driver and shake out their dumb ideas > in the review process. Well that's why I agreed that we should remove those exports. > > Adrian knows this, yet he habitually sends zero-warning export-removal > > patches and I habitually ignore them. I guess we must both enjoy this or > > something. > > And I think almost everyone disagrees with you. We just carry too much > crap around because of your subborness in this issue, and it gets really > annoying to have some high up on the food chain fighting his longly flight > against the other people. I would like to see "everyone" explain what we lose by giving developers a bit of warning before we break their stuff. And it would need to be a good explanation, Christoph. A measured one which recognises and then weighs the tradeoffs involved, and the costs. An inebriated-sounding rant will not impress, sorry. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: > Sure. But I think it is better to give people some warning when we're > planning on breaking out-of-tree things. I do occasionally receive reports > of "hey, the X driver which I get from Y doesn't work any more". Often > it's open-source stuff, too. I see no point in irritating our users more than > we need to. There is no fucking reason they should have used it in the first place. Until we have something like a coherent export API instead of random crap exported because someone once needed it to do a dumb thing it'll happen. Only way to avoid it is to merge the driver and shake out their dumb ideas in the review process. > Adrian knows this, yet he habitually sends zero-warning export-removal > patches and I habitually ignore them. I guess we must both enjoy this or > something. And I think almost everyone disagrees with you. We just carry too much crap around because of your subborness in this issue, and it gets really annoying to have some high up on the food chain fighting his longly flight against the other people. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: Sure. But I think it is better to give people some warning when we're planning on breaking out-of-tree things. I do occasionally receive reports of hey, the X driver which I get from Y doesn't work any more. Often it's open-source stuff, too. I see no point in irritating our users more than we need to. There is no fucking reason they should have used it in the first place. Until we have something like a coherent export API instead of random crap exported because someone once needed it to do a dumb thing it'll happen. Only way to avoid it is to merge the driver and shake out their dumb ideas in the review process. Adrian knows this, yet he habitually sends zero-warning export-removal patches and I habitually ignore them. I guess we must both enjoy this or something. And I think almost everyone disagrees with you. We just carry too much crap around because of your subborness in this issue, and it gets really annoying to have some high up on the food chain fighting his longly flight against the other people. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 10:08:08 +0100 Christoph Hellwig [EMAIL PROTECTED] wrote: On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: Sure. But I think it is better to give people some warning when we're planning on breaking out-of-tree things. I do occasionally receive reports of hey, the X driver which I get from Y doesn't work any more. Often it's open-source stuff, too. I see no point in irritating our users more than we need to. There is no fucking reason they should have used it in the first place. fucking except fucking we fucking exported fucking it fucking so fucking they fucking used fucking it. fucking. Hey, this would be easier with a sed script! And it so improves the communication! Until we have something like a coherent export API instead of random crap exported because someone once needed it to do a dumb thing it'll happen. Only way to avoid it is to merge the driver and shake out their dumb ideas in the review process. Well that's why I agreed that we should remove those exports. Adrian knows this, yet he habitually sends zero-warning export-removal patches and I habitually ignore them. I guess we must both enjoy this or something. And I think almost everyone disagrees with you. We just carry too much crap around because of your subborness in this issue, and it gets really annoying to have some high up on the food chain fighting his longly flight against the other people. I would like to see everyone explain what we lose by giving developers a bit of warning before we break their stuff. And it would need to be a good explanation, Christoph. A measured one which recognises and then weighs the tradeoffs involved, and the costs. An inebriated-sounding rant will not impress, sorry. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 02:23:24AM -0700, Andrew Morton wrote: On Mon, 10 Sep 2007 10:08:08 +0100 Christoph Hellwig [EMAIL PROTECTED] wrote: ... Adrian knows this, yet he habitually sends zero-warning export-removal patches and I habitually ignore them. I guess we must both enjoy this or something. And I think almost everyone disagrees with you. We just carry too much crap around because of your subborness in this issue, and it gets really annoying to have some high up on the food chain fighting his longly flight against the other people. I would like to see everyone explain what we lose by giving developers a bit of warning before we break their stuff. ... If it is really your intention that everyone warns external module authors in advance, the interesting part of the process are patches like commit 7d12e780e003f93433d49ce78cfedf4b4c52adc5 that break the API for virtually every module. EXPORT_UNUSED_SYMBOL can't handle such API changes, you'll need to add other parts to the process. In this case it would most likely require adding a new IRQ API plus new APIs in several subsystems where structs changed, mantaining both in parallel for some time, and then dropping the older ones after some time. What we lose is a bit of flexibility when changing kernel code plus a bit of time when adding replacement APIs and maintaining both in parallel. cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
From: Christoph Hellwig [EMAIL PROTECTED]
Date: Sun, 9 Sep 2007 21:39:20 +0100
On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
sys_{open,read} can finally be unexported.
Andrew, can you please put this in? Having these exports for syscalls around
hsa been a long-time annoyance that can finally be fixed now.
I'm happy to see this go too, but please add the necessary
export to arch/sparc64/kernel/sparc64_ksyms.S so that the
solaris system call table reference in
arch/sparc64/solaris/systbls.S can be satisfied.
This keeps coming up, and people keep missing the reference
in the solaris compatibility module.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 05:18:38AM -0700, David Miller wrote: I'm happy to see this go too, but please add the necessary export to arch/sparc64/kernel/sparc64_ksyms.S so that the solaris system call table reference in arch/sparc64/solaris/systbls.S can be satisfied. This keeps coming up, and people keep missing the reference in the solaris compatibility module. Solaris emulation has it's own solaris_open which chains through to sparc32_open, not sys_open. Similarly for read it uses the CHAIN macro which seems to go directly to the syscall table instead of using the sys_read symbol. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
I would like to see everyone explain what we lose by giving developers a bit of warning before we break their stuff. We lose the ability to get anything done on a timescale that makes it happen. Instead we have this continual cycle of remove, akpm says no, submitter forgets, 3 month pause, remove, akpm says no, ... Adrian does now and then try and remove stuff its stupid to remove. People NAK those quite promptly. We've been trying to deprecate sys_open and the other related exports since 1.3.x or thereabouts. There is almost no correct sane way to use these exports either, because your fs struct may be shared so horrible things happen. I'm (minus the language selection) with Christoph Effing Hellwig on this one - for these symbols at least. When we break stuff people moan and we can put them back, providing they go into the Linus tree fairly early in the -rc sequence. In the cases we've inadvertently broken stuff before people have moaned fairly quickly too - eg when the tty layer took some inlines into _GPL exports by accident. Alan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 02:23:24AM -0700, Andrew Morton wrote: And I think almost everyone disagrees with you. We just carry too much crap around because of your subborness in this issue, and it gets really annoying to have some high up on the food chain fighting his longly flight against the other people. I would like to see everyone explain what we lose by giving developers a bit of warning before we break their stuff. And it would need to be a good explanation, Christoph. A measured one which recognises and then weighs the tradeoffs involved, and the costs. An inebriated-sounding rant will not impress, sorry. Fine, then. Just how much of a warning is needed in this particular case? Normally I'm the last one to support Adrian's exportectomy binges, but in case of sys_open() it's _way_ overdue. There had been warnings for quite a while; moreover, fixing the breakage in any case that is not already badly broken is going to take a few minutes. People who had not found spare ten minutes during the last few years have my sincere condolences, but they bloody can do that when it hits the fan and I see no reason to believe that a warning in build time or modprobe time would do more than multiple warnings on maillists. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 02:23:24 -0700 Andrew Morton [EMAIL PROTECTED] wrote: I would like to see everyone explain what we lose by giving developers a bit of warning before we break their stuff. here's the skinny: 99% of the external module developers won't notice until they're gone, even with the warning. And that's an optimistic estimate. Warnings don't work for this. All this kind of thing buys us is an excuse so that we can say but we warned you for 3 months. It doesn't actually change anything else. Maybe I'm too pessimistic in my assumption that external open module writers don't actually follow mainline closely; and maybe part of me would love for them to follow it closer, so close that they would even consider submitting their driver for it. But sadly from experience... warnings dont' work. Only when things break hard people notice. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 13:43:58 +0100 Al Viro [EMAIL PROTECTED] wrote: On Mon, Sep 10, 2007 at 02:23:24AM -0700, Andrew Morton wrote: And I think almost everyone disagrees with you. We just carry too much crap around because of your subborness in this issue, and it gets really annoying to have some high up on the food chain fighting his longly flight against the other people. I would like to see everyone explain what we lose by giving developers a bit of warning before we break their stuff. And it would need to be a good explanation, Christoph. A measured one which recognises and then weighs the tradeoffs involved, and the costs. An inebriated-sounding rant will not impress, sorry. Fine, then. Just how much of a warning is needed in this particular case? A single kernel release seems sufficient. It gives the maintainers of such code time to hear about the breakage and time to fix it. Normally I'm the last one to support Adrian's exportectomy binges, but in case of sys_open() it's _way_ overdue. We have two ways of warning people about upcoming changes: __deprecated_for_modules will cause a build-time warning and EXPORT_UNUSED_SYMBOL() will generate a modprobe-time warning. A year or so ago we decided that EXPORT_UNUSED_SYMBOL() should be associated with a comment which reminds us when to kill the export. This is all pretty lightweight. There had been warnings for quite a while; moreover, fixing the breakage in any case that is not already badly broken is going to take a few minutes. People who had not found spare ten minutes during the last few years have my sincere condolences, but they bloody can do that when it hits the fan and I see no reason to believe that a warning in build time or modprobe time would do more than multiple warnings on maillists. You guys keep on talking about developers. It's not to do with them. Fact is, people use external modules. To get their machines working correctly, to get their work done, to do stuff they want done. Many of these people are non-programmers. So when they download a new kernel and find that the module which they use doesn't work because of something which we've done, they get pissed off, and we lose a tester. This has happened many times. Furthermore I have on at least two occasions been working a bug with a reporter who was unable (or unwilling, I forget) to run the latest kernel because we'd broken some out-of-tree code which that tester uses. So, very occasionally the breakage interferes with our ability to fix our bugs. Now, the above two problems are minor ones, but it is very very easy for us to lessen them. We do two things: a) Adrian's patch does -EXPORT_SYMBOL(foo); +EXPORT_SYMBOL_UNUSED(foo); /* Remove in 2.6.25 */ b) Each release we do a grep for EXPORT_SYMBOL_UNUSED and clean up the dead ones. Total cost of this effort: maybe ten developer minutes per release, and a few tens of additional bytes in the released vmlinux. I think that for a few additional testers and a few less-pissed-off users (nothing to do with developers), this cost is justified. That's all. Also, Adrian goes on and on with weird theories about how I'm picking on him. But other patches (such as 7d12e780e003f93433d49ce78c) DO OTHER STUFF. Like simplify the code, and make it smaller, faster or more maintainable or more reliable. So the tradeoff is quite different from a one-liner which does nothing but kill an export. And, contrary to his claims, we _do_ put temporary back-compat wrappers in there when we change interfaces on those relatively rare occasions when it is possible, and when we remember to do it. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
A single kernel release seems sufficient. It gives the maintainers of such code time to hear about the breakage and time to fix it. Users don't report warnings generally. They won't even see modprobe warnings or anything in dmesg. Short of using their sound card to scream Next release you are screwed they won't notice (and if you the sound card trick they'll think they got rooted) Alan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 18:44:54 +0100 Alan Cox [EMAIL PROTECTED] wrote: A single kernel release seems sufficient. It gives the maintainers of such code time to hear about the breakage and time to fix it. Users don't report warnings generally. They won't even see modprobe warnings or anything in dmesg. Short of using their sound card to scream Next release you are screwed they won't notice (and if you the sound card trick they'll think they got rooted) I once made the mistake of putting a please tell [EMAIL PROTECTED] printk in 3c59x.c. My inbox nearly died. Then there's that damned PCI bus hidden behind transparent bus printk which I've actually removed from -mm because so many people keep reporting it and we don't do anything about it. All it takes is a couple of people to report the problem to the maintainer over a few-weeks period. It doesn't seem unreasonable to expect this to happen. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 10:25:56AM -0700, Andrew Morton wrote:
...
Also, Adrian goes on and on with weird theories about how I'm picking on
him. But other patches (such as 7d12e780e003f93433d49ce78c) DO OTHER
STUFF. Like simplify the code, and make it smaller, faster or more
maintainable or more reliable.
The unexport of sys_{open,read} actually makes the kernel smaller...
So the tradeoff is quite different from a
one-liner which does nothing but kill an export. And, contrary to his
claims, we _do_ put temporary back-compat wrappers in there when we
change interfaces on those relatively rare occasions when it is possible,
and when we remember to do it.
Your tradeoff misses the impact on external modules.
The unexport of sys_open will not break many modules, while
commit 7d12e780e003f93433d49ce78c most likely broke the majority of
external modules.
Do we guarantee some API stability to module authors or do we not
guarantee this?
Emphasizing on API stability in the cases that don't matter much while
breaking the API in cases that affect most modules doesn't make any
sense at all.
And your remember to do it is an important point. As an example, every
change to a struct that is part of the signature of one or exportted
functions does change the API of all of these functions. If we offer any
API stability for external modules we need to review all patches that
touch include/ because many of them contain changes to the modules API
that might otherwise get missed.
Let's either continue to state that their is no stable API for external
modules or define some API stability rules and do whatever is required
for implementing them.
cu
Adrian
--
Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
Only a promise, Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote:
On Mon, 10 Sep 2007 21:58:21 +0200 Adrian Bunk [EMAIL PROTECTED] wrote:
On Mon, Sep 10, 2007 at 10:25:56AM -0700, Andrew Morton wrote:
...
Also, Adrian goes on and on with weird theories about how I'm picking on
him. But other patches (such as 7d12e780e003f93433d49ce78c) DO OTHER
STUFF. Like simplify the code, and make it smaller, faster or more
maintainable or more reliable.
The unexport of sys_{open,read} actually makes the kernel smaller...
So the tradeoff is quite different from a
one-liner which does nothing but kill an export. And, contrary to his
claims, we _do_ put temporary back-compat wrappers in there when we
change interfaces on those relatively rare occasions when it is possible,
and when we remember to do it.
Your tradeoff misses the impact on external modules.
The unexport of sys_open will not break many modules, while
commit 7d12e780e003f93433d49ce78c most likely broke the majority of
external modules.
Do we guarantee some API stability to module authors or do we not
guarantee this?
Neither. We look at each change and make sensible decisions based upon a
number of factors.
In my experience, the only factor is whether a patch has to go through
you or not...
Emphasizing on API stability in the cases that don't matter much while
breaking the API in cases that affect most modules doesn't make any
sense at all.
And your remember to do it is an important point. As an example, every
change to a struct that is part of the signature of one or exportted
functions does change the API of all of these functions. If we offer any
API stability for external modules we need to review all patches that
touch include/ because many of them contain changes to the modules API
that might otherwise get missed.
Let's either continue to state that their is no stable API for external
modules or define some API stability rules and do whatever is required
for implementing them.
There is no benefit in making some rigid set of rules.
Is is considered beneficial to provide API stability for external
modules or not?
cu
Adrian
--
Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
Only a promise, Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On 09/11/2007 12:18 AM, Adrian Bunk wrote: On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote: There is no benefit in making some rigid set of rules. Is is considered beneficial to provide API stability for external modules or not? If I may... Yes, it is. Just not at any significant cost and Andrew is saying that he considers the _UNUSED() thing not significant. Rene. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Tue, Sep 11, 2007 at 12:15:56AM +0200, Rene Herman wrote: On 09/11/2007 12:18 AM, Adrian Bunk wrote: On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote: There is no benefit in making some rigid set of rules. Is is considered beneficial to provide API stability for external modules or not? If I may... Yes, it is. Just not at any significant cost and Andrew is saying that he considers the _UNUSED() thing not significant. But there is no API stability for external modules this way. It simply doesn't make sense to give the few sys_open() abusers even more grace period while changes to the IRQ API affecting nearly everyone are allowed without any requirements of ensuring API stability. I'm not a fan of API stability for external modules, but if API stability was considered important it should be done consequently and not only for some patches that have the bad fate of having to go through Andrew to Linus. Rene. cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On 09/11/2007 12:41 AM, Adrian Bunk wrote: On Tue, Sep 11, 2007 at 12:15:56AM +0200, Rene Herman wrote: On 09/11/2007 12:18 AM, Adrian Bunk wrote: On Mon, Sep 10, 2007 at 01:17:59PM -0700, Andrew Morton wrote: There is no benefit in making some rigid set of rules. Is is considered beneficial to provide API stability for external modules or not? If I may... Yes, it is. Just not at any significant cost and Andrew is saying that he considers the _UNUSED() thing not significant. But there is no API stability for external modules this way. I agree that doing things only half is semi-regularly worse than doing them not at all, and this specific case might be the worst example of all, as I read that using sys_open/read is actively harmful, so, well... I read the thread since I tend to keep lots of external crap around. Not in any way that would mean I'd have any grounds for complaining about anything; mostly just driver stuff in various states of completeness that I never seem to get around to cleaning up enough to submit to anyone. But as such, I can comment on the fact that I'm much more likely to notice the warning than I am to notice a thread on LKML, say. How much more likely I'd be to then also actually do anything about it before it just breaks anyway is another matter, but again, well... It simply doesn't make sense to give the few sys_open() abusers even more grace period while changes to the IRQ API affecting nearly everyone are allowed without any requirements of ensuring API stability. I'm not a fan of API stability for external modules, but if API stability was considered important it should be done consequently and not only for some patches that have the bad fate of having to go through Andrew to Linus. In this case I believe it makes sense to just rip it out, but generally it doesn't need to be such a fully robotic yes/no decision, I'd say. Rene. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 11:41:18PM +0100, Arjan van de Ven wrote: > On Mon, 10 Sep 2007 00:22:03 +0200 > Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: > > > On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig > > > <[EMAIL PROTECTED]> wrote: > > > > Adrian knows this, yet he habitually sends zero-warning > > > export-removal patches and I habitually ignore them. I guess we > > > must both enjoy this or something. > > > > You might rename EXPORT_UNUSED_SYMBOL to EXPORT_UNUSED_SYMBOL_ADRIAN > > because AFAIK I am still the only person who was ever dumb enough to > > use it after you wanted me to do so... > > > > Everyone else is allowed to always add, remove and change exports > > as he likes, but I should go through this special process. > > > > It makes no sense (except for keeping me busy) to treat some patches > > I send special while changes made by other people that break the > > modules API are still allowed. > > > > Andrew, please define API rules, IOW rules for addition, removal and > > changing of exported code, that are valid for *everyone* or go to > > hell with your EXPORT_UNUSED_SYMBOL. > > > Adrian, > > as much as I personally disagree with Andrew's policy here (esp for > these symbols, they have been deprecated for years now), it's trivial > to just follow his requirements and get this over with. >... Andrew wants a deprecation period for these symbols where the few users are most likely doing something wrong when using them, but if someone e.g. changes the IRQ API in a way that breaks most external modules no deprecation period is required. If the kernel should get some module API stability processes like EXPORT_UNUSED_SYMBOL() deprecation periods have to be made mandatory for *all* API changes and removals. But forcing EXPORT_UNUSED_SYMBOL on me while allowing other people to do bigger API changes without deprecation periods is not a policy, it's a personal offence. > Greetings, > Arjan van de Ven cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 00:22:03 +0200 Adrian Bunk <[EMAIL PROTECTED]> wrote: > On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: > > On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig > > <[EMAIL PROTECTED]> wrote: > > Adrian knows this, yet he habitually sends zero-warning > > export-removal patches and I habitually ignore them. I guess we > > must both enjoy this or something. > > You might rename EXPORT_UNUSED_SYMBOL to EXPORT_UNUSED_SYMBOL_ADRIAN > because AFAIK I am still the only person who was ever dumb enough to > use it after you wanted me to do so... > > Everyone else is allowed to always add, remove and change exports > as he likes, but I should go through this special process. > > It makes no sense (except for keeping me busy) to treat some patches > I send special while changes made by other people that break the > modules API are still allowed. > > Andrew, please define API rules, IOW rules for addition, removal and > changing of exported code, that are valid for *everyone* or go to > hell with your EXPORT_UNUSED_SYMBOL. Adrian, as much as I personally disagree with Andrew's policy here (esp for these symbols, they have been deprecated for years now), it's trivial to just follow his requirements and get this over with. As for who cares.. I do care still, unused exports make the kernel bigger for everyone, and for the most cases, encourage incorrect APIs to be used by driver writers (they act as a trap; the symbols generally are not used by anything in the kernel because they're the wrong API to use for drivers; having them exported wrongly suggests to new driver writers that they are good things to use). I plan to do another run of finding all unused exports and just marking the lot as _UNUSED exports. (that's not the same as removing them quite yet, but at least it's a good warning for driver writers that they should think twice about the API to see if it's the right one to use) Greetings, Arjan van de Ven - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote:
> On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig <[EMAIL PROTECTED]> wrote:
>
> > On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
> > > sys_{open,read} can finally be unexported.
> >
> > Andrew, can you please put this in? Having these exports for syscalls
> > around
> > hsa been a long-time annoyance that can finally be fixed now.
>
> Sure. But I think it is better to give people some warning when we're
> planning on breaking out-of-tree things. I do occasionally receive reports
> of "hey, the X driver which I get from Y doesn't work any more". Often
> it's open-source stuff, too. I see no point in irritating our users more than
> we need to.
>
> If we're changing an API or removing a function then there's nothing we can
> do, but in the case where we're simply deleting an export, it's exceedingly
> easy for us to EXPORT_UNUSED_SYMBOL (and __deprecated_for_modules?) for a
> few months.
>
> Adrian knows this, yet he habitually sends zero-warning export-removal
> patches and I habitually ignore them. I guess we must both enjoy this or
> something.
You might rename EXPORT_UNUSED_SYMBOL to EXPORT_UNUSED_SYMBOL_ADRIAN
because AFAIK I am still the only person who was ever dumb enough to
use it after you wanted me to do so...
Everyone else is allowed to always add, remove and change exports
as he likes, but I should go through this special process.
It makes no sense (except for keeping me busy) to treat some patches I
send special while changes made by other people that break the modules
API are still allowed.
Andrew, please define API rules, IOW rules for addition, removal and
changing of exported code, that are valid for *everyone* or go to hell
with your EXPORT_UNUSED_SYMBOL.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig <[EMAIL PROTECTED]> wrote:
> On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
> > sys_{open,read} can finally be unexported.
>
> Andrew, can you please put this in? Having these exports for syscalls around
> hsa been a long-time annoyance that can finally be fixed now.
Sure. But I think it is better to give people some warning when we're
planning on breaking out-of-tree things. I do occasionally receive reports
of "hey, the X driver which I get from Y doesn't work any more". Often
it's open-source stuff, too. I see no point in irritating our users more than
we need to.
If we're changing an API or removing a function then there's nothing we can
do, but in the case where we're simply deleting an export, it's exceedingly
easy for us to EXPORT_UNUSED_SYMBOL (and __deprecated_for_modules?) for a
few months.
Adrian knows this, yet he habitually sends zero-warning export-removal
patches and I habitually ignore them. I guess we must both enjoy this or
something.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
> sys_{open,read} can finally be unexported.
Andrew, can you please put this in? Having these exports for syscalls around
hsa been a long-time annoyance that can finally be fixed now.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
sys_{open,read} can finally be unexported.
Andrew, can you please put this in? Having these exports for syscalls around
hsa been a long-time annoyance that can finally be fixed now.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig [EMAIL PROTECTED] wrote:
On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
sys_{open,read} can finally be unexported.
Andrew, can you please put this in? Having these exports for syscalls around
hsa been a long-time annoyance that can finally be fixed now.
Sure. But I think it is better to give people some warning when we're
planning on breaking out-of-tree things. I do occasionally receive reports
of hey, the X driver which I get from Y doesn't work any more. Often
it's open-source stuff, too. I see no point in irritating our users more than
we need to.
If we're changing an API or removing a function then there's nothing we can
do, but in the case where we're simply deleting an export, it's exceedingly
easy for us to EXPORT_UNUSED_SYMBOL (and __deprecated_for_modules?) for a
few months.
Adrian knows this, yet he habitually sends zero-warning export-removal
patches and I habitually ignore them. I guess we must both enjoy this or
something.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote:
On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig [EMAIL PROTECTED] wrote:
On Sun, Sep 09, 2007 at 10:25:28PM +0200, Adrian Bunk wrote:
sys_{open,read} can finally be unexported.
Andrew, can you please put this in? Having these exports for syscalls
around
hsa been a long-time annoyance that can finally be fixed now.
Sure. But I think it is better to give people some warning when we're
planning on breaking out-of-tree things. I do occasionally receive reports
of hey, the X driver which I get from Y doesn't work any more. Often
it's open-source stuff, too. I see no point in irritating our users more than
we need to.
If we're changing an API or removing a function then there's nothing we can
do, but in the case where we're simply deleting an export, it's exceedingly
easy for us to EXPORT_UNUSED_SYMBOL (and __deprecated_for_modules?) for a
few months.
Adrian knows this, yet he habitually sends zero-warning export-removal
patches and I habitually ignore them. I guess we must both enjoy this or
something.
You might rename EXPORT_UNUSED_SYMBOL to EXPORT_UNUSED_SYMBOL_ADRIAN
because AFAIK I am still the only person who was ever dumb enough to
use it after you wanted me to do so...
Everyone else is allowed to always add, remove and change exports
as he likes, but I should go through this special process.
It makes no sense (except for keeping me busy) to treat some patches I
send special while changes made by other people that break the modules
API are still allowed.
Andrew, please define API rules, IOW rules for addition, removal and
changing of exported code, that are valid for *everyone* or go to hell
with your EXPORT_UNUSED_SYMBOL.
cu
Adrian
--
Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
Only a promise, Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 10 Sep 2007 00:22:03 +0200 Adrian Bunk [EMAIL PROTECTED] wrote: On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig [EMAIL PROTECTED] wrote: Adrian knows this, yet he habitually sends zero-warning export-removal patches and I habitually ignore them. I guess we must both enjoy this or something. You might rename EXPORT_UNUSED_SYMBOL to EXPORT_UNUSED_SYMBOL_ADRIAN because AFAIK I am still the only person who was ever dumb enough to use it after you wanted me to do so... Everyone else is allowed to always add, remove and change exports as he likes, but I should go through this special process. It makes no sense (except for keeping me busy) to treat some patches I send special while changes made by other people that break the modules API are still allowed. Andrew, please define API rules, IOW rules for addition, removal and changing of exported code, that are valid for *everyone* or go to hell with your EXPORT_UNUSED_SYMBOL. Adrian, as much as I personally disagree with Andrew's policy here (esp for these symbols, they have been deprecated for years now), it's trivial to just follow his requirements and get this over with. As for who cares.. I do care still, unused exports make the kernel bigger for everyone, and for the most cases, encourage incorrect APIs to be used by driver writers (they act as a trap; the symbols generally are not used by anything in the kernel because they're the wrong API to use for drivers; having them exported wrongly suggests to new driver writers that they are good things to use). I plan to do another run of finding all unused exports and just marking the lot as _UNUSED exports. (that's not the same as removing them quite yet, but at least it's a good warning for driver writers that they should think twice about the API to see if it's the right one to use) Greetings, Arjan van de Ven - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Sun, Sep 09, 2007 at 11:41:18PM +0100, Arjan van de Ven wrote: On Mon, 10 Sep 2007 00:22:03 +0200 Adrian Bunk [EMAIL PROTECTED] wrote: On Sun, Sep 09, 2007 at 02:59:40PM -0700, Andrew Morton wrote: On Sun, 9 Sep 2007 21:39:20 +0100 Christoph Hellwig [EMAIL PROTECTED] wrote: Adrian knows this, yet he habitually sends zero-warning export-removal patches and I habitually ignore them. I guess we must both enjoy this or something. You might rename EXPORT_UNUSED_SYMBOL to EXPORT_UNUSED_SYMBOL_ADRIAN because AFAIK I am still the only person who was ever dumb enough to use it after you wanted me to do so... Everyone else is allowed to always add, remove and change exports as he likes, but I should go through this special process. It makes no sense (except for keeping me busy) to treat some patches I send special while changes made by other people that break the modules API are still allowed. Andrew, please define API rules, IOW rules for addition, removal and changing of exported code, that are valid for *everyone* or go to hell with your EXPORT_UNUSED_SYMBOL. Adrian, as much as I personally disagree with Andrew's policy here (esp for these symbols, they have been deprecated for years now), it's trivial to just follow his requirements and get this over with. ... Andrew wants a deprecation period for these symbols where the few users are most likely doing something wrong when using them, but if someone e.g. changes the IRQ API in a way that breaks most external modules no deprecation period is required. If the kernel should get some module API stability processes like EXPORT_UNUSED_SYMBOL() deprecation periods have to be made mandatory for *all* API changes and removals. But forcing EXPORT_UNUSED_SYMBOL on me while allowing other people to do bigger API changes without deprecation periods is not a policy, it's a personal offence. Greetings, Arjan van de Ven cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Aug 27, 2007 at 03:53:02PM -0700, Arjan van de Ven wrote:
> On Mon, 27 Aug 2007 23:27:23 +0200
> Adrian Bunk <[EMAIL PROTECTED]> wrote:
>
> > On Wed, Aug 22, 2007 at 02:06:48AM -0700, Andrew Morton wrote:
> > >...
> > > Changes since 2.6.23-rc2-mm2:
> > >...
> > > git-alsa.patch
> > >...
> > > git trees
> > >...
> >
> > sys_{open,read} can finally be unexported.
> >
>
> isn't sys_close in the same boat?
Still used in fs/binfmt_misc.c ...
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 27 Aug 2007 23:27:23 +0200
Adrian Bunk <[EMAIL PROTECTED]> wrote:
> On Wed, Aug 22, 2007 at 02:06:48AM -0700, Andrew Morton wrote:
> >...
> > Changes since 2.6.23-rc2-mm2:
> >...
> > git-alsa.patch
> >...
> > git trees
> >...
>
> sys_{open,read} can finally be unexported.
>
isn't sys_close in the same boat?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, 27 Aug 2007 23:27:23 +0200
Adrian Bunk [EMAIL PROTECTED] wrote:
On Wed, Aug 22, 2007 at 02:06:48AM -0700, Andrew Morton wrote:
...
Changes since 2.6.23-rc2-mm2:
...
git-alsa.patch
...
git trees
...
sys_{open,read} can finally be unexported.
isn't sys_close in the same boat?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm patch] unexport sys_{open,read}
On Mon, Aug 27, 2007 at 03:53:02PM -0700, Arjan van de Ven wrote:
On Mon, 27 Aug 2007 23:27:23 +0200
Adrian Bunk [EMAIL PROTECTED] wrote:
On Wed, Aug 22, 2007 at 02:06:48AM -0700, Andrew Morton wrote:
...
Changes since 2.6.23-rc2-mm2:
...
git-alsa.patch
...
git trees
...
sys_{open,read} can finally be unexported.
isn't sys_close in the same boat?
Still used in fs/binfmt_misc.c ...
cu
Adrian
--
Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
Only a promise, Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
