diff mbox series

[v4] hugetlbfs: Get unmapped area below TASK_UNMAPPED_BASE for hugetlbfs

Message ID 20200513135742.102064-1-hushijie3@huawei.com (mailing list archive)
State New, archived
Headers show
Series [v4] hugetlbfs: Get unmapped area below TASK_UNMAPPED_BASE for hugetlbfs | expand

Commit Message

Shijie Hu May 13, 2020, 1:57 p.m. UTC
Here is a final patch to solve that hugetlb_get_unmapped_area() can't
get unmapped area below mmap base for huge pages based on a few previous
discussions and patches from me.

I'm so sorry. When sending v2 and v3 patches, I forget to cc:
linux-mm@kvack.org and linux-kernel@vger.kernel.org. No records of these
two patches found on the https://lkml.org/lkml/.

Patch V1: https://lkml.org/lkml/2020/4/27/440

Changes in V2:
* Follow Mike's suggestions, move hugetlb_get_unmapped_area() routines
from "fs/hugetlbfs/inode.c" to "arch/arm64/mm/hugetlbpage.c", without
changing core code.
* Add mmap_is_legacy() function, and only fall back to the bottom-up
function on no-legacy mode.

Changes in V3:
* Add *bottomup() and *topdown() two function, and check if
mm->get_unmapped_area is equal to arch_get_unmapped_area() instead of
checking mmap_is_legacy() to determine which function should be used.

Changes in V4:
* Follow the suggestions of Will and Mike, move back this patch to core
code.

------

In a 32-bit program, running on arm64 architecture. When the address
space below mmap base is completely exhausted, shmat() for huge pages
will return ENOMEM, but shmat() for normal pages can still success on
no-legacy mode. This seems not fair.

For normal pages, get_unmapped_area() calling flows are:
    => mm->get_unmapped_area()
	if on legacy mode,
	    => arch_get_unmapped_area()
			=> vm_unmapped_area()
	if on no-legacy mode,
	    => arch_get_unmapped_area_topdown()
			=> vm_unmapped_area()

For huge pages, get_unmapped_area() calling flows are:
    => file->f_op->get_unmapped_area()
		=> hugetlb_get_unmapped_area()
			=> vm_unmapped_area()

To solve this issue, we only need to make hugetlb_get_unmapped_area() take
the same way as mm->get_unmapped_area(). Add *bottomup() and *topdown()
two functions, and check current mm->get_unmapped_area() to decide which 
one to use. If mm->get_unmapped_area is equal to arch_get_unmapped_area(),
hugetlb_get_unmapped_area() calls bottomup routine, otherwise calls topdown
routine.

Signed-off-by: Shijie Hu <hushijie3@huawei.com>
---
 fs/hugetlbfs/inode.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 8 deletions(-)

Comments

kernel test robot May 13, 2020, 7:11 p.m. UTC | #1
Hi Shijie,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc5 next-20200512]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Shijie-Hu/hugetlbfs-Get-unmapped-area-below-TASK_UNMAPPED_BASE-for-hugetlbfs/20200513-221024
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 24085f70a6e1b0cb647ec92623284641d8270637
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

fs/hugetlbfs/inode.c: In function 'hugetlb_get_unmapped_area':
>> fs/hugetlbfs/inode.c:268:31: error: 'arch_get_unmapped_area' undeclared (first use in this function); did you mean 'thp_get_unmapped_area'?
268 |  if (mm->get_unmapped_area == arch_get_unmapped_area)
|                               ^~~~~~~~~~~~~~~~~~~~~~
|                               thp_get_unmapped_area
fs/hugetlbfs/inode.c:268:31: note: each undeclared identifier is reported only once for each function it appears in

vim +268 fs/hugetlbfs/inode.c

   240	
   241	static unsigned long
   242	hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
   243			unsigned long len, unsigned long pgoff, unsigned long flags)
   244	{
   245		struct mm_struct *mm = current->mm;
   246		struct vm_area_struct *vma;
   247		struct hstate *h = hstate_file(file);
   248	
   249		if (len & ~huge_page_mask(h))
   250			return -EINVAL;
   251		if (len > TASK_SIZE)
   252			return -ENOMEM;
   253	
   254		if (flags & MAP_FIXED) {
   255			if (prepare_hugepage_range(file, addr, len))
   256				return -EINVAL;
   257			return addr;
   258		}
   259	
   260		if (addr) {
   261			addr = ALIGN(addr, huge_page_size(h));
   262			vma = find_vma(mm, addr);
   263			if (TASK_SIZE - len >= addr &&
   264			    (!vma || addr + len <= vm_start_gap(vma)))
   265				return addr;
   266		}
   267	
 > 268		if (mm->get_unmapped_area == arch_get_unmapped_area)
   269			return hugetlb_get_unmapped_area_bottomup(file, addr, len,
   270					pgoff, flags);
   271		return hugetlb_get_unmapped_area_topdown(file, addr, len,
   272				pgoff, flags);
   273	}
   274	#endif
   275	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 991c60c7ffe0..bbee893e88da 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -191,13 +191,60 @@  static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
 
 #ifndef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
 static unsigned long
+hugetlb_get_unmapped_area_bottomup(struct file *file, unsigned long addr,
+		unsigned long len, unsigned long pgoff, unsigned long flags)
+{
+	struct hstate *h = hstate_file(file);
+	struct vm_unmapped_area_info info;
+
+	info.flags = 0;
+	info.length = len;
+	info.low_limit = current->mm->mmap_base;
+	info.high_limit = TASK_SIZE;
+	info.align_mask = PAGE_MASK & ~huge_page_mask(h);
+	info.align_offset = 0;
+	return vm_unmapped_area(&info);
+}
+
+static unsigned long
+hugetlb_get_unmapped_area_topdown(struct file *file, unsigned long addr,
+		unsigned long len, unsigned long pgoff, unsigned long flags)
+{
+	struct hstate *h = hstate_file(file);
+	struct vm_unmapped_area_info info;
+
+	info.flags = VM_UNMAPPED_AREA_TOPDOWN;
+	info.length = len;
+	info.low_limit = max(PAGE_SIZE, mmap_min_addr);
+	info.high_limit = current->mm->mmap_base;
+	info.align_mask = PAGE_MASK & ~huge_page_mask(h);
+	info.align_offset = 0;
+	addr = vm_unmapped_area(&info);
+
+	/*
+	 * A failed mmap() very likely causes application failure,
+	 * so fall back to the bottom-up function here. This scenario
+	 * can happen with large stack limits and large mmap()
+	 * allocations.
+	 */
+	if (unlikely(offset_in_page(addr))) {
+		VM_BUG_ON(addr != -ENOMEM);
+		info.flags = 0;
+		info.low_limit = current->mm->mmap_base;
+		info.high_limit = TASK_SIZE;
+		addr = vm_unmapped_area(&info);
+	}
+
+	return addr;
+}
+
+static unsigned long
 hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
 		unsigned long len, unsigned long pgoff, unsigned long flags)
 {
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma;
 	struct hstate *h = hstate_file(file);
-	struct vm_unmapped_area_info info;
 
 	if (len & ~huge_page_mask(h))
 		return -EINVAL;
@@ -218,13 +265,11 @@  hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
 			return addr;
 	}
 
-	info.flags = 0;
-	info.length = len;
-	info.low_limit = TASK_UNMAPPED_BASE;
-	info.high_limit = TASK_SIZE;
-	info.align_mask = PAGE_MASK & ~huge_page_mask(h);
-	info.align_offset = 0;
-	return vm_unmapped_area(&info);
+	if (mm->get_unmapped_area == arch_get_unmapped_area)
+		return hugetlb_get_unmapped_area_bottomup(file, addr, len,
+				pgoff, flags);
+	return hugetlb_get_unmapped_area_topdown(file, addr, len,
+			pgoff, flags);
 }
 #endif