Messages in this thread Patch in this message | | | Date | Thu, 04 Dec 2014 07:12:45 +0100 | From | Manfred Spraul <> | Subject | Re: [PATCH] kernel: sysctl: use 'unsigned long' type for 'zero' variable |
| |
Hi Andrew,
On 12/04/2014 12:25 AM, Andrew Morton wrote: > On Wed, 03 Dec 2014 15:41:21 +0300 Andrey Ryabinin <a.ryabinin@samsung.com> wrote: > >> Use the 'unsigned long' type for 'zero' variable to fix this. >> Changing type to 'unsigned long' shouldn't affect any other users >> of this variable. >> >> Reported-by: Dmitry Vyukov <dvyukov@google.com> >> Fixes: ed4d4902ebdd ("mm, hugetlb: remove hugetlb_zero and hugetlb_infinity") >> Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com> >> --- >> kernel/sysctl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 15f2511..45c45c9 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -120,7 +120,7 @@ static int sixty = 60; >> >> static int __maybe_unused neg_one = -1; >> >> -static int zero; >> +static unsigned long zero; >> static int __maybe_unused one = 1; >> static int __maybe_unused two = 2; >> static int __maybe_unused four = 4; > Yeah, this is ghastly. > > Look at > > { > .procname = "numa_balancing", > .data = NULL, /* filled in by handler */ > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = sysctl_numa_balancing, > .extra1 = &zero, > .extra2 = &one, > }, > > Now extra1 points at a long and extra2 points at an int. > sysctl_numa_balancing() calls proc_dointvec_minmax() and I think your > patch just broke big-endian 64-bit machines. "sched_autogroup_enabled" > breaks as well. What about getting rid of "extra1" and "extra2" as well and replace it with "min" and "max"? I've attached an idea
> and change proc_dointvec_minmax() and a million other functions to take > `union sysctl_payload *' arguments. But I haven't thought about it much. Another idea: why do we pass "int *" instead of "int"?
With "int", we could use .int_min = 0; .int_max = 1;
-- Manfred From 7a210bec3d9dc3382ef0d6809a7742856373bbee Mon Sep 17 00:00:00 2001 From: Manfred Spraul <manfred@colorfullife.com> Date: Thu, 4 Dec 2014 07:03:39 +0100 Subject: [PATCH] Allow type safe & documented sysctl
Idea from Andrew: - add a union into struct ctl_table instead of the void * - further idea: replace "extra1" and "extra2" with min/max - use it for ipc
Notes: - not tested - not coding style reviewed - open FIXME in ipc_sysctl.c
Signed-off-by: Manfred Spraul <manfred@colorfullife.com> --- include/linux/sysctl.h | 16 ++++++++++++++-- ipc/ipc_sysctl.c | 34 ++++++++++++++++++---------------- 2 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index b7361f8..acc7581 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -111,8 +111,20 @@ struct ctl_table struct ctl_table *child; /* Deprecated */ proc_handler *proc_handler; /* Callback for text formatting */ struct ctl_table_poll *poll; - void *extra1; - void *extra2; + union { + void *extra1; + int *int_min; + long *long_min; + unsigned int *uint_min; + unsigned long *ulong_min; + }; + union { + void *extra2; + int *int_max; + long *long_max; + unsigned int *uint_max; + unsigned long *ulong_max; + }; }; struct ctl_node { diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c index c3f0326..50a6e1c 100644 --- a/ipc/ipc_sysctl.c +++ b/ipc/ipc_sysctl.c @@ -167,6 +167,7 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.shm_ctlmax), .mode = 0644, .proc_handler = proc_ipc_doulongvec_minmax, + /* FIXME: Why no ulong_min & ulong_max ?? */ }, { .procname = "shmall", @@ -174,6 +175,7 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.shm_ctlall), .mode = 0644, .proc_handler = proc_ipc_doulongvec_minmax, + /* FIXME: Why no ulong_min & ulong_max ?? */ }, { .procname = "shmmni", @@ -188,8 +190,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.shm_rmid_forced), .mode = 0644, .proc_handler = proc_ipc_dointvec_minmax_orphans, - .extra1 = &zero, - .extra2 = &one, + .int_min = &zero, + .int_max = &one, }, { .procname = "msgmax", @@ -197,8 +199,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.msg_ctlmax), .mode = 0644, .proc_handler = proc_ipc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &int_max, + .int_min = &zero, + .int_max = &int_max, }, { .procname = "msgmni", @@ -206,8 +208,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.msg_ctlmni), .mode = 0644, .proc_handler = proc_ipc_callback_dointvec_minmax, - .extra1 = &zero, - .extra2 = &int_max, + .int_min = &zero, + .int_max = &int_max, }, { .procname = "msgmnb", @@ -215,8 +217,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.msg_ctlmnb), .mode = 0644, .proc_handler = proc_ipc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &int_max, + .int_min = &zero, + .int_max = &int_max, }, { .procname = "sem", @@ -231,8 +233,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_ipcauto_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .int_min = &zero, + .int_max = &one, }, #ifdef CONFIG_CHECKPOINT_RESTORE { @@ -241,8 +243,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.ids[IPC_SEM_IDS].next_id), .mode = 0644, .proc_handler = proc_ipc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &int_max, + .int_min = &zero, + .int_max = &int_max, }, { .procname = "msg_next_id", @@ -250,8 +252,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.ids[IPC_MSG_IDS].next_id), .mode = 0644, .proc_handler = proc_ipc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &int_max, + .int_min = &zero, + .int_max = &int_max, }, { .procname = "shm_next_id", @@ -259,8 +261,8 @@ static struct ctl_table ipc_kern_table[] = { .maxlen = sizeof(init_ipc_ns.ids[IPC_SHM_IDS].next_id), .mode = 0644, .proc_handler = proc_ipc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &int_max, + .int_min = &zero, + .int_max = &int_max, }, #endif {} -- 1.9.3
| |