Messages in this thread | | | Date | Thu, 04 Dec 2014 14:35:20 +0300 | From | Andrey Ryabinin <> | Subject | Re: [PATCH] kernel: sysctl: use 'unsigned long' type for 'zero' variable |
| |
On 12/04/2014 03:19 AM, Andrew Morton wrote: > On Wed, 3 Dec 2014 15:25:24 -0800 Andrew Morton <akpm@linux-foundation.org> 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. > > Taking another look at this... > > numa_balancing will continue to work on big-endian because of course > zero is still zero when byteswapped. But that's such a hack, isn't > documented and doesn't work for "one", "sixty", etc. >
Yeah, I agree it's a bit hacky.
> I'm thinking a better fix here is to switch hugetlb_sysctl_handler to > use `int's. 2^32 hugepages is enough for anybody. >
It's 8 petabytes for 2MB pages, so yeah should be enough. Perhaps it also makes sense to change types for counters in 'struct hstate' from longs to ints.
> hugetlb_overcommit_handler() will need conversion also. > > Perhaps auditing all the proc_doulongvec_minmax callsites is the way to > attack this. >
I've looked through this yesterday and didn't found anything obviously wrong. Though I could easily miss something.
| |