2008-02-13 22:22:30

by Harvey Harrison

[permalink] [raw]
Subject: [RFC PATCH] feature-removal: add documentation for exported symbols going away

Signed-off-by: Harvey Harrison <[email protected]>
---
Documentation/feature-removal-schedule.txt | 10 ------
Documentation/feature-removal/exported-symbols.txt | 34 ++++++++++++++++++++
arch/x86/kernel/io_delay.c | 2 +-
net/ipv4/inet_hashtables.c | 2 +-
4 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 4d3aa51..b09b193 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -118,16 +118,6 @@ Who: Adrian Bunk <[email protected]>

---------------------------

-What: Unused EXPORT_SYMBOL/EXPORT_SYMBOL_GPL exports
- (temporary transition config option provided until then)
- The transition config option will also be removed at the same time.
-When: before 2.6.19
-Why: Unused symbols are both increasing the size of the kernel binary
- and are often a sign of "wrong API"
-Who: Arjan van de Ven <[email protected]>
-
----------------------------
-
What: vm_ops.nopage
When: Soon, provided in-kernel callers have been converted
Why: This interface is replaced by vm_ops.fault, but it has been around
diff --git a/Documentation/feature-removal/exported-symbols.txt b/Documentation/feature-removal/exported-symbols.txt
new file mode 100644
index 0000000..c847792
--- /dev/null
+++ b/Documentation/feature-removal/exported-symbols.txt
@@ -0,0 +1,34 @@
+The following is a list of symbols whose exports are unused in the kernel
+tree and will be removed. Unused symbols are both increasing the size of
+the kernel binary and are often a sign of a "wrong API"
+
+When adding a symbol, change to EXPORT_UNUSED_SYMBOL{_GPL} in the source
+and schedule for removal in the first stable version the unused symbol is
+marked in + 3. This will give out of tree code ~9 months to stop using the
+export or make a case why the export should be kept.
+
+CONFIG_UNUSED_SYMBOLS is provided to enable unused symbol exports.
+
+Please include the following:
+
+What: the_symbol
+Where: filename the symbol is exported from
+When: next stable version+3
+Why: a reason would be nice
+
+---------------------------
+
+What: __inet_hash_connect
+Where: net/ipv4/inet_hashtables.c
+When: 2.6.28
+Why: No in tree users
+
+---------------------------
+
+What: io_delay_type
+Where: arch/x86/kernel/io_delay.c
+When: 2.6.28
+Why: No in tree users
+
+---------------------------
+
diff --git a/arch/x86/kernel/io_delay.c b/arch/x86/kernel/io_delay.c
index bd49321..a7b8dfb 100644
--- a/arch/x86/kernel/io_delay.c
+++ b/arch/x86/kernel/io_delay.c
@@ -13,7 +13,7 @@
#include <asm/io.h>

int io_delay_type __read_mostly = CONFIG_DEFAULT_IO_DELAY_TYPE;
-EXPORT_SYMBOL_GPL(io_delay_type);
+EXPORT_UNUSED_SYMBOL_GPL(io_delay_type);

static int __initdata io_delay_override;

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 9cac6c0..66d354f 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -494,7 +494,7 @@ out:
return ret;
}
}
-EXPORT_SYMBOL_GPL(__inet_hash_connect);
+EXPORT_UNUSED_SYMBOL_GPL(__inet_hash_connect);

/*
* Bind a port for a connect operation and hash it.
--
1.5.4.1.1278.gc75be



2008-02-13 22:35:28

by Adrian Bunk

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Wed, Feb 13, 2008 at 02:22:06PM -0800, Harvey Harrison wrote:
>...
> --- /dev/null
> +++ b/Documentation/feature-removal/exported-symbols.txt
> @@ -0,0 +1,34 @@
> +The following is a list of symbols whose exports are unused in the kernel
> +tree and will be removed. Unused symbols are both increasing the size of
> +the kernel binary and are often a sign of a "wrong API"
> +
> +When adding a symbol, change to EXPORT_UNUSED_SYMBOL{_GPL} in the source
> +and schedule for removal in the first stable version the unused symbol is
> +marked in + 3. This will give out of tree code ~9 months to stop using the
> +export or make a case why the export should be kept.
> +
> +CONFIG_UNUSED_SYMBOLS is provided to enable unused symbol exports.
> +
> +Please include the following:
> +
> +What: the_symbol
> +Where: filename the symbol is exported from
> +When: next stable version+3
> +Why: a reason would be nice
> +
> +---------------------------
> +
> +What: __inet_hash_connect
> +Where: net/ipv4/inet_hashtables.c
> +When: 2.6.28
> +Why: No in tree users
> +
> +---------------------------
> +
> +What: io_delay_type
> +Where: arch/x86/kernel/io_delay.c
> +When: 2.6.28
> +Why: No in tree users
> +
> +---------------------------
>...

NAK.

It's a well-known and documented fact that we not have a stable API for
modules.

And as long as we anyway break the modules API in so many other cases
with each kernel release there's simply no point in not removing exports
immediately.

Oh, and for increasing the amount of nonsense even further, you should
explain why unused exports that just sneaked into 2.6.25-rc1 must be
shipped in 4 stable kernels before they can be removed.

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

2008-02-13 22:42:47

by Harvey Harrison

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Thu, 2008-02-14 at 00:34 +0200, Adrian Bunk wrote:
> On Wed, Feb 13, 2008 at 02:22:06PM -0800, Harvey Harrison wrote:
> > +
> > +What: __inet_hash_connect
> > +Where: net/ipv4/inet_hashtables.c
> > +When: 2.6.28
> > +Why: No in tree users
> > +
> > +---------------------------
> > +
> > +What: io_delay_type
> > +Where: arch/x86/kernel/io_delay.c
> > +When: 2.6.28
> > +Why: No in tree users
> > +
> > +---------------------------
> >...
>
> NAK.
>
> It's a well-known and documented fact that we not have a stable API for
> modules.
>
> And as long as we anyway break the modules API in so many other cases
> with each kernel release there's simply no point in not removing exports
> immediately.
>
> Oh, and for increasing the amount of nonsense even further, you should
> explain why unused exports that just sneaked into 2.6.25-rc1 must be
> shipped in 4 stable kernels before they can be removed.
>

Without comment on the validity of the exports themselves, I was more
interested in the principle of announcing these removals, and if it
was wanted.

Cheers,

Harvey

2008-02-13 22:43:36

by Adrian Bunk

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

BTW:

Sorry and it's not your fault if my answer was a bit harsh.

Your patch forces a nonsense that until now only Andrew tried to enforce
(which BTW made me avoiding him whenever possible and never looking at
-mm anymore).

There's simply no point in treating the removal of exports differently
from the many other API breaks we have in each release.

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

2008-02-13 22:55:26

by Andrew Morton

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Thu, 14 Feb 2008 00:43:08 +0200
Adrian Bunk <[email protected]> wrote:

> There's simply no point in treating the removal of exports differently
> from the many other API breaks we have in each release.

I have repeatedly and relatively patiently explained to you what the
point is. Simply saying that there isn't one doesn't actually make
it go away.

2008-02-13 22:56:38

by Adrian Bunk

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Wed, Feb 13, 2008 at 02:42:30PM -0800, Harvey Harrison wrote:
> On Thu, 2008-02-14 at 00:34 +0200, Adrian Bunk wrote:
> > On Wed, Feb 13, 2008 at 02:22:06PM -0800, Harvey Harrison wrote:
> > > +
> > > +What: __inet_hash_connect
> > > +Where: net/ipv4/inet_hashtables.c
> > > +When: 2.6.28
> > > +Why: No in tree users
> > > +
> > > +---------------------------
> > > +
> > > +What: io_delay_type
> > > +Where: arch/x86/kernel/io_delay.c
> > > +When: 2.6.28
> > > +Why: No in tree users
> > > +
> > > +---------------------------
> > >...
> >
> > NAK.
> >
> > It's a well-known and documented fact that we not have a stable API for
> > modules.
> >
> > And as long as we anyway break the modules API in so many other cases
> > with each kernel release there's simply no point in not removing exports
> > immediately.
> >
> > Oh, and for increasing the amount of nonsense even further, you should
> > explain why unused exports that just sneaked into 2.6.25-rc1 must be
> > shipped in 4 stable kernels before they can be removed.
>
> Without comment on the validity of the exports themselves, I was more
> interested in the principle of announcing these removals, and if it
> was wanted.

Sorry again for being too harsh (see my other email).

Every kernel release has a big amount of API changes that have not been
announced previously, and that's a fact that is rooted in the kernel
development model.

Otherwise everyone also had to announce now if he wanted to e.g. change
a function prototype or a struct in 2.6.28.

> Cheers,
>
> Harvey

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

2008-02-13 23:08:47

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Wed, 13 Feb 2008 14:22:06 -0800
Harvey Harrison <[email protected]> wrote:

> Signed-off-by: Harvey Harrison <[email protected]>
> ---
> Documentation/feature-removal-schedule.txt | 10 ------
> Documentation/feature-removal/exported-symbols.txt | 34
> ++++++++++++++++++++
> arch/x86/kernel/io_delay.c | 2 +-
> net/ipv4/inet_hashtables.c | 2 +- 4 files
> changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/feature-removal-schedule.txt
> b/Documentation/feature-removal-schedule.txt index 4d3aa51..b09b193
> 100644 --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -118,16 +118,6 @@ Who: Adrian Bunk <[email protected]>
>
> ---------------------------
>
> -What: Unused EXPORT_SYMBOL/EXPORT_SYMBOL_GPL exports
> - (temporary transition config option provided until then)
> - The transition config option will also be removed at the
> same time. -When: before 2.6.19


3 releases is too long imo
1 release is plenty (esp since it's really 1 release + one devel cycle)

2008-02-13 23:23:19

by Adrian Bunk

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Wed, Feb 13, 2008 at 02:54:05PM -0800, Andrew Morton wrote:
> On Thu, 14 Feb 2008 00:43:08 +0200
> Adrian Bunk <[email protected]> wrote:
>
> > There's simply no point in treating the removal of exports differently
> > from the many other API breaks we have in each release.
>
> I have repeatedly and relatively patiently explained to you what the
> point is. Simply saying that there isn't one doesn't actually make
> it go away.

Yes, we had this when you insisted on the deprecation period for
sys_open which still brings me to explosion each time such topics come
up and brought you _very_ near at being the first person ever in my
killfile.

I don't get your point why bigger API changes should still be allowed
without any advance warning while removing an export should require
deprecation periods.

If you want to offer a stable API for external modules then please do
it completely and with all consequences.

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

2008-02-13 23:44:41

by Andrew Morton

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Thu, 14 Feb 2008 01:22:48 +0200
Adrian Bunk <[email protected]> wrote:

> I don't get your point why bigger API changes should still be allowed
> without any advance warning while removing an export should require
> deprecation periods.

Because the cost to us of giving people a few months warning is miniscule,
and doing so a) is courteous to our users and b) possibly avoids driving
away testers.

otoh the cost to us of avoiding large API changes is much much higher, so
the tradeoff has a different outcome.

> If you want to offer a stable API for external modules then please do
> it completely and with all consequences.

Sorry, but you have this belief that if we do X in one case then we should
rigorously do X in all cases. That everything we do sets precedents which
must be forever and in all cases followed. That it is all about rules and
sticking to them. That if you can catch person A taking action B then you
can trap that person into doing B in all cases for ever more.

Well it doesn't work like that. Each case is different and each case has
pros and cons and each one needs to be weighed according to those pros and
cons.

2008-02-13 23:59:20

by Adrian Bunk

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On Wed, Feb 13, 2008 at 03:43:31PM -0800, Andrew Morton wrote:
> On Thu, 14 Feb 2008 01:22:48 +0200
> Adrian Bunk <[email protected]> wrote:
>
> > I don't get your point why bigger API changes should still be allowed
> > without any advance warning while removing an export should require
> > deprecation periods.
>
> Because the cost to us of giving people a few months warning is miniscule,
> and doing so a) is courteous to our users and b) possibly avoids driving
> away testers.

The miniscule cost is my spare time and my nerves.

These unexport discussions are _the_ thing that have turned kernel
development from having been fun into creating anger for me. If your
goal is to bring me to quitting kernel development and spending my spare
time differently you've already gotten quite near to reaching your goal.

> otoh the cost to us of avoiding large API changes is much much higher, so
> the tradeoff has a different outcome.
>
> > If you want to offer a stable API for external modules then please do
> > it completely and with all consequences.
>
> Sorry, but you have this belief that if we do X in one case then we should
> rigorously do X in all cases. That everything we do sets precedents which
> must be forever and in all cases followed. That it is all about rules and
> sticking to them. That if you can catch person A taking action B then you
> can trap that person into doing B in all cases for ever more.
>
> Well it doesn't work like that. Each case is different and each case has
> pros and cons and each one needs to be weighed according to those pros and
> cons.

Unexports are done immediately when there's a subsystem maintainer
taking a patch and deprecation periods are required when a patch has to
go through you...

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

2008-02-14 00:30:51

by Alan

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

> Unexports are done immediately when there's a subsystem maintainer
> taking a patch and deprecation periods are required when a patch has to
> go through you...

Agreed - with the expect of stuff which is used in tree or forms part of
a logical exported API we should just throw them out without messing. The
only exceptions I can see that make sense are

- Where a subsystem maintainer says not to
- Where we know that a commonly used out of tree module needs it

Most of the excess symbols should thus just go away. Its pretty unlikely
that they are being used.

Alan

2008-02-15 03:05:24

by Rene Herman

[permalink] [raw]
Subject: Re: [RFC PATCH] feature-removal: add documentation for exported symbols going away

On 13-02-08 23:22, Harvey Harrison wrote:

> +---------------------------
> +
> +What: io_delay_type
> +Where: arch/x86/kernel/io_delay.c
> +When: 2.6.28
> +Why: No in tree users

The entirety of io_delay.c should be gone long before .28. It's a short term
thing till the port 0x80 problems have been dealt with.

Rene.