| Message ID | 20220401101232.2790280-3-liupeng256@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hugetlb: Fix confusing behavior | expand |
On 01.04.22 12:12, Peng Liu wrote: > When __setup() return '0', using invalid option values causes the > entire kernel boot option string to be reported as Unknown. Hugetlb > calls __setup() and will return '0' when set invalid parameter > string. > > The following phenomenon is observed: > cmdline: > hugepagesz=1Y hugepages=1 > dmesg: > HugeTLB: unsupported hugepagesz=1Y > HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring > Unknown kernel command line parameters "hugepagesz=1Y hugepages=1" > > Since hugetlb will print warn or error information before return for > invalid parameter string, just use return '1' to avoid print again. > > Signed-off-by: Peng Liu <liupeng256@huawei.com> > --- > mm/hugetlb.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 9cd746432ca9..6dde34c115c9 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -4131,12 +4131,11 @@ static int __init hugepages_setup(char *s) > int count; > unsigned long tmp; > char *p = s; > - int ret = 1; Adding this in #1 to remove it in #2 is a bit sub-optimal IMHO.
On 2022/4/1 18:46, David Hildenbrand wrote: > On 01.04.22 12:12, Peng Liu wrote: >> When __setup() return '0', using invalid option values causes the >> entire kernel boot option string to be reported as Unknown. Hugetlb >> calls __setup() and will return '0' when set invalid parameter >> string. >> >> The following phenomenon is observed: >> cmdline: >> hugepagesz=1Y hugepages=1 >> dmesg: >> HugeTLB: unsupported hugepagesz=1Y >> HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring >> Unknown kernel command line parameters "hugepagesz=1Y hugepages=1" >> >> Since hugetlb will print warn or error information before return for >> invalid parameter string, just use return '1' to avoid print again. >> >> Signed-off-by: Peng Liu<liupeng256@huawei.com> >> --- >> mm/hugetlb.c | 18 ++++++++---------- >> 1 file changed, 8 insertions(+), 10 deletions(-) >> >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 9cd746432ca9..6dde34c115c9 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -4131,12 +4131,11 @@ static int __init hugepages_setup(char *s) >> int count; >> unsigned long tmp; >> char *p = s; >> - int ret = 1; > Adding this in #1 to remove it in #2 is a bit sub-optimal IMHO. > For #2, which is not necessary for stable, #1 may be needed for stable, this is why we split #2 into a single patch.
On 02.04.22 03:33, liupeng (DM) wrote: > > On 2022/4/1 18:46, David Hildenbrand wrote: >> On 01.04.22 12:12, Peng Liu wrote: >>> When __setup() return '0', using invalid option values causes the >>> entire kernel boot option string to be reported as Unknown. Hugetlb >>> calls __setup() and will return '0' when set invalid parameter >>> string. >>> >>> The following phenomenon is observed: >>> cmdline: >>> hugepagesz=1Y hugepages=1 >>> dmesg: >>> HugeTLB: unsupported hugepagesz=1Y >>> HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring >>> Unknown kernel command line parameters "hugepagesz=1Y hugepages=1" >>> >>> Since hugetlb will print warn or error information before return for >>> invalid parameter string, just use return '1' to avoid print again. >>> >>> Signed-off-by: Peng Liu <liupeng256@huawei.com> >>> --- >>> mm/hugetlb.c | 18 ++++++++---------- >>> 1 file changed, 8 insertions(+), 10 deletions(-) >>> >>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >>> index 9cd746432ca9..6dde34c115c9 100644 >>> --- a/mm/hugetlb.c >>> +++ b/mm/hugetlb.c >>> @@ -4131,12 +4131,11 @@ static int __init hugepages_setup(char *s) >>> int count; >>> unsigned long tmp; >>> char *p = s; >>> - int ret = 1; >> Adding this in #1 to remove it in #2 is a bit sub-optimal IMHO. >> > For #2, which is not necessary for stable, #1 may be needed for stable, > this is why we split #2 into a single patch. > Again, I don't think #1 is stable material, sorry.
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 9cd746432ca9..6dde34c115c9 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -4131,12 +4131,11 @@ static int __init hugepages_setup(char *s) int count; unsigned long tmp; char *p = s; - int ret = 1; if (!parsed_valid_hugepagesz) { pr_warn("HugeTLB: hugepages=%s does not follow a valid hugepagesz, ignoring\n", s); parsed_valid_hugepagesz = true; - return 0; + return 1; } /* @@ -4152,7 +4151,7 @@ static int __init hugepages_setup(char *s) if (mhp == last_mhp) { pr_warn("HugeTLB: hugepages= specified twice without interleaving hugepagesz=, ignoring hugepages=%s\n", s); - return 0; + return 1; } while (*p) { @@ -4163,7 +4162,7 @@ static int __init hugepages_setup(char *s) if (p[count] == ':') { if (!hugetlb_node_alloc_supported()) { pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n"); - return 0; + return 1; } if (tmp >= nr_online_nodes) goto invalid; @@ -4201,11 +4200,10 @@ static int __init hugepages_setup(char *s) last_mhp = mhp; - return ret; + return 1; invalid: pr_warn("HugeTLB: Invalid hugepages parameter %s\n", p); - ret = 0; goto out; } __setup("hugepages=", hugepages_setup); @@ -4227,7 +4225,7 @@ static int __init hugepagesz_setup(char *s) if (!arch_hugetlb_valid_size(size)) { pr_err("HugeTLB: unsupported hugepagesz=%s\n", s); - return 0; + return 1; } h = size_to_hstate(size); @@ -4242,7 +4240,7 @@ static int __init hugepagesz_setup(char *s) if (!parsed_default_hugepagesz || h != &default_hstate || default_hstate.max_huge_pages) { pr_warn("HugeTLB: hugepagesz=%s specified twice, ignoring\n", s); - return 0; + return 1; } /* @@ -4273,14 +4271,14 @@ static int __init default_hugepagesz_setup(char *s) parsed_valid_hugepagesz = false; if (parsed_default_hugepagesz) { pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n", s); - return 0; + return 1; } size = (unsigned long)memparse(s, NULL); if (!arch_hugetlb_valid_size(size)) { pr_err("HugeTLB: unsupported default_hugepagesz=%s\n", s); - return 0; + return 1; } hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
When __setup() return '0', using invalid option values causes the entire kernel boot option string to be reported as Unknown. Hugetlb calls __setup() and will return '0' when set invalid parameter string. The following phenomenon is observed: cmdline: hugepagesz=1Y hugepages=1 dmesg: HugeTLB: unsupported hugepagesz=1Y HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring Unknown kernel command line parameters "hugepagesz=1Y hugepages=1" Since hugetlb will print warn or error information before return for invalid parameter string, just use return '1' to avoid print again. Signed-off-by: Peng Liu <liupeng256@huawei.com> --- mm/hugetlb.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)