| Message ID | 20200318220634.32100-3-mike.kravetz@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean up hugetlb boot command line processing | expand |
Le 18/03/2020 à 23:06, Mike Kravetz a écrit : > Now that architectures provide arch_hugetlb_valid_size(), parsing > of "hugepagesz=" can be done in architecture independent code. > Create a single routine to handle hugepagesz= parsing and remove > all arch specific routines. We can also remove the interface > hugetlb_bad_size() as this is no longer used outside arch independent > code. > > This also provides consistent behavior of hugetlbfs command line > options. The hugepagesz= option should only be specified once for > a specific size, but some architectures allow multiple instances. > This appears to be more of an oversight when code was added by some > architectures to set up ALL huge pages sizes. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > --- > arch/arm64/mm/hugetlbpage.c | 15 --------------- > arch/powerpc/mm/hugetlbpage.c | 15 --------------- > arch/riscv/mm/hugetlbpage.c | 16 ---------------- > arch/s390/mm/hugetlbpage.c | 18 ------------------ > arch/sparc/mm/init_64.c | 22 ---------------------- > arch/x86/mm/hugetlbpage.c | 16 ---------------- > include/linux/hugetlb.h | 1 - > mm/hugetlb.c | 24 ++++++++++++++++++------ > 8 files changed, 18 insertions(+), 109 deletions(-) > [snip] > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 2f99359b93af..cd4ec07080fb 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void) > } > subsys_initcall(hugetlb_init); > > -/* Should be called on processing a hugepagesz=... option */ > -void __init hugetlb_bad_size(void) > -{ > - parsed_valid_hugepagesz = false; > -} > - > void __init hugetlb_add_hstate(unsigned int order) > { > struct hstate *h; > @@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s) > } > __setup("hugepages=", hugetlb_nrpages_setup); > > +static int __init hugepagesz_setup(char *s) > +{ > + unsigned long long size; > + char *saved_s = s; > + > + size = memparse(s, &s); You don't use s after that, so you can pass NULL instead of &s and avoid the saved_s > + > + if (!arch_hugetlb_valid_size(size)) { > + parsed_valid_hugepagesz = false; > + pr_err("HugeTLB: unsupported hugepagesz %s\n", saved_s); > + return 0; > + } > + > + hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT); > + return 1; > +} > +__setup("hugepagesz=", hugepagesz_setup); > + > static int __init default_hugepagesz_setup(char *s) > { > unsigned long long size; > Christophe
On 3/19/20 12:04 AM, Christophe Leroy wrote: > > > Le 18/03/2020 à 23:06, Mike Kravetz a écrit : >> Now that architectures provide arch_hugetlb_valid_size(), parsing >> of "hugepagesz=" can be done in architecture independent code. >> Create a single routine to handle hugepagesz= parsing and remove >> all arch specific routines. We can also remove the interface >> hugetlb_bad_size() as this is no longer used outside arch independent >> code. >> >> This also provides consistent behavior of hugetlbfs command line >> options. The hugepagesz= option should only be specified once for >> a specific size, but some architectures allow multiple instances. >> This appears to be more of an oversight when code was added by some >> architectures to set up ALL huge pages sizes. >> >> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> >> --- >> arch/arm64/mm/hugetlbpage.c | 15 --------------- >> arch/powerpc/mm/hugetlbpage.c | 15 --------------- >> arch/riscv/mm/hugetlbpage.c | 16 ---------------- >> arch/s390/mm/hugetlbpage.c | 18 ------------------ >> arch/sparc/mm/init_64.c | 22 ---------------------- >> arch/x86/mm/hugetlbpage.c | 16 ---------------- >> include/linux/hugetlb.h | 1 - >> mm/hugetlb.c | 24 ++++++++++++++++++------ >> 8 files changed, 18 insertions(+), 109 deletions(-) >> > > [snip] > >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 2f99359b93af..cd4ec07080fb 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void) >> } >> subsys_initcall(hugetlb_init); >> -/* Should be called on processing a hugepagesz=... option */ >> -void __init hugetlb_bad_size(void) >> -{ >> - parsed_valid_hugepagesz = false; >> -} >> - >> void __init hugetlb_add_hstate(unsigned int order) >> { >> struct hstate *h; >> @@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s) >> } >> __setup("hugepages=", hugetlb_nrpages_setup); >> +static int __init hugepagesz_setup(char *s) >> +{ >> + unsigned long long size; >> + char *saved_s = s; >> + >> + size = memparse(s, &s); > > You don't use s after that, so you can pass NULL instead of &s and avoid the saved_s Thanks! I'll incorporate in v2.
On Thu, Mar 19, 2020 at 12:04 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > > > > Le 18/03/2020 à 23:06, Mike Kravetz a écrit : > > Now that architectures provide arch_hugetlb_valid_size(), parsing > > of "hugepagesz=" can be done in architecture independent code. > > Create a single routine to handle hugepagesz= parsing and remove > > all arch specific routines. We can also remove the interface > > hugetlb_bad_size() as this is no longer used outside arch independent > > code. > > > > This also provides consistent behavior of hugetlbfs command line > > options. The hugepagesz= option should only be specified once for > > a specific size, but some architectures allow multiple instances. > > This appears to be more of an oversight when code was added by some > > architectures to set up ALL huge pages sizes. > > > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > > --- > > arch/arm64/mm/hugetlbpage.c | 15 --------------- > > arch/powerpc/mm/hugetlbpage.c | 15 --------------- > > arch/riscv/mm/hugetlbpage.c | 16 ---------------- > > arch/s390/mm/hugetlbpage.c | 18 ------------------ > > arch/sparc/mm/init_64.c | 22 ---------------------- > > arch/x86/mm/hugetlbpage.c | 16 ---------------- > > include/linux/hugetlb.h | 1 - > > mm/hugetlb.c | 24 ++++++++++++++++++------ > > 8 files changed, 18 insertions(+), 109 deletions(-) > > > > [snip] > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > index 2f99359b93af..cd4ec07080fb 100644 > > --- a/mm/hugetlb.c > > +++ b/mm/hugetlb.c > > @@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void) > > } > > subsys_initcall(hugetlb_init); > > > > -/* Should be called on processing a hugepagesz=... option */ > > -void __init hugetlb_bad_size(void) > > -{ > > - parsed_valid_hugepagesz = false; > > -} > > - > > void __init hugetlb_add_hstate(unsigned int order) > > { > > struct hstate *h; > > @@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s) > > } > > __setup("hugepages=", hugetlb_nrpages_setup); > > > > +static int __init hugepagesz_setup(char *s) > > +{ > > + unsigned long long size; > > + char *saved_s = s; > > + > > + size = memparse(s, &s); > > You don't use s after that, so you can pass NULL instead of &s and avoid > the saved_s > +1 Acked-by: Mina Almasry <almasrymina@google.com>
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c index da30127086d0..4aa9534a45d7 100644 --- a/arch/arm64/mm/hugetlbpage.c +++ b/arch/arm64/mm/hugetlbpage.c @@ -476,18 +476,3 @@ bool __init arch_hugetlb_valid_size(unsigned long long size) return false; } - -static __init int setup_hugepagesz(char *opt) -{ - unsigned long long ps = memparse(opt, &opt); - - if arch_hugetlb_valid_size(ps)) { - add_huge_page_size(ps); - return 1; - } - - hugetlb_bad_size(); - pr_err("hugepagesz: Unsupported page size %llu K\n", ps >> 10); - return 0; -} -__setup("hugepagesz=", setup_hugepagesz); diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c index b78f660252f3..166960ba1236 100644 --- a/arch/powerpc/mm/hugetlbpage.c +++ b/arch/powerpc/mm/hugetlbpage.c @@ -589,21 +589,6 @@ static int __init add_huge_page_size(unsigned long long size) return 0; } -static int __init hugepage_setup_sz(char *str) -{ - unsigned long long size; - - size = memparse(str, &str); - - if (add_huge_page_size(size) != 0) { - hugetlb_bad_size(); - pr_err("Invalid huge page size specified(%llu)\n", size); - } - - return 1; -} -__setup("hugepagesz=", hugepage_setup_sz); - static int __init hugetlbpage_init(void) { bool configured = false; diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c index f1990882f16c..bdf89d7eb714 100644 --- a/arch/riscv/mm/hugetlbpage.c +++ b/arch/riscv/mm/hugetlbpage.c @@ -22,22 +22,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size) return false; } -static __init int setup_hugepagesz(char *opt) -{ - unsigned long long ps = memparse(opt, &opt); - - if (arch_hugetlb_valid_size(ps)) { - hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT); - return 1; - } - - hugetlb_bad_size(); - pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20); - return 0; - -} -__setup("hugepagesz=", setup_hugepagesz); - #ifdef CONFIG_CONTIG_ALLOC static __init int gigantic_pages_init(void) { diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c index d92e8c5c3e71..b809762f206e 100644 --- a/arch/s390/mm/hugetlbpage.c +++ b/arch/s390/mm/hugetlbpage.c @@ -261,24 +261,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size) return false; } -static __init int setup_hugepagesz(char *opt) -{ - unsigned long long size; - char *string = opt; - - size = memparse(opt, &opt); - if (arch_hugetlb_valid_size(size)) { - hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT); - } else { - hugetlb_bad_size(); - pr_err("hugepagesz= specifies an unsupported page size %s\n", - string); - return 0; - } - return 1; -} -__setup("hugepagesz=", setup_hugepagesz); - static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags) diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c index 4cc248817b19..5c29203fd460 100644 --- a/arch/sparc/mm/init_64.c +++ b/arch/sparc/mm/init_64.c @@ -398,28 +398,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size) return true; } - -static int __init setup_hugepagesz(char *string) -{ - unsigned long long hugepage_size; - int rc = 0; - - hugepage_size = memparse(string, &string); - - if (!arch_hugetlb_valid_size(hugepage_size)) { - hugetlb_bad_size(); - pr_err("hugepagesz=%llu not supported by MMU.\n", - hugepage_size); - goto out; - } - - add_huge_page_size(hugepage_size); - rc = 1; - -out: - return rc; -} -__setup("hugepagesz=", setup_hugepagesz); #endif /* CONFIG_HUGETLB_PAGE */ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep) diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c index 51e6208fdeec..dd3ed09f6c23 100644 --- a/arch/x86/mm/hugetlbpage.c +++ b/arch/x86/mm/hugetlbpage.c @@ -191,22 +191,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size) return false; } -static __init int setup_hugepagesz(char *opt) -{ - unsigned long long ps = memparse(opt, &opt); - - if (arch_hugetlb_valid_size(ps)) { - hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT); - } else { - hugetlb_bad_size(); - printk(KERN_ERR "hugepagesz: Unsupported page size %llu M\n", - ps >> 20); - return 0; - } - return 1; -} -__setup("hugepagesz=", setup_hugepagesz); - #ifdef CONFIG_CONTIG_ALLOC static __init int gigantic_pages_init(void) { diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index 33343eb980d0..47244853ceb4 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -504,7 +504,6 @@ int huge_add_to_page_cache(struct page *page, struct address_space *mapping, int __init __alloc_bootmem_huge_page(struct hstate *h); int __init alloc_bootmem_huge_page(struct hstate *h); -void __init hugetlb_bad_size(void); void __init hugetlb_add_hstate(unsigned order); struct hstate *size_to_hstate(unsigned long size); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 2f99359b93af..cd4ec07080fb 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void) } subsys_initcall(hugetlb_init); -/* Should be called on processing a hugepagesz=... option */ -void __init hugetlb_bad_size(void) -{ - parsed_valid_hugepagesz = false; -} - void __init hugetlb_add_hstate(unsigned int order) { struct hstate *h; @@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s) } __setup("hugepages=", hugetlb_nrpages_setup); +static int __init hugepagesz_setup(char *s) +{ + unsigned long long size; + char *saved_s = s; + + size = memparse(s, &s); + + if (!arch_hugetlb_valid_size(size)) { + parsed_valid_hugepagesz = false; + pr_err("HugeTLB: unsupported hugepagesz %s\n", saved_s); + return 0; + } + + hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT); + return 1; +} +__setup("hugepagesz=", hugepagesz_setup); + static int __init default_hugepagesz_setup(char *s) { unsigned long long size;
Now that architectures provide arch_hugetlb_valid_size(), parsing of "hugepagesz=" can be done in architecture independent code. Create a single routine to handle hugepagesz= parsing and remove all arch specific routines. We can also remove the interface hugetlb_bad_size() as this is no longer used outside arch independent code. This also provides consistent behavior of hugetlbfs command line options. The hugepagesz= option should only be specified once for a specific size, but some architectures allow multiple instances. This appears to be more of an oversight when code was added by some architectures to set up ALL huge pages sizes. Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> --- arch/arm64/mm/hugetlbpage.c | 15 --------------- arch/powerpc/mm/hugetlbpage.c | 15 --------------- arch/riscv/mm/hugetlbpage.c | 16 ---------------- arch/s390/mm/hugetlbpage.c | 18 ------------------ arch/sparc/mm/init_64.c | 22 ---------------------- arch/x86/mm/hugetlbpage.c | 16 ---------------- include/linux/hugetlb.h | 1 - mm/hugetlb.c | 24 ++++++++++++++++++------ 8 files changed, 18 insertions(+), 109 deletions(-)