| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 5 Mar 2020 16:09:42 -0800
From: Mike Kravetz <mike.kravetz@...cle.com>
To: "Longpeng (Mike)" <longpeng2@...wei.com>
Cc: arei.gonglei@...wei.com, huangzhichao@...wei.com,
Matthew Wilcox <willy@...radead.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Qian Cai <cai@....pw>, linux-kernel@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [PATCH] mm/hugetlb: avoid weird message in hugetlb_init
On 3/4/20 7:30 PM, Longpeng(Mike) wrote:
> From: Longpeng <longpeng2@...wei.com>
>
> Some architectures(e.g. x86,risv) doesn't add 2M-hstate by default,
> so if we add 'default_hugepagesz=2M' but without 'hugepagesz=2M' in
> cmdline, we'll get a message as follow:
> "HugeTLB: unsupported default_hugepagesz 2097152. Reverting to 2097152"
Yes, that is a weird message that should not be printed. Thanks for
pointing out the issue!
> As architecture-specific HPAGE_SIZE hstate should be supported by
> default, we can avoid this weird message by add it if we hadn't yet.
As you have discovered, some of the hugetlb size setup is done in
architecture specific code. And other code is architecture independent.
The code which verifies huge page sizes (hugepagesz=) is architecture
specific. The code which reads default_hugepagesz= is architecture
independent. In fact, no verification of the 'default_hugepagesz' value
is made when value is read from the command line. The value is verified
later after command line processing. Current code considers the value
'valid' if it corresponds to a hstate previously setup by architecture
specific code. If architecture specific code did not set up a corresponding
hstate, an error like that above is reported.
Some architectures such as arm, powerpc and sparc set up hstates for all
supported sizes. Other architectures such as riscv, s390 and x86 only
set up hstates for those requested on the command line with hugepagesz=.
Depending on config options, x86 and riscv may or may not set up PUD_SIZE
hstates.
Therefore, on s390 and x86 and riscv (with certain config options) it
would be possible to specify a valid huge page size (PUD_SIZE) with
default_hugepagesz=, and have that value be rejected. This is because
the architecture specific code will not set up that hstate by default.
The code you have proposed handles the case where the value specified
by default_hugepagesz= coresponds to HPAGE_SIZE. That is because the
architecture independent code will set up the hstate for HPAGE_SIZE.
HPAGE_SIZE is the only valid huge page size known by the architecture
independent code.
I am thinking we may want to have a more generic solution by allowing
the default_hugepagesz= processing code to verify the passed size and
set up the corresponding hstate. This would require more cooperation
between architecture specific and independent code. This could be
accomplished with a simple arch_hugetlb_valid_size() routine provided
by the architectures. Below is an untested patch to add such support
to the architecture independent code and x86. Other architectures would
be similar.
In addition, with architectures providing arch_hugetlb_valid_size() it
should be possible to have a common routine in architecture independent
code to read/process hugepagesz= command line arguments.
Of course, another approach would be to simply require ALL architectures
to set up hstates for ALL supported huge page sizes.
Thoughts?
--
Mike Kravetz
diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
index f65cfb48cfdd..dc00c3df1f22 100644
--- a/arch/x86/include/asm/hugetlb.h
+++ b/arch/x86/include/asm/hugetlb.h
@@ -7,6 +7,9 @@
#define hugepages_supported() boot_cpu_has(X86_FEATURE_PSE)
+#define __HAVE_ARCH_HUGETLB_VALID_SIZE
+extern bool __init arch_hugetlb_valid_size(unsigned long size);
+
static inline int is_hugepage_only_range(struct mm_struct *mm,
unsigned long addr,
unsigned long len) {
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 5bfd5aef5378..1c4372bfe782 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -181,13 +181,22 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
#endif /* CONFIG_HUGETLB_PAGE */
#ifdef CONFIG_X86_64
+bool __init arch_hugetlb_valid_size(unsigned long size)
+{
+ if (size == PMD_SIZE)
+ return true;
+ else if (size == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES))
+ return true;
+ else
+ return false;
+}
+
static __init int setup_hugepagesz(char *opt)
{
unsigned long ps = memparse(opt, &opt);
- if (ps == PMD_SIZE) {
- hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
- } else if (ps == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES)) {
- hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+
+ if (arch_hugetlb_valid_size(ps)) {
+ hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
} else {
hugetlb_bad_size();
printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n",
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 50480d16bd33..822d0d8559c7 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -678,6 +678,13 @@ static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
return &mm->page_table_lock;
}
+#ifndef __HAVE_ARCH_HUGETLB_VALID_SIZE
+static inline bool arch_hugetlb_valid_size(unsigned long size)
+{
+ return (size == HPAGE_SIZE);
+}
+#endif
+
#ifndef hugepages_supported
/*
* Some platform decide whether they support huge pages at boot
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 7fb31750e670..fc3f0f1e3a27 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3078,17 +3078,13 @@ static int __init hugetlb_init(void)
if (!hugepages_supported())
return 0;
+ /* if default_hstate_size != 0, corresponding hstate was added */
if (!size_to_hstate(default_hstate_size)) {
- if (default_hstate_size != 0) {
- pr_err("HugeTLB: unsupported default_hugepagesz %lu. Reverting to %lu\n",
- default_hstate_size, HPAGE_SIZE);
- }
-
default_hstate_size = HPAGE_SIZE;
- if (!size_to_hstate(default_hstate_size))
- hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
+ hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
}
default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
+
if (default_hstate_max_huge_pages) {
if (!default_hstate.max_huge_pages)
default_hstate.max_huge_pages = default_hstate_max_huge_pages;
@@ -3195,7 +3191,15 @@ __setup("hugepages=", hugetlb_nrpages_setup);
static int __init hugetlb_default_setup(char *s)
{
- default_hstate_size = memparse(s, &s);
+ unsigned long size = memparse(s, &s);
+
+ if (arch_hugetlb_valid_size(size)) {
+ default_hstate_size = size;
+ hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+ } else {
+ pr_err("HugeTLB: unsupported default_hugepagesz %lu.\n", size);
+ hugetlb_bad_size();
+ }
return 1;
}
__setup("default_hugepagesz=", hugetlb_default_setup);
Powered by blists - more mailing lists