* [PATCH] HUGETLBFS: Align memory request to multiple of huge page size to avoid underallocating.
@ 2012-02-27 14:09 Steven Truelove
2012-02-27 23:42 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Steven Truelove @ 2012-02-27 14:09 UTC (permalink / raw)
To: wli, akpm; +Cc: linux-kernel, Steven Truelove
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@utoronto.ca>
---
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);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] HUGETLBFS: Align memory request to multiple of huge page size to avoid underallocating.
2012-02-27 14:09 [PATCH] HUGETLBFS: Align memory request to multiple of huge page size to avoid underallocating Steven Truelove
@ 2012-02-27 23:42 ` Andrew Morton
2012-02-28 2:55 ` Steven Truelove
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2012-02-27 23:42 UTC (permalink / raw)
To: Steven Truelove; +Cc: wli, linux-kernel, linux-mm
On Mon, 27 Feb 2012 09:09:28 -0500
Steven Truelove <steven.truelove@utoronto.ca> 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@utoronto.ca>
> ---
> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HUGETLBFS: Align memory request to multiple of huge page size to avoid underallocating.
2012-02-27 23:42 ` Andrew Morton
@ 2012-02-28 2:55 ` Steven Truelove
2012-02-28 3:04 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Steven Truelove @ 2012-02-28 2:55 UTC (permalink / raw)
To: Andrew Morton; +Cc: wli, linux-kernel, linux-mm
On 27/02/2012 6:42 PM, Andrew Morton wrote:
> On Mon, 27 Feb 2012 09:09:28 -0500
> Steven Truelove<steven.truelove@utoronto.ca> 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@utoronto.ca>
>> ---
>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HUGETLBFS: Align memory request to multiple of huge page size to avoid underallocating.
2012-02-28 2:55 ` Steven Truelove
@ 2012-02-28 3:04 ` Andrew Morton
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2012-02-28 3:04 UTC (permalink / raw)
To: Steven Truelove; +Cc: wli, linux-kernel, linux-mm
On Mon, 27 Feb 2012 21:55:17 -0500 Steven Truelove <steven.truelove@utoronto.ca> wrote:
> > 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.
Or pass a value of 0 from shmget.
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-02-28 3:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-27 14:09 [PATCH] HUGETLBFS: Align memory request to multiple of huge page size to avoid underallocating Steven Truelove
2012-02-27 23:42 ` Andrew Morton
2012-02-28 2:55 ` Steven Truelove
2012-02-28 3:04 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).