sys_{open,read} can finally be unexported.
Signed-off-by: Adrian Bunk <[email protected]>
---
This patch has been sent on:
- 27 Aug 2007
fs/open.c | 1 -
fs/read_write.c | 1 -
2 files changed, 2 deletions(-)
6f6884f9ee675f2e804c6c58ca46337f9765dd0d
diff --git a/fs/open.c b/fs/open.c
index 23f334d..c0814de 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1057,7 +1057,6 @@ asmlinkage long sys_open(const char __user *filename, int flags, int mode)
prevent_tail_call(ret);
return ret;
}
-EXPORT_SYMBOL_GPL(sys_open);
asmlinkage long sys_openat(int dfd, const char __user *filename, int flags,
int mode)
diff --git a/fs/read_write.c b/fs/read_write.c
index 507ddff..d913d1e 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -370,7 +370,6 @@ asmlinkage ssize_t sys_read(unsigned int fd, char __user * buf, size_t count)
return ret;
}
-EXPORT_SYMBOL_GPL(sys_read);
asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf, size_t count)
{
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.
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.
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
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
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
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.
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.
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
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.
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.
> 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
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.
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.
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.
> 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
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.
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
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.
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
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.
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
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.
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
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
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