[patch] hugetlb: add hugepage reservation upon mremap expansion.

[patch] hugetlb: add hugepage reservation upon mremap expansion.

[Ken Chen]

hugetlb: add hugepage reservation upon mremap expansion

hugetlb page has a semantics that it reserves pages up front at the time
of mmap.  We need to extend the same reservation scheme for the mremap
expansion case.

Signed-off-by: Ken Chen <kenchen@google.com>

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index c36d851..5d22933 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -28,6 +28,8 @@ int hugetlb_mempolicy_sysctl_handler(
 
 int move_hugetlb_page_tables(struct vm_area_struct *vma, unsigned long old_addr,
 			     unsigned long new_addr, unsigned long len);
+int hugetlb_expand_resv(struct vm_area_struct *vma, unsigned long old_len,
+			 unsigned long new_len);
 int copy_hugetlb_page_range(struct mm_struct *, struct mm_struct *, struct vm_area_struct *);
 int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *,
 			struct page **, struct vm_area_struct **,
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 6f5b56f..1f8e333 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2250,6 +2250,34 @@ int move_hugetlb_page_tables(
 	return len + old_addr - old_end;
 }
 
+int hugetlb_expand_resv(struct vm_area_struct *vma, unsigned long old_len,
+			 unsigned long new_len)
+{
+	struct hstate *h = hstate_vma(vma);
+	int ret = 0;
+
+	if (old_len >= new_len)
+		goto out;
+
+	if (vma->vm_flags & VM_MAYSHARE) {
+		struct inode *inode = vma->vm_file->f_mapping->host;
+		unsigned long from, to, vm_flags;
+
+		from = (vma->vm_pgoff >> huge_page_order(h)) +
+			(old_len >> huge_page_shift(h));
+		to   = (vma->vm_pgoff >> huge_page_order(h)) +
+			(new_len >> huge_page_shift(h));
+		vm_flags = vma->vm_flags;
+
+		ret = hugetlb_reserve_pages(inode, from, to, vma, vm_flags);
+	} else if (is_vma_resv_set(vma, HPAGE_RESV_OWNER)) {
+		long expand = (new_len - old_len) >> huge_page_shift(h);
+		ret = hugetlb_acct_memory(h, expand);
+	}
+out:
+	return ret;
+}
+
 void __unmap_hugepage_range(struct vm_area_struct *vma, unsigned long start,
 			    unsigned long end, struct page *ref_page)
 {
diff --git a/mm/mremap.c b/mm/mremap.c
index 9f6c903..010f93a 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -410,7 +410,7 @@ unsigned long do_mremap(unsigned long addr,
 	unsigned long flags, unsigned long new_addr)
 {
 	struct mm_struct *mm = current->mm;
-	struct vm_area_struct *vma;
+	struct vm_area_struct *vma = NULL;
 	unsigned long ret = -EINVAL;
 	unsigned long charged = 0;
 
@@ -525,6 +525,15 @@ unsigned long do_mremap(unsigned long addr,
 out:
 	if (ret & ~PAGE_MASK)
 		vm_unacct_memory(charged);
+	else if (vma && is_vm_hugetlb_page(vma)) {
+		unsigned long ret2;
+
+		ret2 = hugetlb_expand_resv(vma, old_len, new_len);
+		if (ret2) {
+			ret = ret2;
+			vm_unacct_memory(charged);
+		}
+	}
 	return ret;
 }
 

Re: [patch] hugetlb: add hugepage reservation upon mremap expansion.

[Hugh Dickins]

On Thu, 3 Nov 2011, Ken Chen wrote:

> hugetlb: add hugepage reservation upon mremap expansion
> 
> hugetlb page has a semantics that it reserves pages up front at the time
> of mmap.  We need to extend the same reservation scheme for the mremap
> expansion case.

If you do go this way, then I think there needs to be more to it.

hugetlbfs_file_mmap() has that line near the bottom where it updates
inode->i_size to cover the mapping: without doing something similar
in your hugetlbfs mremap, won't the enlarged mapping just give SIGBUS
on the enlargement (unless it happens to be already mapped elsewhere)?

But changing i_size in mmap and mremap is unusual, if not simply wrong.
The size of mmap or mremap is the size of a userspace mapping, which
is modified by mmap, mremap, munmap.  The size of the underlying object
is usually independent of that, and modified by ftruncate or write.

It was suggested a few years ago that it would be helpful if mremap of
a shared anonymous shmem object would change the size of the underlying
object, instead of just giving SIGBUS on the enlargement; but in the
end I played cautious, and made no change there.

Since hugetlbfs is already peculiar in setting i_size in its mmap,
I guess you would not be wrong to extend that peculiarity to its mremap.
But you will have some trouble with the locking: see "locking order of
mmap_sem and various FS" thread on LKML a couple of days ago -
I think you will need to fix taking i_mutex under mmap_sem somehow.

Hugh

> 
> Signed-off-by: Ken Chen <kenchen@google.com>
> 
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index c36d851..5d22933 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -28,6 +28,8 @@ int hugetlb_mempolicy_sysctl_handler(
>  
>  int move_hugetlb_page_tables(struct vm_area_struct *vma, unsigned long old_addr,
>  			     unsigned long new_addr, unsigned long len);
> +int hugetlb_expand_resv(struct vm_area_struct *vma, unsigned long old_len,
> +			 unsigned long new_len);
>  int copy_hugetlb_page_range(struct mm_struct *, struct mm_struct *, struct vm_area_struct *);
>  int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *,
>  			struct page **, struct vm_area_struct **,
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 6f5b56f..1f8e333 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2250,6 +2250,34 @@ int move_hugetlb_page_tables(
>  	return len + old_addr - old_end;
>  }
>  
> +int hugetlb_expand_resv(struct vm_area_struct *vma, unsigned long old_len,
> +			 unsigned long new_len)
> +{
> +	struct hstate *h = hstate_vma(vma);
> +	int ret = 0;
> +
> +	if (old_len >= new_len)
> +		goto out;
> +
> +	if (vma->vm_flags & VM_MAYSHARE) {
> +		struct inode *inode = vma->vm_file->f_mapping->host;
> +		unsigned long from, to, vm_flags;
> +
> +		from = (vma->vm_pgoff >> huge_page_order(h)) +
> +			(old_len >> huge_page_shift(h));
> +		to   = (vma->vm_pgoff >> huge_page_order(h)) +
> +			(new_len >> huge_page_shift(h));
> +		vm_flags = vma->vm_flags;
> +
> +		ret = hugetlb_reserve_pages(inode, from, to, vma, vm_flags);
> +	} else if (is_vma_resv_set(vma, HPAGE_RESV_OWNER)) {
> +		long expand = (new_len - old_len) >> huge_page_shift(h);
> +		ret = hugetlb_acct_memory(h, expand);
> +	}
> +out:
> +	return ret;
> +}
> +
>  void __unmap_hugepage_range(struct vm_area_struct *vma, unsigned long start,
>  			    unsigned long end, struct page *ref_page)
>  {
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 9f6c903..010f93a 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -410,7 +410,7 @@ unsigned long do_mremap(unsigned long addr,
>  	unsigned long flags, unsigned long new_addr)
>  {
>  	struct mm_struct *mm = current->mm;
> -	struct vm_area_struct *vma;
> +	struct vm_area_struct *vma = NULL;
>  	unsigned long ret = -EINVAL;
>  	unsigned long charged = 0;
>  
> @@ -525,6 +525,15 @@ unsigned long do_mremap(unsigned long addr,
>  out:
>  	if (ret & ~PAGE_MASK)
>  		vm_unacct_memory(charged);
> +	else if (vma && is_vm_hugetlb_page(vma)) {
> +		unsigned long ret2;
> +
> +		ret2 = hugetlb_expand_resv(vma, old_len, new_len);
> +		if (ret2) {
> +			ret = ret2;
> +			vm_unacct_memory(charged);
> +		}
> +	}
>  	return ret;
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [patch] hugetlb: add hugepage reservation upon mremap expansion.

[Ken Chen]

On Fri, Nov 4, 2011 at 3:13 PM, Hugh Dickins <hughd@google.com> wrote:
> But changing i_size in mmap and mremap is unusual, if not simply wrong.
> The size of mmap or mremap is the size of a userspace mapping, which
> is modified by mmap, mremap, munmap.  The size of the underlying object
> is usually independent of that, and modified by ftruncate or write.
>
> It was suggested a few years ago that it would be helpful if mremap of
> a shared anonymous shmem object would change the size of the underlying
> object, instead of just giving SIGBUS on the enlargement; but in the
> end I played cautious, and made no change there.

Indeed, that would be undesirable to modify i_size in mremap().  For small
page, the only mapping one can truly expand beyond file size via mremap()
is private anon.  All other cases it will SIGBUS at the time of fault. In
that respect, I think hugetlb should match the behavior of small page as
well.  I will add a size check for hugetlb mapping and disallow new size
to pass beyond i_size.

Then there are cases of taking partial region and perform an mremap
expansion: e.g.
  addr = mmap(.., SIZE, ..)
  mremap(addr, SIZE / 2, SIZE, ...);

Which for shared mapping, it is fine because everything is already
reserved.  For private mapping, a new set of expansion would be needed.
I will re-post with updated changes on this.

- Ken