On Wednesday 17 January 2007 03:33, Eric W. Biederman wrote:
> There has not been much maintenance on sysctl in years, and as a result is
> there is a lot to do to allow future interesting work to happen, and being
> ambitious I'm trying to do it all at once :)
>
> The patches in this series fall into several general categories.

[...]

The patches look good to me.

-Andi
_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

minor extra space in table below...

Kirill

> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  fs/xfs/linux-2.6/xfs_sysctl.c |  258 ++++++++++++++++++++++++++++------------
>  1 files changed, 180 insertions(+), 78 deletions(-)
> 
> diff --git a/fs/xfs/linux-2.6/xfs_sysctl.c b/fs/xfs/linux-2.6/xfs_sysctl.c
> index af777e9..5a0eefc 100644
> --- a/fs/xfs/linux-2.6/xfs_sysctl.c
> +++ b/fs/xfs/linux-2.6/xfs_sysctl.c
> @@ -55,95 +55,197 @@ xfs_stats_clear_proc_handler(
>  #endif /* CONFIG_PROC_FS */
>  
>  STATIC ctl_table xfs_table[] = {
> -	{XFS_RESTRICT_CHOWN, "restrict_chown", &xfs_params.restrict_chown.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.restrict_chown.min, &xfs_params.restrict_chown.max},
> -
> -	{XFS_SGID_INHERIT, "irix_sgid_inherit", &xfs_params.sgid_inherit.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.sgid_inherit.min, &xfs_params.sgid_inherit.max},
> -
> -	{XFS_SYMLINK_MODE, "irix_symlink_mode", &xfs_params.symlink_mode.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.symlink_mode.min, &xfs_params.symlink_mode.max},
> -
> -	{XFS_PANIC_MASK, "panic_mask", &xfs_params.panic_mask.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.panic_mask.min, &xfs_params.panic_mask.max},
> -
> -	{XFS_ERRLEVEL, "error_level", &xfs_params.error_level.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.error_level.min, &xfs_params.error_level.max},
> -
> -	{XFS_SYNCD_TIMER, "xfssyncd_centisecs", &xfs_params.syncd_timer.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.syncd_timer.min, &xfs_params.syncd_timer.max},
> -
> -	{XFS_INHERIT_SYNC, "inherit_sync", &xfs_params.inherit_sync.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.inherit_sync.min, &xfs_params.inherit_sync.max},
> -
> -	{XFS_INHERIT_NODUMP, "inherit_nodump", &xfs_params.inherit_nodump.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.inherit_nodump.min, &xfs_params.inherit_nodump.max},
> -
> -	{XFS_INHERIT_NOATIME, "inherit_noatime", &xfs_params.inherit_noatim.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.inherit_noatim.min, &xfs_params.inherit_noatim.max},
> -
> -	{XFS_BUF_TIMER, "xfsbufd_centisecs", &xfs_params.xfs_buf_timer.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.xfs_buf_timer.min, &xfs_params.xfs_buf_timer.max},
> -
> -	{XFS_BUF_AGE, "age_buffer_centisecs", &xfs_params.xfs_buf_age.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.xfs_buf_age.min, &xfs_params.xfs_buf_age.max},
> -
> -	{XFS_INHERIT_NOSYM, "inherit_nosymlinks", &xfs_params.inherit_nosym.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.inherit_nosym.min, &xfs_params.inherit_nosym.max},
> -
> -	{XFS_ROTORSTEP, "rotorstep", &xfs_params.rotorstep.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.rotorstep.min, &xfs_params.rotorstep.max},
> -
> -	{XFS_INHERIT_NODFRG, "inherit_nodefrag", &xfs_params.inherit_nodfrg.val,
> -	sizeof(int), 0644, NULL, &proc_dointvec_minmax,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.inherit_nodfrg.min, &xfs_params.inherit_nodfrg.max},
> +	{
> +		.ctl_name	= XFS_RESTRICT_CHOWN,
> +		.procname	= "restrict_chown",
> +		.data		= &xfs_params.restrict_chown.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.restrict_chown.min,
> +		.extra2		= &xfs_params.restrict_chown.max
> +	},
> +	{
> +		.ctl_name	= XFS_SGID_INHERIT,
> +		.procname	= "irix_sgid_inherit",
> +		.data		= &xfs_params.sgid_inherit.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.sgid_inherit.min,
> +		.extra2		= &xfs_params.sgid_inherit.max
> +	},
> +	{
> +		.ctl_name	= XFS_SYMLINK_MODE,
> +		.procname	= "irix_symlink_mode",
> +		.data		= &xfs_params.symlink_mode.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.symlink_mode.min,
> +		.extra2		= &xfs_params.symlink_mode.max
> +	},
> +	{
> +		.ctl_name	= XFS_PANIC_MASK,
> +		.procname	= "panic_mask",
> +		.data		= &xfs_params.panic_mask.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	=  &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.panic_mask.min,
> +		.extra2		= &xfs_params.panic_mask.max
> +	},
>  
> +	{
> +		.ctl_name	= XFS_ERRLEVEL,
> +		.procname	= "error_level",
> +		.data		= &xfs_params.error_level.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.error_level.min,
> +		.extra2		= &xfs_params.error_level.max
> +	},
> +	{
> +		.ctl_name	= XFS_SYNCD_TIMER,
> +		.procname	= "xfssyncd_centisecs",
> +		.data		= &xfs_params.syncd_timer.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.syncd_timer.min,
> +		.extra2		= &xfs_params.syncd_timer.max
> +	},
> +	{
> +		.ctl_name	= XFS_INHERIT_SYNC,
> +		.procname	= "inherit_sync",
> +		.data		= &xfs_params.inherit_sync.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.inherit_sync.min,
> +		.extra2		= &xfs_params.inherit_sync.max
> +	},
> +	{
> +		.ctl_name	= XFS_INHERIT_NODUMP,
> +		.procname	= "inherit_nodump",
> +		.data		= &xfs_params.inherit_nodump.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec, NULL,
> +		.extra1		= &xfs_params.inherit_nodump.min,
> +		.extra2		= &xfs_params.inherit_nodump.max
> +	},
> +	{
> +		.ctl_name	= XFS_INHERIT_NOATIME,
> +		.procname	= "inherit_noatime",
> +		.data		= &xfs_params.inherit_noatim.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec, NULL,
> +		.extra1		= &xfs_params.inherit_noatim.min,
> +		.extra2		= &xfs_params.inherit_noatim.max
> +	},
> +	{
> +		.ctl_name	= XFS_BUF_TIMER,
> +		.procname	= "xfsbufd_centisecs",
> +		.data		= &xfs_params.xfs_buf_timer.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.xfs_buf_timer.min,
> +		.extra2		= &xfs_params.xfs_buf_timer.max
> +	},
> +	{
> +		.ctl_name	= XFS_BUF_AGE,
> +		.procname	= "age_buffer_centisecs",
> +		.data		= &xfs_params.xfs_buf_age.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec, NULL,
> +		.extra1		= &xfs_params.xfs_buf_age.min,
> +		.extra2		= &xfs_params.xfs_buf_age.max
> +	},
> +	{
> +		.ctl_name	= XFS_INHERIT_NOSYM,
> +		.procname	= "inherit_nosymlinks",
> +		.data		= &xfs_params.inherit_nosym.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.inherit_nosym.min,
> +		.extra2		= &xfs_params.inherit_nosym.max
> +	},
> +	{
> +		.ctl_name	= XFS_ROTORSTEP,
> +		.procname	= "rotorstep",
> +		.data		= &xfs_params.rotorstep.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.rotorstep.min,
> +		.extra2		= &xfs_params.rotorstep.max
> +	},
> +	{
> +		.ctl_name	= XFS_INHERIT_NODFRG,
> +		.procname	= "inherit_nodefrag",
> +		.data		= &xfs_params.inherit_nodfrg.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &proc_dointvec_minmax,
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.inherit_nodfrg.min,
> +		.extra2		= &xfs_params.inherit_nodfrg.max
> +	},
>  	/* please keep this the last entry */
>  #ifdef CONFIG_PROC_FS
> -	{XFS_STATS_CLEAR, "stats_clear", &xfs_params.stats_clear.val,
> -	sizeof(int), 0644, NULL, &xfs_stats_clear_proc_handler,
> -	&sysctl_intvec, NULL,
> -	&xfs_params.stats_clear.min, &xfs_params.stats_clear.max},
> +	{
> +		.ctl_name	= XFS_STATS_CLEAR,
> +		.procname	= "stats_clear",
> +		.data		= &xfs_params.stats_clear.val,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	=  &xfs_stats_clear_proc_handler,
<<< minor. extra space.
> +		.strategy	= &sysctl_intvec,
> +		.extra1		= &xfs_params.stats_clear.min,
> +		.extra2		= &xfs_params.stats_clear.max
> +	},
>  #endif /* CONFIG_PROC_FS */
>  
> -	{0}
> +	{}
>  };
>  
>  STATIC ctl_table xfs_dir_table[] = {
> -	{FS_XFS, "xfs", NULL, 0, 0555, xfs_table},
> -	{0}
> +	{
> +		.ctl_name	= FS_XFS,
> +		.procname	= "xfs",
> +		.mode		= 0555,
> +		.child		= xfs_table
> +	},
> +	{}
>  };
>  
>  STATIC ctl_table xfs_root_table[] = {
> -	{CTL_FS, "fs",  NULL, 0, 0555, xfs_dir_table},
> -	{0}
> +	{
> +		.ctl_name	= CTL_FS,
> +		.procname	= "fs",
> +		.mode		= 0555,
> +		.child		= xfs_dir_table
> +	},
> +	{}
>  };
>  
>  void

_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

another small minor note.

> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  arch/frv/kernel/pm.c |   50 +++++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/frv/kernel/pm.c b/arch/frv/kernel/pm.c
> index c1840d6..aa50333 100644
> --- a/arch/frv/kernel/pm.c
> +++ b/arch/frv/kernel/pm.c
> @@ -401,17 +401,53 @@ static int cm_sysctl(ctl_table *table, int __user *name, int nlen,
>  
>  static struct ctl_table pm_table[] =
>  {
> -	{CTL_PM_SUSPEND, "suspend", NULL, 0, 0200, NULL, &sysctl_pm_do_suspend},
> -	{CTL_PM_CMODE, "cmode", &clock_cmode_current, sizeof(int), 0644, NULL, &cmode_procctl, &cmode_sysctl, NULL},
> -	{CTL_PM_P0, "p0", &clock_p0_current, sizeof(int), 0644, NULL, &p0_procctl, &p0_sysctl, NULL},
> -	{CTL_PM_CM, "cm", &clock_cm_current, sizeof(int), 0644, NULL, &cm_procctl, &cm_sysctl, NULL},
> -	{0}
> +	{
> +		.ctl_name	= CTL_PM_SUSPEND,
> +		.procname	= "suspend",
> +		.data		= NULL,
> +		.maxlen		= 0,
> +		.mode		= 0200,
> +		.proc_handler	= &sysctl_pm_do_suspend,
> +	},
> +	{
> +		.ctl_name	= CTL_PM_CMODE,
> +		.procname	= "cmode",
> +		.data		= &clock_cmode_current,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &cmode_procctl,
> +		.strategy	= &cmode_sysctl,
> +	},
> +	{
> +		.ctl_name	= CTL_PM_P0,
> +		.procname	= "p0",
> +		.data		= &clock_p0_current,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &p0_procctl,
> +		.strategy	= &p0_sysctl,
> +	},
> +	{
> +		.ctl_name	= CTL_PM_CM,
> +		.procname	= "cm",
> +		.data		= &clock_cm_current,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= &cm_procctl,
> +		.strategy	= &cm_sysctl,
> +	},
> +	{ .ctl_name = 0}
in next patch (26/59) you write just "{ }". .ctl_name = 0 not required here.


>  };
>  
>  static struct ctl_table pm_dir_table[] =
>  {
> -	{CTL_PM, "pm", NULL, 0, 0555, pm_table},
> -	{0}
> +	{
> +		.ctl_name	= CTL_PM,
> +		.procname	= "pm",
> +		.mode		= 0555,
> +		.child		= pm_table,
> +	},
> +	{ .ctl_name = 0}
>  };
>  
>  /*

_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

IDs not sorted in enum. see below.

> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> 
> We need to have the the definition of all top level sysctl
> directories registers in sysctl.h so we don't conflict by
> accident and cause abi problems.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  arch/s390/appldata/appldata.h |    3 +--
>  arch/s390/kernel/debug.c      |    1 -
>  arch/s390/mm/cmm.c            |    4 ----
>  include/linux/sysctl.h        |    7 +++++++
>  4 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/s390/appldata/appldata.h b/arch/s390/appldata/appldata.h
> index 0429481..4069b81 100644
> --- a/arch/s390/appldata/appldata.h
> +++ b/arch/s390/appldata/appldata.h
> @@ -21,8 +21,7 @@
>  #define APPLDATA_RECORD_NET_SUM_ID	0x03	/* must be < 256 !     */
>  #define APPLDATA_RECORD_PROC_ID		0x04
>  
> -#define CTL_APPLDATA 		2120	/* sysctl IDs, must be unique */
> -#define CTL_APPLDATA_TIMER 	2121
> +#define CTL_APPLDATA_TIMER 	2121	/* sysctl IDs, must be unique */
>  #define CTL_APPLDATA_INTERVAL 	2122
>  #define CTL_APPLDATA_MEM	2123
>  #define CTL_APPLDATA_OS		2124
> diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
> index bb57bc0..c81f8e5 100644
> --- a/arch/s390/kernel/debug.c
> +++ b/arch/s390/kernel/debug.c
> @@ -852,7 +852,6 @@ debug_finish_entry(debug_info_t * id, debug_entry_t* active, int level,
>  static int debug_stoppable=1;
>  static int debug_active=1;
>  
> -#define CTL_S390DBF 5677
>  #define CTL_S390DBF_STOPPABLE 5678
>  #define CTL_S390DBF_ACTIVE 5679
>  
> diff --git a/arch/s390/mm/cmm.c b/arch/s390/mm/cmm.c
> index 607f50e..df733d5 100644
> --- a/arch/s390/mm/cmm.c
> +++ b/arch/s390/mm/cmm.c
> @@ -256,10 +256,6 @@ cmm_skip_blanks(char *cp, char **endp)
>  }
>  
>  #ifdef CONFIG_CMM_PROC
> -/* These will someday get removed. */
> -#define VM_CMM_PAGES		1111
> -#define VM_CMM_TIMED_PAGES	1112
> -#define VM_CMM_TIMEOUT		1113
>  
>  static struct ctl_table cmm_table[];
>  
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 71c16b4..56d0161 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -73,6 +73,8 @@ enum
>  	CTL_SUNRPC=7249,	/* sunrpc debug */
>  	CTL_PM=9899,		/* frv power management */
>  	CTL_FRV=9898,		/* frv specific sysctls */
> +	CTL_S390DBF=5677,	/* s390 debug */
> +	CTL_APPLDATA=2120,	/* s390 appldata */
<<<< not sorted by ID? imho should be sorted. otherwise can'be unnotied when inserted above.

>  };
>  
>  /* CTL_BUS names: */
> @@ -205,6 +207,11 @@ enum
>  	VM_PANIC_ON_OOM=33,	/* panic at out-of-memory */
>  	VM_VDSO_ENABLED=34,	/* map VDSO into new processes? */
>  	VM_MIN_SLAB=35,		 /* Percent pages ignored by zone reclaim */
> +
> +	/* s390 vm cmm sysctls */
> +	VM_CMM_PAGES=1111,
> +	VM_CMM_TIMED_PAGES=1112,
> +	VM_CMM_TIMEOUT=1113,
>  };
>  
>  

_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

Eric, though I personally don't care much:
1. I ask for not setting your authorship/copyright on the code which you just copied
  from other places. Just doesn't look polite IMHO.
2. I would propose to not introduce utsname_sysctl.c.
  both files are too small and minor that I can't see much reasons splitting them.

Kirill

> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> 
> This is just a simple cleanup to keep kernel/sysctl.c
> from getting to crowded with special cases, and by
> keeping all of the utsname logic to together it makes
> the code a little more readable.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  kernel/Makefile         |    1 +
>  kernel/sysctl.c         |  115 -------------------------------------
>  kernel/utsname_sysctl.c |  146 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 147 insertions(+), 115 deletions(-)
> 
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 14f4d45..d286c44 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_SECCOMP) += seccomp.o
>  obj-$(CONFIG_RCU_TORTURE_TEST) += rcutorture.o
>  obj-$(CONFIG_RELAY) += relay.o
>  obj-$(CONFIG_UTS_NS) += utsname.o
> +obj-$(CONFIG_SYSCTL) += utsname_sysctl.o
>  obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
>  obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
>  
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 7420761..a8c0a03 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -135,13 +135,6 @@ static int parse_table(int __user *, int, void __user *, size_t __user *,
>  		void __user *, size_t, ctl_table *);
>  #endif
>  
> -static int proc_do_uts_string(ctl_table *table, int write, struct file *filp,
> -		  void __user *buffer, size_t *lenp, loff_t *ppos);
> -
> -static int sysctl_uts_string(ctl_table *table, int __user *name, int nlen,
> -		  void __user *oldval, size_t __user *oldlenp,
> -		  void __user *newval, size_t newlen);
> -
>  #ifdef CONFIG_SYSVIPC
>  static int sysctl_ipc_data(ctl_table *table, int __user *name, int nlen,
>  		  void __user *oldval, size_t __user *oldlenp,
> @@ -174,27 +167,6 @@ extern ctl_table inotify_table[];
>  int sysctl_legacy_va_layout;
>  #endif
>  
> -static void *get_uts(ctl_table *table, int write)
> -{
> -	char *which = table->data;
> -#ifdef CONFIG_UTS_NS
> -	struct uts_namespace *uts_ns = current->nsproxy->uts_ns;
> -	which = (which - (char *)&init_uts_ns) + (char *)uts_ns;
> -#endif
> -	if (!write)
> -		down_read(&uts_sem);
> -	else
> -		down_write(&uts_sem);
> -	return which;
> -}
> -
> -static void put_uts(ctl_table *table, int write, void *which)
> -{
> -	if (!write)
> -		up_read(&uts_sem);
> -	else
> -		up_write(&uts_sem);
> -}
>  
>  #ifdef CONFIG_SYSVIPC
>  static void *get_ipc(ctl_table *table, int write)
> @@ -275,51 +247,6 @@ static ctl_table root_table[] = {
>  
>  static ctl_table kern_table[] = {
>  	{
> -		.ctl_name	= KERN_OSTYPE,
> -		.procname	= "ostype",
> -		.data		= init_uts_ns.name.sysname,
> -		.maxlen		= sizeof(init_uts_ns.name.sysname),
> -		.mode		= 0444,
> -		.proc_handler	= &proc_do_uts_string,
> -		.strategy	= &sysctl_uts_string,
> -	},
> -	{
> -		.ctl_name	= KERN_OSRELEASE,
> -		.procname	= "osrelease",
> -		.data		= init_uts_ns.name.release,
> -		.maxlen		= sizeof(init_uts_ns.name.release),
> -		.mode		= 0444,
> -		.proc_handler	= &proc_do_uts_string,
> -		.strategy	= &sysctl_uts_string,
> -	},
> -	{
> -		.ctl_name	= KERN_VERSION,
> -		.procname	= "version",
> -		.data		= init_uts_ns.name.version,
> -		.maxlen		= sizeof(init_uts_ns.name.version),
> -		.mode		= 0444,
> -		.proc_handler	= &proc_do_uts_string,
> -		.strategy	= &sysctl_uts_string,
> -	},
> -	{
> -		.ctl_name	= KERN_NODENAME,
> -		.procname	= "hostname",
> -		.data		= init_uts_ns.name.nodename,
> -		.maxlen		= sizeof(init_uts_ns.name.nodename),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_do_uts_string,
> -		.strategy	= &sysctl_uts_string,
> -	},
> -	{
> -		.ctl_name	= KERN_DOMAINNAME,
> -		.procname	= "domainname",
> -		.data		= init_uts_ns.name.domainname,
> -		.maxlen		= sizeof(init_uts_ns.name.domainname),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_do_uts_string,
> -		.strategy	= &sysctl_uts_string,
> -	},
> -	{
>  		.ctl_name	= KERN_PANIC,
>  		.procname	= "panic",
>  		.data		= &panic_timeout,
> @@ -1746,21 +1673,6 @@ int proc_dostring(ctl_table *table, int write, struct file *filp,
>  			       buffer, lenp, ppos);
>  }
>  
> -/*
> - *	Special case of dostring for the UTS structure. This has locks
> - *	to observe. Should this be in kernel/sys.c ????
> - */
> -
> -static int proc_do_uts_string(ctl_table *table, int write, struct file *filp,
> -		  void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -	int r;
> -	void *which;
> -	which = get_uts(table, write);
> -	r = _proc_do_string(which, table->maxlen,write,filp,buffer,lenp, ppos);
> -	put_uts(table, write, which);
> -	return r;
> -}
>  
>  static int do_proc_dointvec_conv(int *negp, unsigned long *lvalp,
>  				 int *valp,
> @@ -2379,12 +2291,6 @@ int proc_dostring(ctl_table *table, int write, struct file *filp,
>  	return -ENOSYS;
>  }
>  
> -static int proc_do_uts_string(ctl_table *table, int write, struct file *filp,
> -		void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -	return -ENOSYS;
> -}
> -
>  #ifdef CONFIG_SYSVIPC
>  static int proc_do_ipc_string(ctl_table *table, int write, struct file *filp,
>  		void __user *buffer, size_t *lenp, loff_t *ppos)
> @@ -2602,21 +2508,6 @@ int sysctl_ms_jiffies(ctl_table *table, int __user *name, int nlen,
>  }
>  
>  
> -/* The generic string strategy routine: */
> -static int sysctl_uts_string(ctl_table *table, int __user *name, int nlen,
> -		  void __user *oldval, size_t __user *oldlenp,
> -		  void __user *newval, size_t newlen)
> -{
> -	struct ctl_table uts_table;
> -	int r, write;
> -	write = newval && newlen;
> -	memcpy(&uts_table, table, sizeof(uts_table));
> -	uts_table.data = get_uts(table, write);
> -	r = sysctl_string(&uts_table, name, nlen,
> -		oldval, oldlenp, newval, newlen);
> -	put_uts(table, write, uts_table.data);
> -	return r;
> -}
>  
>  #ifdef CONFIG_SYSVIPC
>  /* The generic sysctl ipc data routine. */
> @@ -2723,12 +2614,6 @@ int sysctl_ms_jiffies(ctl_table *table, int __user *name, int nlen,
>  	return -ENOSYS;
>  }
>  
> -static int sysctl_uts_string(ctl_table *table, int __user *name, int nlen,
> -		  void __user *oldval, size_t __user *oldlenp,
> -		  void __user *newval, size_t newlen)
> -{
> -	return -ENOSYS;
> -}
>  static int sysctl_ipc_data(ctl_table *table, int __user *name, int nlen,
>  		void __user *oldval, size_t __user *oldlenp,
>  		void __user *newval, size_t newlen)
> diff --git a/kernel/utsname_sysctl.c b/kernel/utsname_sysctl.c
> new file mode 100644
> index 0000000..324aa13
> --- /dev/null
> +++ b/kernel/utsname_sysctl.c
> @@ -0,0 +1,146 @@
> +/*
> + *  Copyright (C) 2007
> + *
> + *  Author: Eric Biederman <ebiederm@xmision.com>
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License as
> + *  published by the Free Software Foundation, version 2 of the
> + *  License.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/uts.h>
> +#include <linux/utsname.h>
> +#include <linux/version.h>
> +#include <linux/sysctl.h>
> +
> +static void *get_uts(ctl_table *table, int write)
> +{
> +	char *which = table->data;
> +#ifdef CONFIG_UTS_NS
> +	struct uts_namespace *uts_ns = current->nsproxy->uts_ns;
> +	which = (which - (char *)&init_uts_ns) + (char *)uts_ns;
> +#endif
> +	if (!write)
> +		down_read(&uts_sem);
> +	else
> +		down_write(&uts_sem);
> +	return which;
> +}
> +
> +static void put_uts(ctl_table *table, int write, void *which)
> +{
> +	if (!write)
> +		up_read(&uts_sem);
> +	else
> +		up_write(&uts_sem);
> +}
> +
> +#ifdef CONFIG_PROC_FS
> +/*
> + *	Special case of dostring for the UTS structure. This has locks
> + *	to observe. Should this be in kernel/sys.c ????
> + */
> +static int proc_do_uts_string(ctl_table *table, int write, struct file *filp,
> +		  void __user *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	struct ctl_table uts_table;
> +	int r;
> +	memcpy(&uts_table, table, sizeof(uts_table));
> +	uts_table.data = get_uts(table, write);
> +	r = proc_dostring(&uts_table,write,filp,buffer,lenp, ppos);
> +	put_uts(table, write, uts_table.data);
> +	return r;
> +}
> +#else
> +#define proc_do_uts_string NULL
> +#endif
> +
> +
> +#ifdef CONFIG_SYSCTL_SYSCALL
> +/* The generic string strategy routine: */
> +static int sysctl_uts_string(ctl_table *table, int __user *name, int nlen,
> +		  void __user *oldval, size_t __user *oldlenp,
> +		  void __user *newval, size_t newlen)
> +{
> +	struct ctl_table uts_table;
> +	int r, write;
> +	write = newval && newlen;
> +	memcpy(&uts_table, table, sizeof(uts_table));
> +	uts_table.data = get_uts(table, write);
> +	r = sysctl_string(&uts_table, 

1. I ask for not setting your authorship/copyright on the code which you just copied
   from other places. Just doesn't look polite IMHO.
2. please don't name files like ipc/ipc_sysctl.c
   ipc/sysctl.c sounds better IMHO.
3. any reason to introduce CONFIG_SYSVIPC_SYSCTL?
   why not simply do
   > +obj-$(CONFIG_SYSCTL) += sysctl.o
   instead?

Kirill

> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> 
> This is just a simple cleanup to keep kernel/sysctl.c
> from getting to crowded with special cases, and by
> keeping all of the ipc logic to together it makes
> the code a little more readable.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  init/Kconfig     |    6 ++
>  ipc/Makefile     |    1 +
>  ipc/ipc_sysctl.c |  182 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kernel/sysctl.c  |  174 ---------------------------------------------------
>  4 files changed, 189 insertions(+), 174 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index a3f83e2..33bc38d 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -116,6 +116,12 @@ config SYSVIPC
>  	  section 6.4 of the Linux Programmer's Guide, available from
>  	  <http://www.tldp.org/guides.html>.
>  
> +config SYSVIPC_SYSCTL
> +	bool
> +	depends on SYSVIPC
> +	depends on SYSCTL
> +	default y
> +
>  config IPC_NS
>  	bool "IPC Namespaces"
>  	depends on SYSVIPC
> diff --git a/ipc/Makefile b/ipc/Makefile
> index 0a6d626..b93bba6 100644
> --- a/ipc/Makefile
> +++ b/ipc/Makefile
> @@ -4,6 +4,7 @@
>  
>  obj-$(CONFIG_SYSVIPC_COMPAT) += compat.o
>  obj-$(CONFIG_SYSVIPC) += util.o msgutil.o msg.o sem.o shm.o
> +obj-$(CONFIG_SYSVIPC_SYSCTL) += ipc_sysctl.o
>  obj_mq-$(CONFIG_COMPAT) += compat_mq.o
>  obj-$(CONFIG_POSIX_MQUEUE) += mqueue.o msgutil.o $(obj_mq-y)
>  
> diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
> new file mode 100644
> index 0000000..9018009
> --- /dev/null
> +++ b/ipc/ipc_sysctl.c
> @@ -0,0 +1,182 @@
> +/*
> + *  Copyright (C) 2007
> + *
> + *  Author: Eric Biederman <ebiederm@xmision.com>
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License as
> + *  published by the Free Software Foundation, version 2 of the
> + *  License.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/ipc.h>
> +#include <linux/nsproxy.h>
> +#include <linux/sysctl.h>
> +
> +#ifdef CONFIG_IPC_NS
> +static void *get_ipc(ctl_table *table)
> +{
> +	char *which = table->data;
> +	struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns;
> +	which = (which - (char *)&init_ipc_ns) + (char *)ipc_ns;
> +	return which;
> +}
> +#else
> +#define get_ipc(T) ((T)->data)
> +#endif
> +
> +#ifdef CONFIG_PROC_FS
> +static int proc_ipc_dointvec(ctl_table *table, int write, struct file *filp,
> +	void __user *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	struct ctl_table ipc_table;
> +	memcpy(&ipc_table, table, sizeof(ipc_table));
> +	ipc_table.data = get_ipc(table);
> +
> +	return proc_dointvec(&ipc_table, write, filp, buffer, lenp, ppos);
> +}
> +
> +static int proc_ipc_doulongvec_minmax(ctl_table *table, int write,
> +	struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	struct ctl_table ipc_table;
> +	memcpy(&ipc_table, table, sizeof(ipc_table));
> +	ipc_table.data = get_ipc(table);
> +
> +	return proc_doulongvec_minmax(&ipc_table, write, filp, buffer,
> +					lenp, ppos);
> +}
> +
> +#else
> +#define proc_ipc_do_ulongvec_minmax NULL
> +#define proc_ipc_do_intvec	    NULL
> +#endif
> +
> +#ifdef CONFIG_SYSCTL_SYSCALL
> +/* The generic sysctl ipc data routine. */
> +static int sysctl_ipc_data(ctl_table *table, int __user *name, int nlen,
> +		void __user *oldval, size_t __user *oldlenp,
> +		void __user *newval, size_t newlen)
> +{
> +	size_t len;
> +	void *data;
> +
> +	/* Get out of I don't have a variable */
> +	if (!table->data || !table->maxlen)
> +		return -ENOTDIR;
> +
> +	data = get_ipc(table);
> +	if (!data)
> +		return -ENOTDIR;
> +
> +	if (oldval && oldlenp) {
> +		if (get_user(len, oldlenp))
> +			return -EFAULT;
> +		if (len) {
> +			if (len > table->maxlen)
> +				len = table->maxlen;
> +			if (copy_to_user(oldval, data, len))
> +				return -EFAULT;
> +			if (put_user(len, oldlenp))
> +				return -EFAULT;
> +		}
> +	}
> +
> +	if (newval && newlen) {
> +		if (newlen > table->maxlen)
> +			newlen = table->maxlen;
> +
> +		if (copy_from_user(data, newval, newlen))
> +			return -EFAULT;
> +	}
> +	return 1;
> +}
> +#else
> +#define sysctl_ipc_data NULL
> +#endif
> +
> +static struct ctl_table ipc_kern_table[] = {
> +	{
> +		.ctl_name	= KERN_SHMMAX,
> +		.procname	= "shmmax",
> +		.data		= &init_ipc_ns.shm_ctlmax,
> +		.maxlen		= sizeof (init_ipc_ns.shm_ctlmax),
> +		.mode		= 0644,
> +		.proc_handler	= proc_ipc_doulongvec_minmax,
> +		.strategy	= sysctl_ipc_data,
> +	},
> +	{
> +		.ctl_name	= KERN_SHMALL,
> +		.procname	= "shmall",
> +		.data		= &init_ipc_ns.shm_ctlall,
> +		.maxlen		= sizeof (init_ipc_ns.shm_ctlall),
> +		.mode		= 0644,
> +		.proc_handler	= proc_ipc_doulongvec_minmax,
> +		.strategy	= sysctl_ipc_data,
> +	},
> +	{
> +		.ctl_name	= KERN_SHMMNI,
> +		.procname	= "shmmni",
> +		.data		= &init_ipc_ns.shm_ctlmni,
> +		.maxlen		= sizeof (init_ipc_ns.shm_ctlmni),
> +		.mode		= 0644,
> +		.proc_handler	= proc_ipc_dointvec,
> +		.strategy	= sysctl_ipc_data,
> +	},
> +	{
> +		.ctl_name	= KERN_MSGMAX,
> +		.procname	= "msgmax",
> +		.data		= &init_ipc_ns.msg_ctlmax,
> +		.maxlen		= sizeof (init_ipc_ns.msg_ctlmax),
> +		.mode		= 0644,
> +		.proc_handler	= proc_ipc_dointvec,
> +		.strategy	= sysctl_ipc_data,
> +	},
> +	{
> +		.ctl_name	= KERN_MSGMNI,
> +		.procname	= "msgmni",
> +		.data		= &init_ipc_ns.msg_ctlmni,
> +		.maxlen		= sizeof (init_ipc_ns.msg_ctlmni),
> +		.mode		= 0644,
> +		.proc_handler	= proc_ipc_dointvec,
> +		.strategy	= sysctl_ipc_data,
> +	},
> +	{
> +		.ctl_name	= KERN_MSGMNB,
> +		.procname	=  "msgmnb",
> +		.data		= &init_ipc_ns.msg_ctlmnb,
> +		.maxlen		= sizeof (init_ipc_ns.msg_ctlmnb),
> +		.mode		= 0644,
> +		.proc_handler	= proc_ipc_dointvec,
> +		.strategy	= sysctl_ipc_data,
> +	},
> +	{
> +		.ctl_name	= KERN_SEM,
> +		.procname	= "sem",
> +		.data		= &init_ipc_ns.sem_ctls,
> +		.maxlen		= 4*sizeof (int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_ipc_dointvec,
> +		.strategy	= sysctl_ipc_data,
> +	},
> +	{}
> +};
> +
> +static struct ctl_table ipc_root_table[] = {
> +	{
> +		.ctl_name	= CTL_KERN,
> +		.procname	= "kernel",
> +		.mode		= 0555,
> +		.child		= ipc_kern_table,
> +	},
> +	{}
> +};
> +
> +static int __init ipc_sysctl_init(void)
> +{
> +	register_sysctl_table(ipc_root_table, 0);
> +	return 0;
> +}
> +
> +__initcall(ipc_sysctl_init);
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index a8c0a03..6e2e608 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -90,12 +90,6 @@ extern char modprobe_path[];
>  #ifdef CONFIG_CHR_DEV_SG
>  extern int sg_big_buff;
>  #endif
> -#ifdef CONFIG_SYSVIPC
> -static int proc_ipc_dointvec(ctl_table *table, int write, struct file *filp,
> -		void __user *buffer, size_t *lenp, loff_t *ppos);
> -static int proc_ipc_doulongvec_minmax(ctl_table *table, int write, struct file *filp,
> -		void __user *buffer, size_t *lenp, loff_t *ppos);
> -#endif
>  
>  #ifdef __sparc__
>  extern char reboot_command [];
> @@ -135,11 +129,6 @@ static int parse_table(int __user *, int, void __user *, size_t __user *,
>  		void __user *, size_t, ctl_table *);
>  #endif
>  
> -#ifdef CONFIG_SYSVIPC
> -static int sysctl_ipc_data(ctl_table *table, int __user *name, int nlen,
> -		  void __user *oldval, size_t __user *oldlenp,
> -		  void __user *newval, size_t newlen);
> -#endif
>  
>  #ifdef CONFIG_PROC_SYSCTL
>  static int proc_do_cad_pid(ctl_table *table, int write, struct file *filp,
> @@ -168,17 +157,6 @@ int sysctl_legacy_va_layout;
>  #endif
>  
>  
> -#ifdef CONFIG_SYSVIPC
> -static void *get_ipc(ctl_table *table, int write)
> -{
> -	char *which = table->data;
> -	struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns;
> -	which = (which - (char *)&init_ipc_ns) + (char *)ipc_ns;
> -	return which;
> -}
> -#else
> -#define get_ipc(T,W) ((T)->data)
> -#endif
>  
>  /* /proc declarations: */
>  
> @@ -400,71 +378,6 @@ static ctl_table kern_table[] = {
>  		.proc_handler	= &proc_dointvec,
>  	},
>  #endif
> -#ifdef CONFIG_SYSVIPC
> -	{
> -		.ctl_name	= KERN_SHMMAX,
> -		.procname	= "shmmax",
> -		.data		= &init_ipc_ns.shm_ctlmax,
> -		.maxlen		= sizeof (init_ipc_ns.shm_ctlmax),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_ipc_doulongvec_minmax,
> -		.strategy	= sysctl_ipc_data,
> -	},
> -	{
> -		.ctl_name	= KERN_SHMALL,
> -		.procname	= "shmall",
> -		.data		= &init_ipc_ns.shm_ctlall,
> -		.maxlen		= sizeof (init_ipc_ns.shm_ctlall),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_ipc_doulongvec_minmax,
> -		.strategy	= sysctl_ipc_data,
> -	},
> -	{
> -		.ctl_name	= KERN_SHMMNI,
> -		.procname	= "shmmni",
> -		.data		= &init_ipc_ns.shm_ctlmni,
> -		.maxlen		= sizeof (init_ipc_ns.shm_ctlmni),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_ipc_dointvec,
> -		.strategy	= sysctl_ipc_data,
> -	},
> -	{
> -		.ctl_name	= KERN_MSGMAX,
> -		.procname	= "msgmax",
> -		.data		= &init_ipc_ns.msg_ctlmax,
> -		.maxlen		= sizeof (init_ipc_ns.msg_ctlmax),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_ipc_dointvec,
> -		.strategy	= sysctl_ipc_data,
> -	},
> -	{
> -		.ctl_name	= KERN_MSGMNI,
> -		.procname	= "msgmni",
> -		.data		= &init_ipc_ns.msg_ctlmni,
> -		.maxlen		= sizeof (init_ipc_ns.msg_ctlmni),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_ipc_dointvec,
> -		.strategy	= sysctl_ipc_data,
> -	},
> -	{
> -		.ctl_name	= KERN_MSGMNB,
> -		.procname	=  "msgmnb",
> -		.data		= &init_ipc_ns.msg_ctlmnb,
> -		.maxlen		= sizeof (init_ipc_ns.msg_ctlmnb),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_ipc_dointvec,
> -		.strategy	= sysctl_ipc_data,
> -	},
> -	{
> -		.ctl_name	= KERN_SEM,
> -		.procname	= "sem",
> -		.data		= &init_ipc_ns.sem_ctls,
> -		.maxlen		= 4*sizeof (int),
> -		.mode		= 0644,
> -		.proc_handler	= &proc_ipc_dointvec,
> -		.strategy	= sysctl_ipc_data,
> -	},
> -#endif
>  #ifdef CONFIG_MAGIC_SYSRQ
>  	{
>  		.ctl_name	= KERN_SYSRQ,
> @@ -2240,27 +2153,6 @@ int proc_dointvec_ms_jiffies(ctl_table *table, int write, struct file *filp,
>  				do_proc_dointvec_ms_jiffies_conv, NULL);
>  }
>  
> -#ifdef CONFIG_SYSVIPC
> -static int proc_ipc_dointvec(ctl_table *table, int write, struct file *filp,
> -	void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -	void *which;
> -	which = get_ipc(table, write);
> -	return __do_proc_dointvec(which, table, write, filp, buffer,
> -			lenp, ppos, NULL, NULL);
> -}
> -
> -static int proc_ipc_doulongvec_minmax(ctl_table *table, int write,
> -	struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -	void *which;
> -	which = get_ipc(table, write);
> -	return __do_proc_doulongvec_minmax(which, table, write, filp, buffer,
> -			lenp, ppos, 1l, 1l);
> -}
> -
> -#endif
> -
>  static int proc_do_cad_pid(ctl_table *table, int write, struct file *filp,
>  			   void __user *buffer, size_t *lenp, loff_t *ppos)
>  {
> @@ -2291,25 +2183,6 @@ int proc_dostring(ctl_table *table, int write, struct file *filp,
>  	return -ENOSYS;
>  }
>  
> -#ifdef CONFIG_SYSVIPC
> -static int proc_do_ipc_string(ctl_table *table, int write, struct file *filp,
> -		void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -	return -ENOSYS;
> -}
> -static int proc_ipc_dointvec(ctl_table *table, int write, struct file *filp,
> -		void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -	return -ENOSYS;
> -}
> -static int proc_ipc_doulongvec_minmax(ctl_table *table, int write,
> -		struct file *filp, void __user *buffer,
> -		size_t *lenp, loff_t *ppos)
> -{
> -	return -ENOSYS;
> -}
> -#endif
> -
>  int proc_dointvec(ctl_table *table, int write, struct file *filp,
>  		  void __user *buffer, size_t *lenp, loff_t *ppos)
>  {
> @@ -2509,47 +2382,6 @@ int sysctl_ms_jiffies(ctl_table *table, int __user *name, int nlen,
>  
>  
>  
> -#ifdef CONFIG_SYSVIPC
> -/* The generic sysctl ipc data routine. */
> -static int sysctl_ipc_data(ctl_table *table, int __user *name, int nlen,
> -		void __user *oldval, size_t __user *oldlenp,
> -		void __user *newval, size_t newlen)
> -{
> -	size_t len;
> -	void *data;
> -
> -	/* Get out of I don't have a variable */
> -	if (!table->data || !table->maxlen)
> -		return -ENOTDIR;
> -
> -	data = get_ipc(table, 1);
> -	if (!data)
> -		return -ENOTDIR;
> -
> -	if (oldval && oldlenp) {
> -		if (get_user(len, oldlenp))
> -			return -EFAULT;
> -		if (len) {
> -			if (len > table->maxlen)
> -				len = table->maxlen;
> -			if (copy_to_user(oldval, data, len))
> -				return -EFAULT;
> -			if (put_user(len, oldlenp))
> -				return -EFAULT;
> -		}
> -	}
> -
> -	if (newval && newlen) {
> -		if (newlen > table->maxlen)
> -			newlen = table->maxlen;
> -
> -		if (copy_from_user(data, newval, newlen))
> -			return -EFAULT;
> -	}
> -	return 1;
> -}
> -#endif
> -
>  #else /* CONFIG_SYSCTL_SYSCALL */
>  
>  
> @@ -2614,12 +2446,6 @@ int sysctl_ms_jiffies(ctl_table *table, int __user *name, int nlen,
>  	return -ENOSYS;
>  }
>  
> -static int sysctl_ipc_data(ctl_table *table, int __user *name, int nlen,
> -		void __user *oldval, size_t __user *oldlenp,
> -		void __user *newval, size_t newlen)
> -{
> -	return -ENOSYS;
> -}
>  #endif /* CONFIG_SYSCTL_SYSCALL */
>  
>  /*

_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

Eric, really good job!

Patches: 1-13, 15-24, 26-32, 34-44, 46-49, 52-55, 57 (all except below)
Acked-By: Kirill Korotaev <dev@openvz.org>

14/59 - minor (extra space)
25/59 - minor note	
33/59 - not sorted sysctl IDs
45/59 - typo
50/59 - copyright/file note
51/59 - copyright/file name/kconfig option notes

56,58,59/59 - will review tomorrow

another issue I have to think over is removal of de->owner.
Alexey Dobriyan has sent recently patching fixing /proc <-> modules refcounting.
I guess w/o these patches your changes are not safe if proc_handler or strategy
are functions from the module.

Thanks,
Kirill

> There has not been much maintenance on sysctl in years, and as a result is
> there is a lot to do to allow future interesting work to happen, and being
> ambitious I'm trying to do it all at once :)
> 
> The patches in this series fall into several general categories.
> 
> - Removal of useless attempts to override the standard sysctls
> 
> - Registers of sysctl numbers in sysctl.h so someone else does not use
>   the magic number and conflict.
> 
> - C99 conversions so it becomes possible to change the layout of 
>   struct ctl_table without breaking everything.
> 
> - Removal of useless claims of module ownership, in the proc dir entries
> 
> - Removal of sys_sysctl support where people had used conflicting sysctl
>   numbers. Trying to break glibc or other applications by changing the
>   ABI is not cool.  9 instances of this in the kernel seems a little
>   extreme.
> 
> - General enhancements when I got the junk I could see out.
> 
> Odds are I missed something, most of the cleanups are simply a result of
> me working on the sysctl core and glancing at the users and going: What?
> 
> Eric
> _______________________________________________
> Containers mailing list
> Containers@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/containers
> 
> 

_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

On Wed, Jan 17, 2007 at 08:14:17PM +0300, Kirill Korotaev wrote:
> another small minor note.
> 
> > From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> > 
> > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> > ---
> >  arch/frv/kernel/pm.c |   50 +++++++++++++++++++++++++++++++++++++++++++-------
> >  1 files changed, 43 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/frv/kernel/pm.c b/arch/frv/kernel/pm.c
> > index c1840d6..aa50333 100644
> > --- a/arch/frv/kernel/pm.c
> > +++ b/arch/frv/kernel/pm.c
> > @@ -401,17 +401,53 @@ static int cm_sysctl(ctl_table *table, int __user *name, int nlen,
> >  
> >  static struct ctl_table pm_table[] =
> >  {
> > -	{CTL_PM_SUSPEND, "suspend", NULL, 0, 0200, NULL, &sysctl_pm_do_suspend},
> > -	{CTL_PM_CMODE, "cmode", &clock_cmode_current, sizeof(int), 0644, NULL, &cmode_procctl, &cmode_sysctl, NULL},
> > -	{CTL_PM_P0, "p0", &clock_p0_current, sizeof(int), 0644, NULL, &p0_procctl, &p0_sysctl, NULL},
> > -	{CTL_PM_CM, "cm", &clock_cm_current, sizeof(int), 0644, NULL, &cm_procctl, &cm_sysctl, NULL},
> > -	{0}
> > +	{
> > +		.ctl_name	= CTL_PM_SUSPEND,
> > +		.procname	= "suspend",
> > +		.data		= NULL,
> > +		.maxlen		= 0,
> > +		.mode		= 0200,
> > +		.proc_handler	= &sysctl_pm_do_suspend,
> > +	},
> > +	{
> > +		.ctl_name	= CTL_PM_CMODE,
> > +		.procname	= "cmode",
> > +		.data		= &clock_cmode_current,
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= &cmode_procctl,
> > +		.strategy	= &cmode_sysctl,
> > +	},
> > +	{
> > +		.ctl_name	= CTL_PM_P0,
> > +		.procname	= "p0",
> > +		.data		= &clock_p0_current,
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= &p0_procctl,
> > +		.strategy	= &p0_sysctl,
> > +	},
> > +	{
> > +		.ctl_name	= CTL_PM_CM,
> > +		.procname	= "cm",
> > +		.data		= &clock_cm_current,
> > +		.maxlen		= sizeof(int),
> > +		.mode		= 0644,
> > +		.proc_handler	= &cm_procctl,
> > +		.strategy	= &cm_sysctl,
> > +	},
> > +	{ .ctl_name = 0}
> in next patch (26/59) you write just "{ }". .ctl_name = 0 not required here.

I'd prefer '{ 0 }' here, but I'm fine with the '{ .ctl_name = 0 }'
too, just '{ }' seems confusing, and it actually might get
misinterpreted too ..

best,
Herbert

> >  };
> >  
> >  static struct ctl_table pm_dir_table[] =
> >  {
> > -	{CTL_PM, "pm", NULL, 0, 0555, pm_table},
> > -	{0}
> > +	{
> > +		.ctl_name	= CTL_PM,
> > +		.procname	= "pm",
> > +		.mode		= 0555,
> > +		.child		= pm_table,
> > +	},
> > +	{ .ctl_name = 0}
> >  };
> >  
> >  /*
> 
> _______________________________________________
> Containers mailing list
> Containers@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

On Wed, Jan 17, 2007 at 12:31:22PM -0700, Eric W. Biederman wrote:
> Kirill Korotaev <dev@sw.ru> writes:
> 
> > Eric, though I personally don't care much:
> > 1. I ask for not setting your authorship/copyright on the code which you just
> > copied
> >   from other places. Just doesn't look polite IMHO.
> 
> I can't claim complete ownership of the code, there was plenty of feed back
> and contributions from others but the final form without a big switch
> statement is mine.  I certainly can't claim the table, it has been in
> that form for years.
> 
> If you notice I actually didn't say whose copyright it was :)  just
> that I wrote the file.
> 
> If there are copyright claims I should include I will be happy to do that.
> Mostly I was just trying to find some stupid boiler plate that would work.

IMHO that is fine ...

> > 2. I would propose to not introduce utsname_sysctl.c.
> >   both files are too small and minor that I can't see much reasons splitting
> > them.
> 
> The impact of moving this code out of sysctl.c is a major
> simplification, to sysctl.c.  Putting them in their own file means we
> can cleanly restrict the code to only be compiled CONFIG_SYSCTL is set.
> 
> It is a necessary first step to implementing a per process /proc/sys.
> 
> It reorganizes the ipc and utsname sysctl from a terribly fragile
> structure to something that is robust and easy to follow.  Code
> scattered all throughout sysctl.c was just a disaster.  We had
> several instances of having to fix bugs with odd combinations of
> CONFIG options, simply because the other spot that needed to be touched
> wasn't obvious.
> 
> So from my perspective this is an extremely worthwhile change that
> will make maintenance easier and is a small first step towards
> some nice future functionality.

yep, agreed ...

best,
Herbert

> Eric
> _______________________________________________
> Containers mailing list
> Containers@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

Fine by me.

Acked-By: David Howells <dhowells@redhat.com>
_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers

On Tue, 16 Jan 2007 09:39:54 -0700
"Eric W. Biederman" <ebiederm@xmission.com> wrote:

> From: Eric W. Biederman <ebiederm@xmission.com> - unquoted
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  init/main.c     |    3 +++
>  kernel/sysctl.c |    3 ---
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/init/main.c b/init/main.c
> index 8b4a7d7..8af5c6e 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -691,6 +691,9 @@ static void __init do_basic_setup(void)
>  #ifdef CONFIG_SYSCTL
>  	sysctl_init();
>  #endif
> +#ifdef CONFIG_PROC_FS
> +	init_irq_proc();
> +#endif
>  
>  	do_initcalls();
>  }
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 600b333..7420761 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1172,8 +1172,6 @@ static ctl_table dev_table[] = {
>  	{ .ctl_name = 0 }
>  };
>  
> -extern void init_irq_proc (void);
> -
>  static DEFINE_SPINLOCK(sysctl_lock);
>  
>  /* called under sysctl_lock */
> @@ -1219,7 +1217,6 @@ void __init sysctl_init(void)
>  {
>  #ifdef CONFIG_PROC_SYSCTL
>  	register_proc_table(root_table, proc_sys_root, &root_table_header);
> -	init_irq_proc();
>  #endif
>  }

sparc64:

init/main.c: In function `do_basic_setup':
init/main.c:707: warning: implicit declaration of function `init_irq_proc'

I couldn't be bothered working out how init/main.c is supposed to get at
its declaration of init_irq_proc().  It's not allowed to include
linux/irq.h.
_______________________________________________
Containers mailing list
Containers@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/containers