[RFC: 2.6 patch] kernel/sys.c: possible cleanups

[RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Adrian Bunk]

This patch contains the following possible cleanups:
- proper prototypes for the following functions:
  - ctrl_alt_del()  (in include/linux/reboot.h)
  - getrusage()     (in include/linux/resource.h)
- make the following needlessly global functions static:
  - kernel_restart_prepare()
  - kernel_kexec()
- remove the following unused EXPORT_SYMBOL:
  - in_egroup_p
- remove the following unused EXPORT_SYMBOL_GPL's:
  - kernel_restart
  - kernel_halt

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

 arch/arm/mach-ixp4xx/nas100d-power.c |    3 +--
 arch/arm/mach-ixp4xx/nslu2-power.c   |    3 +--
 arch/mips/kernel/irixsig.c           |    3 +--
 arch/mips/kernel/sysirix.c           |    2 +-
 arch/um/drivers/mconsole_kern.c      |    2 --
 drivers/char/keyboard.c              |    2 +-
 drivers/s390/char/sclp_quiesce.c     |    3 +--
 include/linux/reboot.h               |    4 ++--
 include/linux/resource.h             |    2 ++
 kernel/exit.c                        |    3 +--
 kernel/sys.c                         |   10 ++--------
 11 files changed, 13 insertions(+), 24 deletions(-)

--- linux-2.6.17-rc1-mm3-full/include/linux/reboot.h.old	2006-04-20 17:10:19.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/include/linux/reboot.h	2006-04-20 17:19:30.000000000 +0200
@@ -59,13 +59,13 @@
  * Architecture independent implemenations of sys_reboot commands.
  */
 
-extern void kernel_restart_prepare(char *cmd);
 extern void kernel_shutdown_prepare(enum system_states state);
 
 extern void kernel_restart(char *cmd);
 extern void kernel_halt(void);
 extern void kernel_power_off(void);
-extern void kernel_kexec(void);
+
+void ctrl_alt_del(void);
 
 /*
  * Emergency restart, callable from an interrupt handler.
--- linux-2.6.17-rc1-mm3-full/include/linux/resource.h.old	2006-04-20 17:24:46.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/include/linux/resource.h	2006-04-20 17:25:18.000000000 +0200
@@ -67,4 +67,6 @@
  */
 #include <asm/resource.h>
 
+int getrusage(struct task_struct *p, int who, struct rusage __user *ru);
+
 #endif
--- linux-2.6.17-rc1-mm3-full/kernel/sys.c.old	2006-04-20 17:10:32.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/kernel/sys.c	2006-04-20 17:16:57.000000000 +0200
@@ -589,7 +589,7 @@
 }
 EXPORT_SYMBOL_GPL(emergency_restart);
 
-void kernel_restart_prepare(char *cmd)
+static void kernel_restart_prepare(char *cmd)
 {
 	blocking_notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
 	system_state = SYSTEM_RESTART;
@@ -615,7 +615,6 @@
 	printk(".\n");
 	machine_restart(cmd);
 }
-EXPORT_SYMBOL_GPL(kernel_restart);
 
 /**
  *	kernel_kexec - reboot the system
@@ -623,7 +622,7 @@
  *	Move into place and start executing a preloaded standalone
  *	executable.  If nothing was preloaded return an error.
  */
-void kernel_kexec(void)
+static void kernel_kexec(void)
 {
 #ifdef CONFIG_KEXEC
 	struct kimage *image;
@@ -637,7 +636,6 @@
 	machine_kexec(image);
 #endif
 }
-EXPORT_SYMBOL_GPL(kernel_kexec);
 
 void kernel_shutdown_prepare(enum system_states state)
 {
@@ -658,8 +656,6 @@
 	machine_halt();
 }
 
-EXPORT_SYMBOL_GPL(kernel_halt);
-
 /**
  *	kernel_power_off - power_off the system
  *
@@ -1666,8 +1662,6 @@
 	return retval;
 }
 
-EXPORT_SYMBOL(in_egroup_p);
-
 DECLARE_RWSEM(uts_sem);
 
 EXPORT_SYMBOL(uts_sem);
--- linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nas100d-power.c.old	2006-04-20 17:19:48.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nas100d-power.c	2006-04-20 17:20:09.000000000 +0200
@@ -20,11 +20,10 @@
 #include <linux/module.h>
 #include <linux/reboot.h>
 #include <linux/interrupt.h>
+#include <linux/reboot.h>
 
 #include <asm/mach-types.h>
 
-extern void ctrl_alt_del(void);
-
 static irqreturn_t nas100d_reset_handler(int irq, void *dev_id, struct pt_regs *regs)
 {
 	/* Signal init to do the ctrlaltdel action, this will bypass init if
--- linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nslu2-power.c.old	2006-04-20 17:20:18.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nslu2-power.c	2006-04-20 17:20:30.000000000 +0200
@@ -20,11 +20,10 @@
 #include <linux/module.h>
 #include <linux/reboot.h>
 #include <linux/interrupt.h>
+#include <linux/reboot.h>
 
 #include <asm/mach-types.h>
 
-extern void ctrl_alt_del(void);
-
 static irqreturn_t nslu2_power_handler(int irq, void *dev_id, struct pt_regs *regs)
 {
 	/* Signal init to do the ctrlaltdel action, this will bypass init if
--- linux-2.6.17-rc1-mm3-full/arch/um/drivers/mconsole_kern.c.old	2006-04-20 17:20:38.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/um/drivers/mconsole_kern.c	2006-04-20 17:20:47.000000000 +0200
@@ -300,8 +300,6 @@
 	machine_restart(NULL);
 }
 
-extern void ctrl_alt_del(void);
-
 void mconsole_cad(struct mc_request *req)
 {
 	mconsole_reply(req, "", 0, 0);
--- linux-2.6.17-rc1-mm3-full/drivers/char/keyboard.c.old	2006-04-20 17:20:55.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/drivers/char/keyboard.c	2006-04-20 17:21:13.000000000 +0200
@@ -39,9 +39,9 @@
 #include <linux/vt_kern.h>
 #include <linux/sysrq.h>
 #include <linux/input.h>
+#include <linux/reboot.h>
 
 static void kbd_disconnect(struct input_handle *handle);
-extern void ctrl_alt_del(void);
 
 /*
  * Exported functions/variables
--- linux-2.6.17-rc1-mm3-full/drivers/s390/char/sclp_quiesce.c.old	2006-04-20 17:21:21.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/drivers/s390/char/sclp_quiesce.c	2006-04-20 17:21:37.000000000 +0200
@@ -13,6 +13,7 @@
 #include <linux/cpumask.h>
 #include <linux/smp.h>
 #include <linux/init.h>
+#include <linux/reboot.h>
 #include <asm/atomic.h>
 #include <asm/ptrace.h>
 #include <asm/sigp.h>
@@ -66,8 +67,6 @@
 }
 #endif
 
-extern void ctrl_alt_del(void);
-
 /* Handler for quiesce event. Start shutdown procedure. */
 static void
 sclp_quiesce_handler(struct evbuf_header *evbuf)
--- linux-2.6.17-rc1-mm3-full/arch/mips/kernel/irixsig.c.old	2006-04-20 17:25:45.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/mips/kernel/irixsig.c	2006-04-20 17:26:04.000000000 +0200
@@ -13,6 +13,7 @@
 #include <linux/smp_lock.h>
 #include <linux/time.h>
 #include <linux/ptrace.h>
+#include <linux/resource.h>
 
 #include <asm/ptrace.h>
 #include <asm/uaccess.h>
@@ -540,8 +541,6 @@
 #define IRIX_P_PGID   2
 #define IRIX_P_ALL    7
 
-extern int getrusage(struct task_struct *, int, struct rusage __user *);
-
 #define W_EXITED     1
 #define W_TRAPPED    2
 #define W_STOPPED    4
--- linux-2.6.17-rc1-mm3-full/arch/mips/kernel/sysirix.c.old	2006-04-20 17:26:23.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/mips/kernel/sysirix.c	2006-04-20 17:26:53.000000000 +0200
@@ -31,6 +31,7 @@
 #include <linux/socket.h>
 #include <linux/security.h>
 #include <linux/syscalls.h>
+#include <linux/resource.h>
 
 #include <asm/ptrace.h>
 #include <asm/page.h>
@@ -235,7 +236,6 @@
 #undef DEBUG_PROCGRPS
 
 extern unsigned long irix_mapelf(int fd, struct elf_phdr __user *user_phdrp, int cnt);
-extern int getrusage(struct task_struct *p, int who, struct rusage __user *ru);
 extern char *prom_getenv(char *name);
 extern long prom_setenv(char *name, char *value);
 
--- linux-2.6.17-rc1-mm3-full/kernel/exit.c.old	2006-04-20 17:27:07.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/kernel/exit.c	2006-04-20 17:27:24.000000000 +0200
@@ -37,6 +37,7 @@
 #include <linux/compat.h>
 #include <linux/pipe_fs_i.h>
 #include <linux/audit.h> /* for audit_free() */
+#include <linux/resource.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -46,8 +47,6 @@
 extern void sem_exit (void);
 extern struct task_struct *child_reaper;
 
-int getrusage(struct task_struct *, int, struct rusage __user *);
-
 static void exit_mm(struct task_struct * tsk);
 
 static void __unhash_process(struct task_struct *p)

[RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Adrian Bunk]

This patch contains the following possible cleanups:
- proper prototypes for the following functions:
  - ctrl_alt_del()  (in include/linux/reboot.h)
  - getrusage()     (in include/linux/resource.h)
- make the following needlessly global functions static:
  - kernel_restart_prepare()
  - kernel_kexec()
- remove the following unused EXPORT_SYMBOL:
  - in_egroup_p
- remove the following unused EXPORT_SYMBOL_GPL's:
  - kernel_restart
  - kernel_halt

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

This patch was already sent on:
- 20 Apr 2006

 arch/arm/mach-ixp4xx/nas100d-power.c |    3 +--
 arch/arm/mach-ixp4xx/nslu2-power.c   |    3 +--
 arch/mips/kernel/irixsig.c           |    3 +--
 arch/mips/kernel/sysirix.c           |    2 +-
 arch/um/drivers/mconsole_kern.c      |    2 --
 drivers/char/keyboard.c              |    2 +-
 drivers/s390/char/sclp_quiesce.c     |    3 +--
 include/linux/reboot.h               |    4 ++--
 include/linux/resource.h             |    2 ++
 kernel/exit.c                        |    3 +--
 kernel/sys.c                         |   10 ++--------
 11 files changed, 13 insertions(+), 24 deletions(-)

--- linux-2.6.17-rc1-mm3-full/include/linux/reboot.h.old	2006-04-20 17:10:19.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/include/linux/reboot.h	2006-04-20 17:19:30.000000000 +0200
@@ -59,13 +59,13 @@
  * Architecture independent implemenations of sys_reboot commands.
  */
 
-extern void kernel_restart_prepare(char *cmd);
 extern void kernel_shutdown_prepare(enum system_states state);
 
 extern void kernel_restart(char *cmd);
 extern void kernel_halt(void);
 extern void kernel_power_off(void);
-extern void kernel_kexec(void);
+
+void ctrl_alt_del(void);
 
 /*
  * Emergency restart, callable from an interrupt handler.
--- linux-2.6.17-rc1-mm3-full/include/linux/resource.h.old	2006-04-20 17:24:46.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/include/linux/resource.h	2006-04-20 17:25:18.000000000 +0200
@@ -67,4 +67,6 @@
  */
 #include <asm/resource.h>
 
+int getrusage(struct task_struct *p, int who, struct rusage __user *ru);
+
 #endif
--- linux-2.6.17-rc1-mm3-full/kernel/sys.c.old	2006-04-20 17:10:32.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/kernel/sys.c	2006-04-20 17:16:57.000000000 +0200
@@ -589,7 +589,7 @@
 }
 EXPORT_SYMBOL_GPL(emergency_restart);
 
-void kernel_restart_prepare(char *cmd)
+static void kernel_restart_prepare(char *cmd)
 {
 	blocking_notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
 	system_state = SYSTEM_RESTART;
@@ -615,7 +615,6 @@
 	printk(".\n");
 	machine_restart(cmd);
 }
-EXPORT_SYMBOL_GPL(kernel_restart);
 
 /**
  *	kernel_kexec - reboot the system
@@ -623,7 +622,7 @@
  *	Move into place and start executing a preloaded standalone
  *	executable.  If nothing was preloaded return an error.
  */
-void kernel_kexec(void)
+static void kernel_kexec(void)
 {
 #ifdef CONFIG_KEXEC
 	struct kimage *image;
@@ -637,7 +636,6 @@
 	machine_kexec(image);
 #endif
 }
-EXPORT_SYMBOL_GPL(kernel_kexec);
 
 void kernel_shutdown_prepare(enum system_states state)
 {
@@ -658,8 +656,6 @@
 	machine_halt();
 }
 
-EXPORT_SYMBOL_GPL(kernel_halt);
-
 /**
  *	kernel_power_off - power_off the system
  *
@@ -1666,8 +1662,6 @@
 	return retval;
 }
 
-EXPORT_SYMBOL(in_egroup_p);
-
 DECLARE_RWSEM(uts_sem);
 
 EXPORT_SYMBOL(uts_sem);
--- linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nas100d-power.c.old	2006-04-20 17:19:48.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nas100d-power.c	2006-04-20 17:20:09.000000000 +0200
@@ -20,11 +20,10 @@
 #include <linux/module.h>
 #include <linux/reboot.h>
 #include <linux/interrupt.h>
+#include <linux/reboot.h>
 
 #include <asm/mach-types.h>
 
-extern void ctrl_alt_del(void);
-
 static irqreturn_t nas100d_reset_handler(int irq, void *dev_id, struct pt_regs *regs)
 {
 	/* Signal init to do the ctrlaltdel action, this will bypass init if
--- linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nslu2-power.c.old	2006-04-20 17:20:18.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/arm/mach-ixp4xx/nslu2-power.c	2006-04-20 17:20:30.000000000 +0200
@@ -20,11 +20,10 @@
 #include <linux/module.h>
 #include <linux/reboot.h>
 #include <linux/interrupt.h>
+#include <linux/reboot.h>
 
 #include <asm/mach-types.h>
 
-extern void ctrl_alt_del(void);
-
 static irqreturn_t nslu2_power_handler(int irq, void *dev_id, struct pt_regs *regs)
 {
 	/* Signal init to do the ctrlaltdel action, this will bypass init if
--- linux-2.6.17-rc1-mm3-full/arch/um/drivers/mconsole_kern.c.old	2006-04-20 17:20:38.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/um/drivers/mconsole_kern.c	2006-04-20 17:20:47.000000000 +0200
@@ -300,8 +300,6 @@
 	machine_restart(NULL);
 }
 
-extern void ctrl_alt_del(void);
-
 void mconsole_cad(struct mc_request *req)
 {
 	mconsole_reply(req, "", 0, 0);
--- linux-2.6.17-rc1-mm3-full/drivers/char/keyboard.c.old	2006-04-20 17:20:55.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/drivers/char/keyboard.c	2006-04-20 17:21:13.000000000 +0200
@@ -39,9 +39,9 @@
 #include <linux/vt_kern.h>
 #include <linux/sysrq.h>
 #include <linux/input.h>
+#include <linux/reboot.h>
 
 static void kbd_disconnect(struct input_handle *handle);
-extern void ctrl_alt_del(void);
 
 /*
  * Exported functions/variables
--- linux-2.6.17-rc1-mm3-full/drivers/s390/char/sclp_quiesce.c.old	2006-04-20 17:21:21.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/drivers/s390/char/sclp_quiesce.c	2006-04-20 17:21:37.000000000 +0200
@@ -13,6 +13,7 @@
 #include <linux/cpumask.h>
 #include <linux/smp.h>
 #include <linux/init.h>
+#include <linux/reboot.h>
 #include <asm/atomic.h>
 #include <asm/ptrace.h>
 #include <asm/sigp.h>
@@ -66,8 +67,6 @@
 }
 #endif
 
-extern void ctrl_alt_del(void);
-
 /* Handler for quiesce event. Start shutdown procedure. */
 static void
 sclp_quiesce_handler(struct evbuf_header *evbuf)
--- linux-2.6.17-rc1-mm3-full/arch/mips/kernel/irixsig.c.old	2006-04-20 17:25:45.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/mips/kernel/irixsig.c	2006-04-20 17:26:04.000000000 +0200
@@ -13,6 +13,7 @@
 #include <linux/smp_lock.h>
 #include <linux/time.h>
 #include <linux/ptrace.h>
+#include <linux/resource.h>
 
 #include <asm/ptrace.h>
 #include <asm/uaccess.h>
@@ -540,8 +541,6 @@
 #define IRIX_P_PGID   2
 #define IRIX_P_ALL    7
 
-extern int getrusage(struct task_struct *, int, struct rusage __user *);
-
 #define W_EXITED     1
 #define W_TRAPPED    2
 #define W_STOPPED    4
--- linux-2.6.17-rc1-mm3-full/arch/mips/kernel/sysirix.c.old	2006-04-20 17:26:23.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/arch/mips/kernel/sysirix.c	2006-04-20 17:26:53.000000000 +0200
@@ -31,6 +31,7 @@
 #include <linux/socket.h>
 #include <linux/security.h>
 #include <linux/syscalls.h>
+#include <linux/resource.h>
 
 #include <asm/ptrace.h>
 #include <asm/page.h>
@@ -235,7 +236,6 @@
 #undef DEBUG_PROCGRPS
 
 extern unsigned long irix_mapelf(int fd, struct elf_phdr __user *user_phdrp, int cnt);
-extern int getrusage(struct task_struct *p, int who, struct rusage __user *ru);
 extern char *prom_getenv(char *name);
 extern long prom_setenv(char *name, char *value);
 
--- linux-2.6.17-rc1-mm3-full/kernel/exit.c.old	2006-04-20 17:27:07.000000000 +0200
+++ linux-2.6.17-rc1-mm3-full/kernel/exit.c	2006-04-20 17:27:24.000000000 +0200
@@ -37,6 +37,7 @@
 #include <linux/compat.h>
 #include <linux/pipe_fs_i.h>
 #include <linux/audit.h> /* for audit_free() */
+#include <linux/resource.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -46,8 +47,6 @@
 extern void sem_exit (void);
 extern struct task_struct *child_reaper;
 
-int getrusage(struct task_struct *, int, struct rusage __user *);
-
 static void exit_mm(struct task_struct * tsk);
 
 static void __unhash_process(struct task_struct *p)

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Andrew Morton]

Adrian Bunk <bunk@stusta.de> wrote:
>
> This patch contains the following possible cleanups:

Please avoid mixing together cleanups

>  - proper prototypes for the following functions:
>    - ctrl_alt_del()  (in include/linux/reboot.h)
>    - getrusage()     (in include/linux/resource.h)
>  - make the following needlessly global functions static:
>    - kernel_restart_prepare()
>    - kernel_kexec()

which I will apply, together with API changes

>  - remove the following unused EXPORT_SYMBOL:
>    - in_egroup_p
>  - remove the following unused EXPORT_SYMBOL_GPL's:
>    - kernel_restart
>    - kernel_halt

which I will not.

We have a process for the latter.  And even if we ignore that process, the
patch ends up sitting in -mm for ages because of the API change, along with
the cleanups, which could be merged up promptly.

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Adrian Bunk]

On Mon, May 01, 2006 at 12:18:03AM -0700, Andrew Morton wrote:
> Adrian Bunk <bunk@stusta.de> wrote:
> >
> > This patch contains the following possible cleanups:
> 
> Please avoid mixing together cleanups
> 
> >  - proper prototypes for the following functions:
> >    - ctrl_alt_del()  (in include/linux/reboot.h)
> >    - getrusage()     (in include/linux/resource.h)
> >  - make the following needlessly global functions static:
> >    - kernel_restart_prepare()
> >    - kernel_kexec()
> 
> which I will apply, together with API changes

Are you splitting the patch yourself or should I send a splitted patch?

> >  - remove the following unused EXPORT_SYMBOL:
> >    - in_egroup_p
> >  - remove the following unused EXPORT_SYMBOL_GPL's:
> >    - kernel_restart
> >    - kernel_halt
> 
> which I will not.
> 
> We have a process for the latter.  And even if we ignore that process, the
> patch ends up sitting in -mm for ages because of the API change, along with
> the cleanups, which could be merged up promptly.

The problem is that we have a lack of a process at the other end:

There is no process to review added exports.

And there are so many exports added with "we will soon use them".

If removing exports requires a process, adding exports requires a 
similar process.

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

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Andrew Morton]

Adrian Bunk <bunk@stusta.de> wrote:
>
> On Mon, May 01, 2006 at 12:18:03AM -0700, Andrew Morton wrote:
> > Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > This patch contains the following possible cleanups:
> > 
> > Please avoid mixing together cleanups
> > 
> > >  - proper prototypes for the following functions:
> > >    - ctrl_alt_del()  (in include/linux/reboot.h)
> > >    - getrusage()     (in include/linux/resource.h)
> > >  - make the following needlessly global functions static:
> > >    - kernel_restart_prepare()
> > >    - kernel_kexec()
> > 
> > which I will apply, together with API changes
> 
> Are you splitting the patch yourself or should I send a splitted patch?

I currently have queued:

drivers/char/hw_random.c: remove assert()'s
drivers/char/applicom.c: proper module_{init,exit}
fs/buffer.c: possible cleanups
fs/open.c: unexport sys_openat

So please redo and resend the rest.

No particular hurry - I'll be out of the patch business for the next week.

> > >  - remove the following unused EXPORT_SYMBOL:
> > >    - in_egroup_p
> > >  - remove the following unused EXPORT_SYMBOL_GPL's:
> > >    - kernel_restart
> > >    - kernel_halt
> > 
> > which I will not.
> > 
> > We have a process for the latter.  And even if we ignore that process, the
> > patch ends up sitting in -mm for ages because of the API change, along with
> > the cleanups, which could be merged up promptly.
> 
> The problem is that we have a lack of a process at the other end:
> 
> There is no process to review added exports.

Yes there is - I and many others frequently query them.  Sure, sometimes
stuff slips through.  But it's a very very minor problem.

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Arjan van de Ven]

> > We have a process for the latter.  And even if we ignore that process, the
> > patch ends up sitting in -mm for ages because of the API change, along with
> > the cleanups, which could be merged up promptly.
> 
> The problem is that we have a lack of a process at the other end:
> 
> There is no process to review added exports.
> 
> And there are so many exports added with "we will soon use them".

.. and many never will, leading to about 900 unused ones, each taking
quite a bit of space.

Some are really stupid (eg sys_openat export is just braindead, sys_open
was temporarily exported until all in-kernel users were switched over to
the firmware loading api, sys_openat seems to just have blindly copied
this without thinking)


> If removing exports requires a process, adding exports requires a 
> similar process.

alternatively we should bite the bullet, and just stick those 900 on the
"we'll kill all these in 3 months" list, have a thing to disable them
now via a config option (so that people actually notice rather than just
having them in the depreciation file) and fix the 5 or 10 or so that
actually will be used soon in those 3 months.

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Andrew Morton]

Arjan van de Ven <arjan@infradead.org> wrote:
>
> > If removing exports requires a process, adding exports requires a 
>  > similar process.
> 
>  alternatively we should bite the bullet, and just stick those 900 on the
>  "we'll kill all these in 3 months" list, have a thing to disable them
>  now via a config option (so that people actually notice rather than just
>  having them in the depreciation file) and fix the 5 or 10 or so that
>  actually will be used soon in those 3 months.
> 

I'd instead suggest that we implement a new EXPORT_SYMBOL_UNEXPORT_SCHEDULED
(?) and use that.  Suitable compile-time and modprobe-time warnings would
be needed.  Put the unexport date in a comment at the
EXPORT_SYMBOL_UNEXPORT_SCHEDULED site or even in the modprobe-time warning
message, if that's convenient:

EXPORT_SYMBOL_UNEXPORT_SCHEDULED(foo, "Dec 2006");

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Arjan van de Ven]

On Mon, 2006-05-01 at 00:49 -0700, Andrew Morton wrote:
> Arjan van de Ven <arjan@infradead.org> wrote:
> >
> > > If removing exports requires a process, adding exports requires a 
> >  > similar process.
> > 
> >  alternatively we should bite the bullet, and just stick those 900 on the
> >  "we'll kill all these in 3 months" list, have a thing to disable them
> >  now via a config option (so that people actually notice rather than just
> >  having them in the depreciation file) and fix the 5 or 10 or so that
> >  actually will be used soon in those 3 months.
> > 
> 
> I'd instead suggest that we implement a new EXPORT_SYMBOL_UNEXPORT_SCHEDULED

EXPORT_UNUSED_SYMBOL() ? (with comment of date)

(well you didn't apply the patch for that I sent you so I suppose you
hate it ;)


> (?) and use that.  Suitable compile-time and modprobe-time warnings would

compiletime warning is hard, because it would require changing
prototype, which is a lot more churn, and I'll bet a lot of exports
don't even have a prototype at all. modprobe time is easy. (middle
ground would be depmod time; that's almost compile time I suppose but a
lot easier, but might need modutils help)

> be needed.  Put the unexport date in a comment at the
> EXPORT_SYMBOL_UNEXPORT_SCHEDULED site or even in the modprobe-time warning
> message, if that's convenient:
> 
> EXPORT_SYMBOL_UNEXPORT_SCHEDULED(foo, "Dec 2006");

comment is easy, putting a date in is really sucky since it grows the size of exports even more..
(which means people pay even more export tax than they do today)

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Adrian Bunk]

On Mon, May 01, 2006 at 12:38:33AM -0700, Andrew Morton wrote:
> Adrian Bunk <bunk@stusta.de> wrote:
>...
> > > >  - remove the following unused EXPORT_SYMBOL:
> > > >    - in_egroup_p
> > > >  - remove the following unused EXPORT_SYMBOL_GPL's:
> > > >    - kernel_restart
> > > >    - kernel_halt
> > > 
> > > which I will not.
> > > 
> > > We have a process for the latter.  And even if we ignore that process, the
> > > patch ends up sitting in -mm for ages because of the API change, along with
> > > the cleanups, which could be merged up promptly.
> > 
> > The problem is that we have a lack of a process at the other end:
> > 
> > There is no process to review added exports.
> 
> Yes there is - I and many others frequently query them.  Sure, sometimes
> stuff slips through.  But it's a very very minor problem.

Linus merges dozens of git trees, and we have exactly zero process for 
noticing issues like [1]. Sure, you can say "Adrian will complain", but 
others can complain equally when I unexport a symbol where I either 
missed the in-kernel users or an in-kernel user is just about to be 
submitted.

And where is a non-minor problem with unexports?

If it accidentially breaks in-kernel stuff people notice immediately, 
and if it breaks external modules there is still the point that we do 
not have a stable API for external modules.

And breaking external modules frequently is a _good_ thing since it 
gives people them a reason for submitting their code for inclusion into 
the kernel. LSM/AppArmor is an example for the benefits of threating 
with immediate removal (no matter whether the result will be merging 
AppArmor or a more secure implementation of AppArmor, or whatever else).

cu
Adrian

[1] http://lkml.org/lkml/2006/3/18/127

-- 

       "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

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Andrew Morton]

Arjan van de Ven <arjan@infradead.org> wrote:
>
> On Mon, 2006-05-01 at 00:49 -0700, Andrew Morton wrote:
> > Arjan van de Ven <arjan@infradead.org> wrote:
> > >
> > > > If removing exports requires a process, adding exports requires a 
> > >  > similar process.
> > > 
> > >  alternatively we should bite the bullet, and just stick those 900 on the
> > >  "we'll kill all these in 3 months" list, have a thing to disable them
> > >  now via a config option (so that people actually notice rather than just
> > >  having them in the depreciation file) and fix the 5 or 10 or so that
> > >  actually will be used soon in those 3 months.
> > > 
> > 
> > I'd instead suggest that we implement a new EXPORT_SYMBOL_UNEXPORT_SCHEDULED
> 
> EXPORT_UNUSED_SYMBOL() ? (with comment of date)

OK.

> (well you didn't apply the patch for that I sent you so I suppose you
> hate it ;)

err, memory fails me.  Wanna dig it out?

> 
> > (?) and use that.  Suitable compile-time and modprobe-time warnings would
> 
> compiletime warning is hard, because it would require changing
> prototype, which is a lot more churn, and I'll bet a lot of exports
> don't even have a prototype at all. modprobe time is easy. (middle
> ground would be depmod time; that's almost compile time I suppose but a
> lot easier, but might need modutils help)

modprobe-time printk is good.

> > be needed.  Put the unexport date in a comment at the
> > EXPORT_SYMBOL_UNEXPORT_SCHEDULED site or even in the modprobe-time warning
> > message, if that's convenient:
> > 
> > EXPORT_SYMBOL_UNEXPORT_SCHEDULED(foo, "Dec 2006");
> 
> comment is easy, putting a date in is really sucky since it grows the size of exports even more..
> (which means people pay even more export tax than they do today)

ok, whatever.

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Adrian Bunk]

On Mon, May 01, 2006 at 12:49:25AM -0700, Andrew Morton wrote:
> Arjan van de Ven <arjan@infradead.org> wrote:
> >
> > > If removing exports requires a process, adding exports requires a 
> >  > similar process.
> > 
> >  alternatively we should bite the bullet, and just stick those 900 on the
> >  "we'll kill all these in 3 months" list, have a thing to disable them
> >  now via a config option (so that people actually notice rather than just
> >  having them in the depreciation file) and fix the 5 or 10 or so that
> >  actually will be used soon in those 3 months.
> > 
> 
> I'd instead suggest that we implement a new EXPORT_SYMBOL_UNEXPORT_SCHEDULED
> (?) and use that.  Suitable compile-time and modprobe-time warnings would
> be needed.  Put the unexport date in a comment at the
> EXPORT_SYMBOL_UNEXPORT_SCHEDULED site or even in the modprobe-time warning
> message, if that's convenient:
> 
> EXPORT_SYMBOL_UNEXPORT_SCHEDULED(foo, "Dec 2006");

__deprecated_for_modules already does everything required for compile 
time warnings.

No need to make the kernel image even bigger for unused stuff.

Looking over the patches I sent, this might affect perhaps 200 unused 
exports today.

So the work would expand to:
- writing 200 feature-removal-schedule.txt entries
- marking 200 functions and variables as __deprecated_for_modules

And in a few months:
- removing 200 feature-removal-schedule.txt entries
- removing 200 __deprecated_for_modules markers
- removing the 200 unused exports

But what do we gain by doing this extra work?
As I said in my other email, there is no good reason to not break 
external modules.

And this is reaching the point where I'm spending too much of the part 
of my spare time I'm devoting to Linux kernel development in following 
useless processes instead of doing something having a result making it 
worth spending my time for.

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

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Arjan van de Ven]

On Mon, 2006-05-01 at 10:59 +0200, Adrian Bunk wrote:
> On Mon, May 01, 2006 at 12:49:25AM -0700, Andrew Morton wrote:
> > Arjan van de Ven <arjan@infradead.org> wrote:
> > >
> > > > If removing exports requires a process, adding exports requires a 
> > >  > similar process.
> > > 
> > >  alternatively we should bite the bullet, and just stick those 900 on the
> > >  "we'll kill all these in 3 months" list, have a thing to disable them
> > >  now via a config option (so that people actually notice rather than just
> > >  having them in the depreciation file) and fix the 5 or 10 or so that
> > >  actually will be used soon in those 3 months.
> > > 
> > 
> > I'd instead suggest that we implement a new EXPORT_SYMBOL_UNEXPORT_SCHEDULED
> > (?) and use that.  Suitable compile-time and modprobe-time warnings would
> > be needed.  Put the unexport date in a comment at the
> > EXPORT_SYMBOL_UNEXPORT_SCHEDULED site or even in the modprobe-time warning
> > message, if that's convenient:
> > 
> > EXPORT_SYMBOL_UNEXPORT_SCHEDULED(foo, "Dec 2006");
> 
> __deprecated_for_modules already does everything required for compile 
> time warnings.

it's wrong level though; it works on the prototype not on the export
itself

> So the work would expand to:
> - writing 200 feature-removal-schedule.txt entries
> - marking 200 functions and variables as __deprecated_for_modules

not really; all one needs is 1 entry in f-r-s covering all 900 exports
and 2 actions: 1) mark those 900 as unused now, and 2) remove those 900
lines a few months from now

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Andrew Morton]

Adrian Bunk <bunk@stusta.de> wrote:
>
> So the work would expand to:
>  - writing 200 feature-removal-schedule.txt entries
>  - marking 200 functions and variables as __deprecated_for_modules
>
>  And in a few months:
>  - removing 200 feature-removal-schedule.txt entries
>  - removing 200 __deprecated_for_modules markers
>  - removing the 200 unused exports

Don't bother with all that stuff - a modprobe-time warning across a few
kernel releases is sufficient to make any developers who are dependent upon
an export aware of their problem.

Changing the export to EXPORT_UNUSED_SYMBOL() and then later removing the
export is adequate.

Re: [RFC: 2.6 patch] kernel/sys.c: possible cleanups

[Adrian Bunk]

On Mon, May 01, 2006 at 02:07:22AM -0700, Andrew Morton wrote:
> Adrian Bunk <bunk@stusta.de> wrote:
> >
> > So the work would expand to:
> >  - writing 200 feature-removal-schedule.txt entries
> >  - marking 200 functions and variables as __deprecated_for_modules
> >
> >  And in a few months:
> >  - removing 200 feature-removal-schedule.txt entries
> >  - removing 200 __deprecated_for_modules markers
> >  - removing the 200 unused exports
> 
> Don't bother with all that stuff - a modprobe-time warning across a few
> kernel releases is sufficient to make any developers who are dependent upon
> an export aware of their problem.
> 
> Changing the export to EXPORT_UNUSED_SYMBOL() and then later removing the
> export is adequate.

OK, let's get this into 2.6.17 - it can't break anything and makes 
developers sooner aware of this.

Can we also create the rule that changing an EXPORT_UNUSED_SYMBOL() back 
to an EXPORT_SYMBOL() requires an in-kernel user of the export? 
Otherwise all this "there is no stable API for external modules" saying 
starts to become nonsense.

If we give developers 6 months of EXPORT_UNUSED_SYMBOL() warning this 
should be enough for them to submit their code for review and inclusion 
into the kernel.

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