[patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Arjan van de Ven]

Hi,
As discussed on lkml before; the patch with the infrastructure to deprecate unused symbols

This is patch one in a series of 17; to not overload lkml the other 16 will be mailed direct;
people who want to see them all can see them at http://www.fenrus.org/unused





This patch temporarily adds EXPORT_UNUSED_SYMBOL and EXPORT_UNUSED_SYMBOL_GPL.
These will be used as transition measure for symbols that aren't used in the 
kernel and are on the way out. When a module uses such a symbol, a warning
is printk'd at modprobe time.

The main reason for removing unused exports is size: eacho export takes roughly
between 100 and 150 bytes of kernel space in the binary. This patch gives
users the option to immediately get this size gain via a config option.

It would be nice to at least get this infrastructure into 2.6.17 even if
the rest of this series won't get there.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 Documentation/feature-removal-schedule.txt |   10 +++
 include/asm-generic/vmlinux.lds.h          |   28 ++++++++++
 include/linux/module.h                     |   20 +++++++
 kernel/module.c                            |   79 +++++++++++++++++++++++++++--
 lib/Kconfig.debug                          |   14 +++++
 5 files changed, 148 insertions(+), 3 deletions(-)

Index: linux-2.6.17-rc3-mm1-unused/Documentation/feature-removal-schedule.txt
===================================================================
--- linux-2.6.17-rc3-mm1-unused.orig/Documentation/feature-removal-schedule.txt
+++ linux-2.6.17-rc3-mm1-unused/Documentation/feature-removal-schedule.txt
@@ -22,6 +22,16 @@ Who:	Adrian Bunk <bunk@stusta.de>
 
 ---------------------------
 
+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 <arjan@linux.intel.com>
+
+---------------------------
+
 What:	RCU API moves to EXPORT_SYMBOL_GPL
 When:	April 2006
 Files:	include/linux/rcupdate.h, kernel/rcupdate.c
Index: linux-2.6.17-rc3-mm1-unused/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux-2.6.17-rc3-mm1-unused.orig/include/asm-generic/vmlinux.lds.h
+++ linux-2.6.17-rc3-mm1-unused/include/asm-generic/vmlinux.lds.h
@@ -58,6 +58,20 @@
 		VMLINUX_SYMBOL(__stop___ksymtab_gpl) = .;		\
 	}								\
 									\
+	/* Kernel symbol table: Normal unused symbols */		\
+	__ksymtab_unused  : AT(ADDR(__ksymtab_unused) - LOAD_OFFSET) {	\
+		VMLINUX_SYMBOL(__start___ksymtab_unused) = .;		\
+		*(__ksymtab_unused)					\
+		VMLINUX_SYMBOL(__stop___ksymtab_unused) = .;		\
+	}								\
+									\
+	/* Kernel symbol table: GPL-only unused symbols */		\
+	__ksymtab_unused_gpl : AT(ADDR(__ksymtab_unused_gpl) - LOAD_OFFSET) { \
+		VMLINUX_SYMBOL(__start___ksymtab_unused_gpl) = .;	\
+		*(__ksymtab_unused_gpl)					\
+		VMLINUX_SYMBOL(__stop___ksymtab_unused_gpl) = .;	\
+	}								\
+									\
 	/* Kernel symbol table: GPL-future-only symbols */		\
 	__ksymtab_gpl_future : AT(ADDR(__ksymtab_gpl_future) - LOAD_OFFSET) { \
 		VMLINUX_SYMBOL(__start___ksymtab_gpl_future) = .;	\
@@ -79,6 +93,20 @@
 		VMLINUX_SYMBOL(__stop___kcrctab_gpl) = .;		\
 	}								\
 									\
+	/* Kernel symbol table: Normal unused symbols */		\
+	__kcrctab_unused  : AT(ADDR(__kcrctab_unused) - LOAD_OFFSET) {	\
+		VMLINUX_SYMBOL(__start___kcrctab_unused) = .;		\
+		*(__kcrctab_unused)					\
+		VMLINUX_SYMBOL(__stop___kcrctab_unused) = .;		\
+	}								\
+									\
+	/* Kernel symbol table: GPL-only unused symbols */		\
+	__kcrctab_unused_gpl : AT(ADDR(__kcrctab_unused_gpl) - LOAD_OFFSET) { \
+		VMLINUX_SYMBOL(__start___kcrctab_unused_gpl) = .;	\
+		*(__kcrctab_unused_gpl)					\
+		VMLINUX_SYMBOL(__stop___kcrctab_unused_gpl) = .;	\
+	}								\
+									\
 	/* Kernel symbol table: GPL-future-only symbols */		\
 	__kcrctab_gpl_future : AT(ADDR(__kcrctab_gpl_future) - LOAD_OFFSET) { \
 		VMLINUX_SYMBOL(__start___kcrctab_gpl_future) = .;	\
Index: linux-2.6.17-rc3-mm1-unused/include/linux/module.h
===================================================================
--- linux-2.6.17-rc3-mm1-unused.orig/include/linux/module.h
+++ linux-2.6.17-rc3-mm1-unused/include/linux/module.h
@@ -204,6 +204,15 @@ void *__symbol_get_gpl(const char *symbo
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)				\
 	__EXPORT_SYMBOL(sym, "_gpl_future")
 
+
+#ifdef CONFIG_UNUSED_SYMBOLS
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#else
+#define EXPORT_UNUSED_SYMBOL(sym)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym)
+#endif
+
 #endif
 
 struct module_ref
@@ -278,6 +287,15 @@ struct module
 	unsigned int num_gpl_syms;
 	const unsigned long *gpl_crcs;
 
+	/* unused exported symbols. */
+	const struct kernel_symbol *unused_syms;
+	unsigned int num_unused_syms;
+	const unsigned long *unused_crcs;
+	/* GPL-only, unused exported symbols. */
+	const struct kernel_symbol *unused_gpl_syms;
+	unsigned int num_unused_gpl_syms;
+	const unsigned long *unused_gpl_crcs;
+
 	/* symbols that will be GPL-only in the near future. */
 	const struct kernel_symbol *gpl_future_syms;
 	unsigned int num_gpl_future_syms;
@@ -470,6 +488,8 @@ void module_remove_driver(struct device_
 #define EXPORT_SYMBOL(sym)
 #define EXPORT_SYMBOL_GPL(sym)
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)
+#define EXPORT_UNUSED_SYMBOL(sym)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
 /* Given an address, look for it in the exception tables. */
 static inline const struct exception_table_entry *
Index: linux-2.6.17-rc3-mm1-unused/kernel/module.c
===================================================================
--- linux-2.6.17-rc3-mm1-unused.orig/kernel/module.c
+++ linux-2.6.17-rc3-mm1-unused/kernel/module.c
@@ -1,4 +1,4 @@
-/* Rewritten by Rusty Russell, on the backs of many others...
+/*
    Copyright (C) 2002 Richard Henderson
    Copyright (C) 2001 Rusty Russell, 2002 Rusty Russell IBM.
 
@@ -120,9 +120,17 @@ extern const struct kernel_symbol __star
 extern const struct kernel_symbol __stop___ksymtab_gpl[];
 extern const struct kernel_symbol __start___ksymtab_gpl_future[];
 extern const struct kernel_symbol __stop___ksymtab_gpl_future[];
+extern const struct kernel_symbol __start___ksymtab_unused[];
+extern const struct kernel_symbol __stop___ksymtab_unused[];
+extern const struct kernel_symbol __start___ksymtab_unused_gpl[];
+extern const struct kernel_symbol __stop___ksymtab_unused_gpl[];
+extern const struct kernel_symbol __start___ksymtab_gpl_future[];
+extern const struct kernel_symbol __stop___ksymtab_gpl_future[];
 extern const unsigned long __start___kcrctab[];
 extern const unsigned long __start___kcrctab_gpl[];
 extern const unsigned long __start___kcrctab_gpl_future[];
+extern const unsigned long __start___kcrctab_unused[];
+extern const unsigned long __start___kcrctab_unused_gpl[];
 
 #ifndef CONFIG_MODVERSIONS
 #define symversion(base, idx) NULL
@@ -142,6 +150,17 @@ static const struct kernel_symbol *looku
 	return NULL;
 }
 
+static void printk_unused_warning(const char *name)
+{
+	printk(KERN_WARNING "Symbol %s is marked as UNUSED, "
+		"however this module is using it. \n", name);
+	printk(KERN_WARNING "This symbol will go away in the future.\n");
+	printk(KERN_WARNING "Please evalute if this is the right api to use, "
+		"and if it really is, submit a report the linux kernel "
+		"mailinglist together with submitting your code for "
+		"inclusion.\n");
+}
+
 /* Find a symbol, return value, crc and module which owns it */
 static unsigned long __find_symbol(const char *name,
 				   struct module **owner,
@@ -184,6 +203,25 @@ static unsigned long __find_symbol(const
 		return ks->value;
 	}
 
+	ks = lookup_symbol(name, __start___ksymtab_unused,
+				 __stop___ksymtab_unused);
+	if (ks) {
+		printk_unused_warning(name);
+		*crc = symversion(__start___kcrctab_unused,
+				  (ks - __start___ksymtab_unused));
+		return ks->value;
+	}
+
+	if (gplok)
+		ks = lookup_symbol(name, __start___ksymtab_unused_gpl,
+				 __stop___ksymtab_unused_gpl);
+	if (ks) {
+		printk_unused_warning(name);
+		*crc = symversion(__start___kcrctab_unused_gpl,
+				  (ks - __start___ksymtab_unused_gpl));
+		return ks->value;
+	}
+
 	/* Now try modules. */ 
 	list_for_each_entry(mod, &modules, list) {
 		*owner = mod;
@@ -202,6 +240,23 @@ static unsigned long __find_symbol(const
 				return ks->value;
 			}
 		}
+		ks = lookup_symbol(name, mod->unused_syms, mod->unused_syms + mod->num_unused_syms);
+		if (ks) {
+			printk_unused_warning(name);
+			*crc = symversion(mod->unused_crcs, (ks - mod->unused_syms));
+			return ks->value;
+		}
+
+		if (gplok) {
+			ks = lookup_symbol(name, mod->unused_gpl_syms,
+					   mod->unused_gpl_syms + mod->num_unused_gpl_syms);
+			if (ks) {
+				printk_unused_warning(name);
+				*crc = symversion(mod->unused_gpl_crcs,
+						  (ks - mod->unused_gpl_syms));
+				return ks->value;
+			}
+		}
 		ks = lookup_symbol(name, mod->gpl_future_syms,
 				   (mod->gpl_future_syms +
 				    mod->num_gpl_future_syms));
@@ -1449,7 +1504,8 @@ static struct module *load_module(void _
 	unsigned int i, symindex = 0, strindex = 0, setupindex, exindex,
 		exportindex, modindex, obsparmindex, infoindex, gplindex,
 		crcindex, gplcrcindex, versindex, pcpuindex, gplfutureindex,
-		gplfuturecrcindex;
+		gplfuturecrcindex, unusedindex, unusedcrcindex, unusedgplindex,
+		unusedgplcrcindex;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -1530,9 +1586,13 @@ static struct module *load_module(void _
 	exportindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab");
 	gplindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_gpl");
 	gplfutureindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_gpl_future");
+	unusedindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_unused");
+	unusedgplindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_unused_gpl");
 	crcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab");
 	gplcrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_gpl");
 	gplfuturecrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_gpl_future");
+	unusedcrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_unused");
+	unusedgplcrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_unused_gpl");
 	setupindex = find_sec(hdr, sechdrs, secstrings, "__param");
 	exindex = find_sec(hdr, sechdrs, secstrings, "__ex_table");
 	obsparmindex = find_sec(hdr, sechdrs, secstrings, "__obsparm");
@@ -1676,14 +1736,27 @@ static struct module *load_module(void _
 		mod->gpl_crcs = (void *)sechdrs[gplcrcindex].sh_addr;
 	mod->num_gpl_future_syms = sechdrs[gplfutureindex].sh_size /
 					sizeof(*mod->gpl_future_syms);
+	mod->num_unused_syms = sechdrs[unusedindex].sh_size /
+					sizeof(*mod->unused_syms);
+	mod->num_unused_gpl_syms = sechdrs[unusedgplindex].sh_size /
+					sizeof(*mod->unused_gpl_syms);
 	mod->gpl_future_syms = (void *)sechdrs[gplfutureindex].sh_addr;
 	if (gplfuturecrcindex)
 		mod->gpl_future_crcs = (void *)sechdrs[gplfuturecrcindex].sh_addr;
 
+	mod->unused_syms = (void *)sechdrs[unusedindex].sh_addr;
+	if (unusedcrcindex)
+		mod->unused_crcs = (void *)sechdrs[unusedcrcindex].sh_addr;
+	mod->unused_gpl_syms = (void *)sechdrs[unusedgplindex].sh_addr;
+	if (unusedgplcrcindex)
+		mod->unused_crcs = (void *)sechdrs[unusedgplcrcindex].sh_addr;
+
 #ifdef CONFIG_MODVERSIONS
 	if ((mod->num_syms && !crcindex) || 
 	    (mod->num_gpl_syms && !gplcrcindex) ||
-	    (mod->num_gpl_future_syms && !gplfuturecrcindex)) {
+	    (mod->num_gpl_future_syms && !gplfuturecrcindex) ||
+	    (mod->num_unused_syms && !unusedcrcindex) ||
+	    (mod->num_unused_gpl_syms && !unusedgplcrcindex)) {
 		printk(KERN_WARNING "%s: No versions for exported symbols."
 		       " Tainting kernel.\n", mod->name);
 		add_taint(TAINT_FORCED_MODULE);
Index: linux-2.6.17-rc3-mm1-unused/lib/Kconfig.debug
===================================================================
--- linux-2.6.17-rc3-mm1-unused.orig/lib/Kconfig.debug
+++ linux-2.6.17-rc3-mm1-unused/lib/Kconfig.debug
@@ -32,6 +32,20 @@ config DEBUG_SHIRQ
 	 to be able to handle interrupts coming in at those points; some don't and
 	 need to be caught.
 
+config UNUSED_SYMBOLS
+	bool "Enable unused/obsolete exported symbols"
+	help
+	  Unused but exported symbols make the kernel needlessly bigger. For that
+	  reason most of these unused exports will soon be removed. This option is
+	  provided temporarily to provide a transition period in case some external
+	  kernel module needs one of these symbols anyway. If you encounter such
+	  a case in your module, consider if you are actually using the right API.
+	  (rationale: since nobody in the kernel is using this in a module, there is
+	  a pretty good chance it's actually the wrong interface to use)
+	  If you really need the symbol, please send a mail to the linux kernel
+	  mailing list mentioning the symbol and why you really need it, and what
+	  the merge plan to the mainline kernel for your module is.
+
 config DEBUG_KERNEL
 	bool "Kernel debugging"
 	help

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Arjan van de Ven]

Randy.Dunlap wrote:
> On Tue, 02 May 2006 16:53:07 +0200 Arjan van de Ven wrote:
> 
>> Hi,
>> As discussed on lkml before; the patch with the infrastructure to deprecate unused symbols
>>
>> This is patch one in a series of 17; to not overload lkml the other 16 will be mailed direct;
>> people who want to see them all can see them at http://www.fenrus.org/unused
>>
>>
>>
>> This patch temporarily adds EXPORT_UNUSED_SYMBOL and EXPORT_UNUSED_SYMBOL_GPL.
>> These will be used as transition measure for symbols that aren't used in the 
>> kernel and are on the way out. When a module uses such a symbol, a warning
>> is printk'd at modprobe time.
>>
>> The main reason for removing unused exports is size: eacho export takes roughly
>> between 100 and 150 bytes of kernel space in the binary. This patch gives
>> users the option to immediately get this size gain via a config option.
> 
> Do the exports take any space at runtime in RAM?

yes; roughly 100 to 150 bytes or so

> scsi patch comments (only one that I have seen) say:
> +EXPORT_UNUSED_SYMBOL(scsi_print_status); /* removal in 2.6.19 */
> 
> and When: above says "before 2.6.19".  Those don't agree.
> Please fix.  Thanks.

there's no conflict actually; they'll be gone in 2.6.19, by removing them just before that ;)

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Randy.Dunlap]

On Tue, 02 May 2006 16:53:07 +0200 Arjan van de Ven wrote:

> Hi,
> As discussed on lkml before; the patch with the infrastructure to deprecate unused symbols
> 
> This is patch one in a series of 17; to not overload lkml the other 16 will be mailed direct;
> people who want to see them all can see them at http://www.fenrus.org/unused
> 
> 
> 
> This patch temporarily adds EXPORT_UNUSED_SYMBOL and EXPORT_UNUSED_SYMBOL_GPL.
> These will be used as transition measure for symbols that aren't used in the 
> kernel and are on the way out. When a module uses such a symbol, a warning
> is printk'd at modprobe time.
> 
> The main reason for removing unused exports is size: eacho export takes roughly
> between 100 and 150 bytes of kernel space in the binary. This patch gives
> users the option to immediately get this size gain via a config option.

Do the exports take any space at runtime in RAM?
or is this only on-disk or wherever the kernel image lives?


> It would be nice to at least get this infrastructure into 2.6.17 even if
> the rest of this series won't get there.

> --- linux-2.6.17-rc3-mm1-unused.orig/Documentation/feature-removal-schedule.txt
> +++ linux-2.6.17-rc3-mm1-unused/Documentation/feature-removal-schedule.txt
> @@ -22,6 +22,16 @@ Who:	Adrian Bunk <bunk@stusta.de>
>  
>  ---------------------------
>  
> +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 <arjan@linux.intel.com>


scsi patch comments (only one that I have seen) say:
+EXPORT_UNUSED_SYMBOL(scsi_print_status); /* removal in 2.6.19 */

and When: above says "before 2.6.19".  Those don't agree.
Please fix.  Thanks.

---
~Randy

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Andrew Morton]

Arjan van de Ven <arjan@linux.intel.com> wrote:
>
>  As discussed on lkml before; the patch with the infrastructure to deprecate unused symbols
> 
>  This is patch one in a series of 17; to not overload lkml the other 16 will be mailed direct;
>  people who want to see them all can see them at http://www.fenrus.org/unused

A lot of these patches go through major APIs and seemingly-randomly prepare
to unexport things based on whether they are presently used within modules.

So, for example, drivers/base/attribute_container.c gets a whole pile of
exports scheduled for removal, regardless of whether the resulting module
API makes *sense*.  Ditto scsi core.  And lib/*.

For example this:

  EXPORT_SYMBOL(generic_getxattr);
 -EXPORT_SYMBOL(generic_listxattr);
 +EXPORT_UNUSED_SYMBOL(generic_listxattr); /* removal in 2.6.19 */
  EXPORT_SYMBOL(generic_setxattr);
  EXPORT_SYMBOL(generic_removexattr);

just seems random to me, and it's setting us up for later churn.

So hum.  Don't you think it'd be better to look at each API as a whole,
make decisions about what parts of it _should_ be offered to modules,
rather then looking empirically at which parts presently _need_ to be
exported?

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Arjan van de Ven]

Andrew Morton wrote:
> So hum.  Don't you think it'd be better to look at each API as a whole,
> make decisions about what parts of it _should_ be offered to modules,
> rather then looking empirically at which parts presently _need_ to be
> exported?

Well so far we as kernel developers have been rather bad at it, with the result
that there are 900 unused ones roughly. Each export takes somewhere between 100
and 150 bytes. *WITHOUT ANY BENEFIT*. The reason to remove them all is to save
that memory NOW. It's easy to add an export back later if it gets used. Yes that
is churn, but it's minor churn. The price for not doing that is a bigger kernel
for everyone, today, without any positive gain of that space..

(and this size excludes even those functions that aren't used at all, but are
only there to be exported. Adrian has been working on removing the really unused
functions in the kernel, via static marking and then gcc noticing the unusedness,
but once they're exported that breaks down)

So I think personally it's worth biting the bullet. I expect 95% of those 900 to
never ever come back. Those 5% will churn, sure. But, to a large degree, the fact
that there's no user is an indication that the API may well not be right in the
first place, or not in demand.

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Jörn Engel]

On Tue, 9 May 2006 18:13:00 +0200, Arjan van de Ven wrote:
> Andrew Morton wrote:
> >So hum.  Don't you think it'd be better to look at each API as a whole,
> >make decisions about what parts of it _should_ be offered to modules,
> >rather then looking empirically at which parts presently _need_ to be
> >exported?
> 
> So I think personally it's worth biting the bullet. I expect 95% of those 
> 900 to
> never ever come back. Those 5% will churn, sure. But, to a large degree, 
> the fact
> that there's no user is an indication that the API may well not be right in 
> the
> first place, or not in demand.

[ Your mailer... ]

Rusty once pointed out that Linux doesn't ever implement complete
interfaces like students are usually told to do.  Instead, only
functions that are actually used are implemented.  Among other things,
this makes sure that implemented code is actually tested and works as
advertised.  Unused code, on the other hand, tends to bitrot.

Well, maybe Rusty was wrong and I shouldn't have listened.  But his
points made a lot of sense back then.  Who knows, it might still make
sense.

Jörn

-- 
This above all: to thine own self be true.
-- Shakespeare

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Alan Cox]

On Maw, 2006-05-09 at 18:13 +0200, Arjan van de Ven wrote:
> Andrew Morton wrote:
> > So hum.  Don't you think it'd be better to look at each API as a whole,
> > make decisions about what parts of it _should_ be offered to modules,
> > rather then looking empirically at which parts presently _need_ to be
> > exported?
> 
> Well so far we as kernel developers have been rather bad at it, with the result
> that there are 900 unused ones roughly. Each export takes somewhere between 100
> and 150 bytes. 

Of course the more technically beneficial approach would be to stop
exports taking such ludicrous amounts of memory. 

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Paul Jackson]

Arjan wrote:
> This is patch one in a series of 17; to not overload lkml the other
> 16 will be mailed direct; people who want to see them all can see
> them at http://www.fenrus.org/unused

Well ... here's one case where your patch series is broken.

Argh - I almost missed this one.  My mailer is setup to tag all
incoming lkml email that mentions the magic word 'cpuset'.  But
it is not setup to catch indirect patches, needless to say.

One of your proposed changes (the only one I reviewed) removed the only
EXPORT_SYMBOL_GPL in kernel/cpuset.c.  That EXPORT is needed because
the routine in question is called from inlines which modules use.

I can't help but wonder how many more such cases your patches miss.

The details ...

Your patch includes this change:


+++++++++++++++++++++++ begin +++++++++++++++++++++++
Index: linux-2.6.17-rc3-mm1-unused/kernel/cpuset.c
===================================================================
--- linux-2.6.17-rc3-mm1-unused.orig/kernel/cpuset.c
+++ linux-2.6.17-rc3-mm1-unused/kernel/cpuset.c
@@ -2338,7 +2338,7 @@ int cpuset_mem_spread_node(void)
 	current->cpuset_mem_spread_rotor = node;
 	return node;
 }
-EXPORT_SYMBOL_GPL(cpuset_mem_spread_node);
+EXPORT_UNUSED_SYMBOL_GPL(cpuset_mem_spread_node); /* removal in 2.6.19 */
 
 /**
  * cpuset_excl_nodes_overlap - Do we overlap @p's mem_exclusive ancestors?
++++++++++++++++++++++++ end +++++++++++++++++++++++


Andrew added this EXPORT, with the following patch:


+++++++++++++++++++++++ begin +++++++++++++++++++++++
From: Andrew Morton <akpm@osdl.org>

It's called from inlines which modules use.

Cc: Paul Jackson <pj@sgi.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 kernel/cpuset.c |    1 +
 1 files changed, 1 insertion(+)

diff -puN kernel/cpuset.c~cpuset-memory-spread-basic-implementation-fix kernel/cpuset.c
--- devel/kernel/cpuset.c~cpuset-memory-spread-basic-implementation-fix 2006-02-06 23:51:0
0.000000000 -0800
+++ devel-akpm/kernel/cpuset.c  2006-02-06 23:51:00.000000000 -0800
@@ -2220,6 +2220,7 @@ int cpuset_mem_spread_node(void)
        current->cpuset_mem_spread_rotor = node;
        return node;
 }
+EXPORT_SYMBOL_GPL(cpuset_mem_spread_node);

 /**
  * cpuset_excl_nodes_overlap - Do we overlap @p's mem_exclusive ancestors?
++++++++++++++++++++++++ end +++++++++++++++++++++++

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Arjan van de Ven]

Paul Jackson wrote:
> Arjan wrote:
>> This is patch one in a series of 17; to not overload lkml the other
>> 16 will be mailed direct; people who want to see them all can see
>> them at http://www.fenrus.org/unused
> 
> Well ... here's one case where your patch series is broken.
> 
> Argh - I almost missed this one.  My mailer is setup to tag all
> incoming lkml email that mentions the magic word 'cpuset'.  But
> it is not setup to catch indirect patches, needless to say.
> 
> One of your proposed changes (the only one I reviewed) removed the only
> EXPORT_SYMBOL_GPL in kernel/cpuset.c.  That EXPORT is needed because
> the routine in question is called from inlines which modules use.

not in the configs I tested at least... but maybe I need to add a specific config to
my set..

Re: [patch 1/17] Infrastructure to mark exported symbols as unused-for-removal-soon

[Paul Jackson]

Arjan wrote:
> not in the configs I tested at least... but maybe I need to add a specific config to
> my set..

Dang - I think you're right.

The original version of the patch that provided the new routine
cpuset_mem_spread_node() on about Feb 2, 2006 required that EXPORT, as
in that version cpuset_mem_spread_node() was called from the inline
versions of page_cache_alloc() and page_cache_alloc_cold().

But the final version of the cpuset_mem_spread_node() patch, on about
Feb 9, 2006, does not seem to require that EXPORT, because the callers
page_cache_alloc() and page_cache_alloc_code() were taken -out-of-line-
for the configurations that made use of cpuset_mem_spread_node().

However the EXPORT had already been added on about Feb 6 or 7, when
everyone and his brother noticed that I had broken the build with my
Feb 2 patch.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401