Re: [PATCH backport to 3.4] hugetlbfs: fix mmap failure in unaligned size request

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 15, 2013 at 09:43:57AM +0800, Jianguo Wu wrote:
> On 2013/5/15 2:04, Luis Henriques wrote:
> 
> > Hi Jianguo,
> > 
> > On Fri, May 10, 2013 at 02:08:19PM +0800, Jianguo Wu wrote:
> >> Hi Greg,
> >> I rebase "[PATCH] hugetlbfs: fix mmap failure in unaligned size request" on 3.4,
> >> could you please apply to 3.4-stable.
> >>
> >> Chagelog:
> >>  - remove hstate_sizelog(), as 3.4 only support default hugepagesize for
> >>    MAP_HUGETLB/SHM_HUGETLB.
> > 
> > Do you think this backport could also be applied to the 3.5 kernel?
> > The original commit can't be applied, but I'm not sure that picking
> > your backport is the right thing to do.
> > 
> 
> Hi Luis,
> 
> I think it can be applied to 3.5.

Great, thanks.  I'll queue it for 3.5 kernel.

Cheers,
--
Luis

> 
> Thanks,
> Jianguo Wu.
> 
> > Cheers,
> > --
> > Luis
> > 
> >>
> >>
> >> From: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
> >>
> >> commit af73e4d9506d3b797509f3c030e7dcd554f7d9c4 upstream.
> >>
> >> The current kernel returns -EINVAL unless a given mmap length is
> >> "almost" hugepage aligned.  This is because in sys_mmap_pgoff() the
> >> given length is passed to vm_mmap_pgoff() as it is without being aligned
> >> with hugepage boundary.
> >>
> >> This is a regression introduced in commit 40716e29243d ("hugetlbfs: fix
> >> alignment of huge page requests"), where alignment code is pushed into
> >> hugetlb_file_setup() and the variable len in caller side is not changed.
> >>
> >> To fix this, this patch partially reverts that commit, and adds
> >> alignment code in caller side.  And it also introduces hstate_sizelog()
> >> in order to get proper hstate to specified hugepage size.
> >>
> >> Addresses https://bugzilla.kernel.org/show_bug.cgi?id=56881
> >>
> >> [akpm@xxxxxxxxxxxxxxxxxxxx: fix warning when CONFIG_HUGETLB_PAGE=n]
> >> Signed-off-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
> >> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> >> Reported-by: <iceman_dvd@xxxxxxxxx>
> >> Cc: Steven Truelove <steven.truelove@xxxxxxxxxxx>
> >> Cc: Jianguo Wu <wujianguo@xxxxxxxxxx>
> >> Cc: Hugh Dickins <hughd@xxxxxxxxxx>
> >> Cc: <stable@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> >> Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> >> Signed-off-by: Jianguo Wu <wujianguo@xxxxxxxxxx>
> >> ---
> >>  fs/hugetlbfs/inode.c    |   20 ++++++++++----------
> >>  include/linux/hugetlb.h |    9 ++++-----
> >>  ipc/shm.c               |    4 +++-
> >>  mm/mmap.c               |    6 +++++-
> >>  4 files changed, 22 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> >> index 001ef01..36ad5b4 100644
> >> --- a/fs/hugetlbfs/inode.c
> >> +++ b/fs/hugetlbfs/inode.c
> >> @@ -927,9 +927,13 @@ static int can_do_hugetlb_shm(void)
> >>  	return capable(CAP_IPC_LOCK) || in_group_p(sysctl_hugetlb_shm_group);
> >>  }
> >>  
> >> -struct file *hugetlb_file_setup(const char *name, unsigned long addr,
> >> -				size_t size, vm_flags_t acctflag,
> >> -				struct user_struct **user, int creat_flags)
> >> +/*
> >> + * Note that size should be aligned to proper hugepage size in caller side,
> >> + * otherwise hugetlb_reserve_pages reserves one less hugepages than intended.
> >> + */
> >> +struct file *hugetlb_file_setup(const char *name, size_t size,
> >> +				vm_flags_t acctflag, struct user_struct **user,
> >> +				int creat_flags)
> >>  {
> >>  	int error = -ENOMEM;
> >>  	struct file *file;
> >> @@ -937,8 +941,6 @@ struct file *hugetlb_file_setup(const char *name, unsigned long addr,
> >>  	struct path path;
> >>  	struct dentry *root;
> >>  	struct qstr quick_string;
> >> -	struct hstate *hstate;
> >> -	unsigned long num_pages;
> >>  
> >>  	*user = NULL;
> >>  	if (!hugetlbfs_vfsmount)
> >> @@ -972,12 +974,10 @@ struct file *hugetlb_file_setup(const char *name, unsigned long addr,
> >>  	if (!inode)
> >>  		goto out_dentry;
> >>  
> >> -	hstate = hstate_inode(inode);
> >> -	size += addr & ~huge_page_mask(hstate);
> >> -	num_pages = ALIGN(size, huge_page_size(hstate)) >>
> >> -			huge_page_shift(hstate);
> >>  	error = -ENOMEM;
> >> -	if (hugetlb_reserve_pages(inode, 0, num_pages, NULL, acctflag))
> >> +	if (hugetlb_reserve_pages(inode, 0,
> >> +			size >> huge_page_shift(hstate_inode(inode)), NULL,
> >> +			acctflag))
> >>  		goto out_inode;
> >>  
> >>  	d_instantiate(path.dentry, inode);
> >> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> >> index 000837e..2c36a71 100644
> >> --- a/include/linux/hugetlb.h
> >> +++ b/include/linux/hugetlb.h
> >> @@ -152,8 +152,7 @@ static inline struct hugetlbfs_sb_info *HUGETLBFS_SB(struct super_block *sb)
> >>  
> >>  extern const struct file_operations hugetlbfs_file_operations;
> >>  extern const struct vm_operations_struct hugetlb_vm_ops;
> >> -struct file *hugetlb_file_setup(const char *name, unsigned long addr,
> >> -				size_t size, vm_flags_t acct,
> >> +struct file *hugetlb_file_setup(const char *name, size_t size, vm_flags_t acct,
> >>  				struct user_struct **user, int creat_flags);
> >>  
> >>  static inline int is_file_hugepages(struct file *file)
> >> @@ -170,8 +169,8 @@ static inline int is_file_hugepages(struct file *file)
> >>  
> >>  #define is_file_hugepages(file)			0
> >>  static inline struct file *
> >> -hugetlb_file_setup(const char *name, unsigned long addr, size_t size,
> >> -		vm_flags_t acctflag, struct user_struct **user, int creat_flags)
> >> +hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
> >> +		struct user_struct **user, int creat_flags)
> >>  {
> >>  	return ERR_PTR(-ENOSYS);
> >>  }
> >> @@ -294,7 +293,7 @@ static inline unsigned hstate_index_to_shift(unsigned index)
> >>  	return hstates[index].order + PAGE_SHIFT;
> >>  }
> >>  
> >> -#else
> >> +#else	/* CONFIG_HUGETLB_PAGE */
> >>  struct hstate {};
> >>  #define alloc_huge_page_node(h, nid) NULL
> >>  #define alloc_bootmem_huge_page(h) NULL
> >> diff --git a/ipc/shm.c b/ipc/shm.c
> >> index 85d81b4..a02ef57 100644
> >> --- a/ipc/shm.c
> >> +++ b/ipc/shm.c
> >> @@ -479,10 +479,12 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> >>  
> >>  	sprintf (name, "SYSV%08x", key);
> >>  	if (shmflg & SHM_HUGETLB) {
> >> +		size_t hugesize = ALIGN(size, huge_page_size(&default_hstate));
> >> +
> >>  		/* hugetlb_file_setup applies strict accounting */
> >>  		if (shmflg & SHM_NORESERVE)
> >>  			acctflag = VM_NORESERVE;
> >> -		file = hugetlb_file_setup(name, 0, size, acctflag,
> >> +		file = hugetlb_file_setup(name, hugesize, acctflag,
> >>  					&shp->mlock_user, HUGETLB_SHMFS_INODE);
> >>  	} else {
> >>  		/*
> >> diff --git a/mm/mmap.c b/mm/mmap.c
> >> index 3635d47..ed884dd 100644
> >> --- a/mm/mmap.c
> >> +++ b/mm/mmap.c
> >> @@ -1130,15 +1130,19 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
> >>  		file = fget(fd);
> >>  		if (!file)
> >>  			goto out;
> >> +		if (is_file_hugepages(file))
> >> +			len = ALIGN(len, huge_page_size(hstate_file(file)));
> >>  	} else if (flags & MAP_HUGETLB) {
> >>  		struct user_struct *user = NULL;
> >> +
> >> +		len = ALIGN(len, huge_page_size(&default_hstate));
> >>  		/*
> >>  		 * VM_NORESERVE is used because the reservations will be
> >>  		 * taken when vm_ops->mmap() is called
> >>  		 * A dummy user value is used because we are not locking
> >>  		 * memory so no accounting is necessary
> >>  		 */
> >> -		file = hugetlb_file_setup(HUGETLB_ANON_FILE, addr, len,
> >> +		file = hugetlb_file_setup(HUGETLB_ANON_FILE, len,
> >>  						VM_NORESERVE, &user,
> >>  						HUGETLB_ANONHUGE_INODE);
> >>  		if (IS_ERR(file))
> >> -- 
> >> 1.7.6.1
> >>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe stable" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > .
> > 
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]