On 8/29/23 18:21, Matthew Wilcox wrote: > On Mon, Aug 28, 2023 at 10:52:12AM +0530, Anshuman Khandual wrote: >> -static int __init cmdline_parse_stack_guard_gap(char *p) >> +static int __init cmdline_parse_stack_guard_gap(char *str) >> { >> unsigned long val; >> - char *endptr; >> >> - val = simple_strtoul(p, &endptr, 10); >> - if (!*endptr) >> - stack_guard_gap = val << PAGE_SHIFT; >> + if (!str) >> + return 0; > > Please explain how this function can be called with a NULL pointer. This is an additional check just in case. We have similar constructs in the following __setup() functions as well. __setup("hashdist=", set_hashdist) __setup("numa_balancing=", setup_numabalancing) __setup("transparent_hugepage=", setup_transparent_hugepage) Also it might be a better to warn, when returning unhandled with 0 like in those scenarios. > >> - return 1; >> + val = simple_strtoul(str, &str, 10); >> + if (!*str && val) { >> + stack_guard_gap = val << PAGE_SHIFT; >> + return 1; >> + } >> + return 0; >> } > > Now you've removed the abillity for someone to say stack_guard_gap=0, > which seems potentially useful. In that case, should the following two scenarios be differentiated ? * stack_guard_gap= - Retains DEFAULT_STACK_GUARD_GAP * stack_guard_gap=0 - Changes to 0 pages