Re: [PATCH] HUGETLBFS: Align memory request to multiple of huge pagesize to avoid underallocating.

[Steven Truelove]

On 27/02/2012 6:42 PM, Andrew Morton wrote:
> On Mon, 27 Feb 2012 09:09:28 -0500
> Steven Truelove<steven.truelove@xxxxxxxxxxx>  wrote:
>
> > When calling shmget with SHM_HUGETLB, shmget aligns the request size to PAGE_SIZE, but this is not sufficient.  Modified hugetlb_file_setup to align requests to the huge page size.
> >
> > Signed-off-by: Steven Truelove<steven.truelove@xxxxxxxxxxx>
> > ---
> >   fs/hugetlbfs/inode.c |    9 ++++++---
> >   1 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> > index 1e85a7a..6c23f09 100644
> > --- a/fs/hugetlbfs/inode.c
> > +++ b/fs/hugetlbfs/inode.c
> > @@ -938,6 +938,8 @@ struct file *hugetlb_file_setup(const char *name, size_t size,
> >   	struct path path;
> >   	struct dentry *root;
> >   	struct qstr quick_string;
> > +	struct hstate *hstate;
> > +	int num_pages;
> >
> >   	*user = NULL;
> >   	if (!hugetlbfs_vfsmount)
> > @@ -967,10 +969,11 @@ struct file *hugetlb_file_setup(const char *name, size_t size,
> >   	if (!inode)
> >   		goto out_dentry;
> >
> > +	hstate = hstate_inode(inode);
> > +	num_pages = (size + huge_page_size(hstate) - 1)>>
> > +			huge_page_shift(hstate);
> >   	error = -ENOMEM;
> > -	if (hugetlb_reserve_pages(inode, 0,
> > -			size>>  huge_page_shift(hstate_inode(inode)), NULL,
> > -			acctflag))
> > +	if (hugetlb_reserve_pages(inode, 0, num_pages, NULL, acctflag))
> >   		goto out_inode;
> >
> >   	d_instantiate(path.dentry, inode);
> A few things...
>
> - sys_mmap_pgoff() does the rounding up prior to calling
>    hugetlb_file_setup().  ipc/shm.c:newseg() does not.
>
>    We should be consistent here: do it in the caller or the callee,
>    not both (or neither!).  I guess doing it in the callee would be
>    best.
>
> - The above code could/should have used ALIGN().  Or round_up(): the
>    difference presently escapes me, even though it was so obvious that
>    we left all these things undocumented.
>
> - What's the point in aligning the length if we don't also look at
>    the start address?  If that isn't a multiple of huge_page_size(), we
>    will need an additional page.

Since mmap has an address to check and shmget does not, if the address 
is going to be checked it will need to be in the caller.  If you like, I 
will leave the size check in hugetlb_file_setup() and remove the size 
check from mmap_pgoff, but replace it with a check of the address.  That 
will centralize the common check (size of buffer), and let mmap_pgoff 
check the part that is unique to it.  Patch shortly.

Steven Truelove

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/