Re: [PATCH 1/3] hugetlbfs: Allow the creation of files suitablefor MAP_PRIVATE on the vfs internal mount

From: Mel Gorman
Date: Thu Aug 27 2009 - 10:18:40 EST

On Wed, Aug 26, 2009 at 11:44:51AM +0100, Eric B Munson wrote:
> There are two means of creating mappings backed by huge pages:
> 1. mmap() a file created on hugetlbfs
> 2. Use shm which creates a file on an internal mount which essentially
> maps it MAP_SHARED
> The internal mount is only used for shared mappings but there is very
> little that stops it being used for private mappings. This patch extends
> hugetlbfs_file_setup() to deal with the creation of files that will be
> mapped MAP_PRIVATE on the internal hugetlbfs mount. This extended API is
> used in a subsequent patch to implement the MAP_HUGETLB mmap() flag.

Hi Eric,

I ran these patches through a series of small tests and I have just one
concern with the changes made to can_do_hugetlb_shm(). If that returns false
because of MAP_HUGETLB, we then proceed to call user_shm_lock(). I think your
intention might have been something like the following patch on top of yours?

For what it's worth, once this was applied, I didn't spot any other
problems, run-time or otherwise.

hugetlbfs: Do not call user_shm_lock() for MAP_HUGETLB

The patch
alters can_do_hugetlb_shm() to check if a file is being created for shared
memory or mmap(). If this returns false, we then unconditionally call
user_shm_lock() triggering a warning. This block should never be entered
for MAP_HUGETLB. This patch partially reverts the problem and fixes the check.

This patch should be considered a fix to

Signed-off-by: Mel Gorman <mel@xxxxxxxxx>
fs/hugetlbfs/inode.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 49d2bf9..c944cc1 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -910,15 +910,9 @@ static struct file_system_type hugetlbfs_fs_type = {

static struct vfsmount *hugetlbfs_vfsmount;

-static int can_do_hugetlb_shm(int creat_flags)
+static int can_do_hugetlb_shm(void)
- if (creat_flags != HUGETLB_SHMFS_INODE)
- return 0;
- if (capable(CAP_IPC_LOCK))
- return 1;
- if (in_group_p(sysctl_hugetlb_shm_group))
- return 1;
- return 0;
+ return capable(CAP_IPC_LOCK) || in_group_p(sysctl_hugetlb_shm_group);

struct file *hugetlb_file_setup(const char *name, size_t size, int acctflag,
@@ -934,7 +928,7 @@ struct file *hugetlb_file_setup(const char *name, size_t size, int acctflag,
if (!hugetlbfs_vfsmount)
return ERR_PTR(-ENOENT);

- if (!can_do_hugetlb_shm(creat_flags)) {
+ if (creat_flags == HUGETLB_SHMFS_INODE && !can_do_hugetlb_shm()) {
*user = current_user();
if (user_shm_lock(size, *user)) {

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at