Cannot use SHM_HUGETLB as a regular user

However, setting up hugetlb_shm_group with the right gid does not work! Looks like hugetlb uses mlock based rlimits which cause shmget with SHM_HUGETLB to fail with -ENOMEM. Setting up right rlimits for mlock through /etc/security/limits.conf works though (regardless of hugetlb_shm_group).
I understand most oracle users use this rlimit to use largepages. But why does this need to be based on mlock!? We do have shmmax and shmall to restrict this resource.
As I see it we have the following options to fix this inconsistency:

1. Do not depend on RLIMIT_MEMLOCK for hugetlb shm mappings. If a user has CAP_IPC_LOCK or if user belongs to /proc/sys/vm/hugetlb_shm_group, he should be able to use shm memory according to shmmax and shmall OR
2. Update the hugetlbpage documentation to mention the resource limit based limitation, and remove the useless /proc/sys/vm/hugetlb_shm_group sysctl
Which one is better? I am leaning towards 1. and have a patch ready for 1. but I might be missing some historical reason for using RLIMIT_MEMLOCK with SHM_HUGETLB.

Quoted 10 lines · Ravikiran G Thirumalai

...
As I see it we have the following options to fix this inconsistency:
1. Do not depend on RLIMIT_MEMLOCK for hugetlb shm mappings. If a user has CAP_IPC_LOCK or if user belongs to /proc/sys/vm/hugetlb_shm_group, he should be able to use shm memory according to shmmax and shmall OR
2. Update the hugetlbpage documentation to mention the resource limit based limitation, and remove the useless /proc/sys/vm/hugetlb_shm_group sysctl
Which one is better? I am leaning towards 1. and have a patch ready for 1. but I might be missing some historical reason for using RLIMIT_MEMLOCK with SHM_HUGETLB.

We should do (1) because the hugetlb_shm_group and CAP_IPC_LOCK bits should both continue to work as they did prior to RLIMIT_MEMLOCK -based management of hugetlb. Please make sure the new RLIMIT_MEMLOCK -based management still enables hugetlb shm when hugetlb_shm_group and CAP_IPC_LOCK don't apply.

Quoted 12 lines · Ravikiran G Thirumalai

Quoted 10 lines

...
As I see it we have the following options to fix this inconsistency:
1. Do not depend on RLIMIT_MEMLOCK for hugetlb shm mappings. If a user has CAP_IPC_LOCK or if user belongs to /proc/sys/vm/hugetlb_shm_group, he should be able to use shm memory according to shmmax and shmall OR
2. Update the hugetlbpage documentation to mention the resource limit based limitation, and remove the useless /proc/sys/vm/hugetlb_shm_group sysctl
Which one is better? I am leaning towards 1. and have a patch ready for 1. but I might be missing some historical reason for using RLIMIT_MEMLOCK with SHM_HUGETLB.

Quoted 6 lines

We should do (1) because the hugetlb_shm_group and CAP_IPC_LOCK bits should both continue to work as they did prior to RLIMIT_MEMLOCK -based management of hugetlb. Please make sure the new RLIMIT_MEMLOCK -based management still enables hugetlb shm when hugetlb_shm_group and CAP_IPC_LOCK don't apply.

OK, here's the patch.

Thanks,
Kiran

Fix hugetlb subsystem so that non root users belonging to hugetlb_shm_group can actually allocate hugetlb backed shm.

Currently non root users cannot even map one large page using SHM_HUGETLB when they belong to the gid in /proc/sys/vm/hugetlb_shm_group. This is because allocation size is verified against RLIMIT_MEMLOCK resource limit even if the user belongs to hugetlb_shm_group.

This patch
1. Fixes hugetlb subsystem so that users with CAP_IPC_LOCK and users belonging to hugetlb_shm_group don't need to be restricted with RLIMIT_MEMLOCK resource limits
2. If a user has sufficient memlock limit he can still allocate the hugetlb shm segment.

Signed-off-by Ravikiran Thirumalai <[email protected]>

---

 Documentation/vm/hugetlbpage.txt |   11 ++++++-----
 fs/hugetlbfs/inode.c             |   18 ++++++++++++------
 include/linux/mm.h               |    2 ++
 mm/mlock.c                       |   11 ++++++++---
 4 files changed, 28 insertions(+), 14 deletions(-)

Index: linux-2.6-tip/fs/hugetlbfs/inode.c
===================================================================

fs/hugetlbfs/inode.c +12 -6

@@ -943,8 +943,15 @@ static struct vfsmount *hugetlbfs_vfsmou
 static int can_do_hugetlb_shm(void)
 {
 	return likely(capable(CAP_IPC_LOCK) ||
-			in_group_p(sysctl_hugetlb_shm_group) ||
-			can_do_mlock());
+			in_group_p(sysctl_hugetlb_shm_group));
+}
+
+static void acct_huge_shm_lock(size_t size, struct user_struct *user)
+{
+	unsigned long pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	spin_lock(&shmlock_user_lock);
+	acct_shm_lock(pages, user);
+	spin_unlock(&shmlock_user_lock);
 }
 
 struct file *hugetlb_file_setup(const char *name, size_t size)

@@ -959,12 +966,11 @@ struct file *hugetlb_file_setup(const ch
 	if (!hugetlbfs_vfsmount)
 		return ERR_PTR(-ENOENT);
 
-	if (!can_do_hugetlb_shm())
+	if (can_do_hugetlb_shm())
+		acct_huge_shm_lock(size, user);
+	else if (!user_shm_lock(size, user))
 		return ERR_PTR(-EPERM);
 
-	if (!user_shm_lock(size, user))
-		return ERR_PTR(-ENOMEM);
-
 	root = hugetlbfs_vfsmount->mnt_root;
 	quick_string.name = name;
 	quick_string.len = strlen(quick_string.name);

Index: linux-2.6-tip/include/linux/mm.h
===================================================================

include/linux/mm.h +2 -0

@@ -737,8 +737,10 @@ extern unsigned long shmem_get_unmapped_
 #endif
 
 extern int can_do_mlock(void);
+extern void acct_shm_lock(unsigned long, struct user_struct *);
 extern int user_shm_lock(size_t, struct user_struct *);
 extern void user_shm_unlock(size_t, struct user_struct *);
+extern spinlock_t shmlock_user_lock;
 
 /*
  * Parameter block passed down to zap_pte_range in exceptional cases.

Index: linux-2.6-tip/mm/mlock.c
===================================================================

mm/mlock.c +8 -3

@@ -637,7 +637,13 @@ SYSCALL_DEFINE0(munlockall)
  * Objects with different lifetime than processes (SHM_LOCK and SHM_HUGETLB
  * shm segments) get accounted against the user_struct instead.
  */
-static DEFINE_SPINLOCK(shmlock_user_lock);
+DEFINE_SPINLOCK(shmlock_user_lock);
+
+void acct_shm_lock(unsigned long pages, struct user_struct *user)
+{
+	get_uid(user);
+	user->locked_shm += pages;
+}
 
 int user_shm_lock(size_t size, struct user_struct *user)
 {

@@ -653,8 +659,7 @@ int user_shm_lock(size_t size, struct us
 	if (!allowed &&
 	    locked + user->locked_shm > lock_limit && !capable(CAP_IPC_LOCK))
 		goto out;
-	get_uid(user);
-	user->locked_shm += locked;
+	acct_shm_lock(locked, user);
 	allowed = 1;
 out:
 	spin_unlock(&shmlock_user_lock);

Index: linux-2.6-tip/Documentation/vm/hugetlbpage.txt
===================================================================

Documentation/vm/hugetlbpage.txt +6 -6

@@ -147,11 +147,12 @@ used to change the file attributes on hu
 
 Also, it is important to note that no such mount command is required if the
 applications are going to use only shmat/shmget system calls.  Users who
-wish to use hugetlb page via shared memory segment should be a member of
-a supplementary group and system admin needs to configure that gid into
-/proc/sys/vm/hugetlb_shm_group.  It is possible for same or different
-applications to use any combination of mmaps and shm* calls, though the
-mount of filesystem will be required for using mmap calls.
+wish to use hugetlb page via shared memory segment should either have
+sufficient memlock resource limits or, they need to be a member of
+a supplementary group, and system admin needs to configure that gid into
+/proc/sys/vm/hugetlb_shm_group. It is possible for same or different
+applications to use any combination of mmaps and shm* calls, though
+the mount of filesystem will be required for using mmap calls.
 
 *******************************************************************
 
 
--

To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

(cc to Mel and Nishanth)

I think this requirement is reasonable. but I also hope Mel or Nishanth review this.

<<intentionally full quote>>

Quoted 42 lines · On Wed, Feb 04, 2009 at 05:11:21PM -0500, [email protected] wrote:

Quoted 12 lines · Ravikiran G Thirumalai

Quoted 10 lines

...
As I see it we have the following options to fix this inconsistency:
1. Do not depend on RLIMIT_MEMLOCK for hugetlb shm mappings. If a user has CAP_IPC_LOCK or if user belongs to /proc/sys/vm/hugetlb_shm_group, he should be able to use shm memory according to shmmax and shmall OR
2. Update the hugetlbpage documentation to mention the resource limit based limitation, and remove the useless /proc/sys/vm/hugetlb_shm_group sysctl
Which one is better? I am leaning towards 1. and have a patch ready for 1. but I might be missing some historical reason for using RLIMIT_MEMLOCK with SHM_HUGETLB.

Quoted 6 lines

We should do (1) because the hugetlb_shm_group and CAP_IPC_LOCK bits should both continue to work as they did prior to RLIMIT_MEMLOCK -based management of hugetlb. Please make sure the new RLIMIT_MEMLOCK -based management still enables hugetlb shm when hugetlb_shm_group and CAP_IPC_LOCK don't apply.

OK, here's the patch.

Thanks,
Kiran

Fix hugetlb subsystem so that non root users belonging to hugetlb_shm_group can actually allocate hugetlb backed shm.

Currently non root users cannot even map one large page using SHM_HUGETLB when they belong to the gid in /proc/sys/vm/hugetlb_shm_group. This is because allocation size is verified against RLIMIT_MEMLOCK resource limit even if the user belongs to hugetlb_shm_group.

This patch
1. Fixes hugetlb subsystem so that users with CAP_IPC_LOCK and users belonging to hugetlb_shm_group don't need to be restricted with RLIMIT_MEMLOCK resource limits
2. If a user has sufficient memlock limit he can still allocate the hugetlb shm segment.

Signed-off-by Ravikiran Thirumalai <[email protected]>

Quoted 12 lines · On Wed, Feb 04, 2009 at 05:11:21PM -0500, [email protected] wrote:

Quoted 10 lines · Ravikiran G Thirumalai

...
As I see it we have the following options to fix this inconsistency:
1. Do not depend on RLIMIT_MEMLOCK for hugetlb shm mappings. If a user has CAP_IPC_LOCK or if user belongs to /proc/sys/vm/hugetlb_shm_group, he should be able to use shm memory according to shmmax and shmall OR
2. Update the hugetlbpage documentation to mention the resource limit based limitation, and remove the useless /proc/sys/vm/hugetlb_shm_group sysctl
Which one is better? I am leaning towards 1. and have a patch ready for 1. but I might be missing some historical reason for using RLIMIT_MEMLOCK with SHM_HUGETLB.

Quoted 6 lines

We should do (1) because the hugetlb_shm_group and CAP_IPC_LOCK bits should both continue to work as they did prior to RLIMIT_MEMLOCK -based management of hugetlb. Please make sure the new RLIMIT_MEMLOCK -based management still enables hugetlb shm when hugetlb_shm_group and CAP_IPC_LOCK don't apply.

OK, here's the patch.

Thanks,
Kiran

Fix hugetlb subsystem so that non root users belonging to hugetlb_shm_group can actually allocate hugetlb backed shm.

Currently non root users cannot even map one large page using SHM_HUGETLB when they belong to the gid in /proc/sys/vm/hugetlb_shm_group. This is because allocation size is verified against RLIMIT_MEMLOCK resource limit even if the user belongs to hugetlb_shm_group.

This patch
1. Fixes hugetlb subsystem so that users with CAP_IPC_LOCK and users belonging to hugetlb_shm_group don't need to be restricted with RLIMIT_MEMLOCK resource limits
2. If a user has sufficient memlock limit he can still allocate the hugetlb shm segment.

Signed-off-by Ravikiran Thirumalai <[email protected]>

---

 Documentation/vm/hugetlbpage.txt |   11 ++++++-----
 fs/hugetlbfs/inode.c             |   18 ++++++++++++------
 include/linux/mm.h               |    2 ++
 mm/mlock.c                       |   11 ++++++++---
 4 files changed, 28 insertions(+), 14 deletions(-)

Index: linux-2.6-tip/fs/hugetlbfs/inode.c
===================================================================

fs/hugetlbfs/inode.c +0 -2

@@ -943,8 +943,15 @@ static struct vfsmount *hugetlbfs_vfsmou
 static int can_do_hugetlb_shm(void)
 {
 	return likely(capable(CAP_IPC_LOCK) ||
-			in_group_p(sysctl_hugetlb_shm_group) ||
-			can_do_mlock());

fs/hugetlbfs/inode.c +0 -3

-	if (!user_shm_lock(size, user))
-		return ERR_PTR(-ENOMEM);
-
 	root = hugetlbfs_vfsmount->mnt_root;
 	quick_string.name = name;
 	quick_string.len = strlen(quick_string.name);

Index: linux-2.6-tip/include/linux/mm.h
===================================================================

include/linux/mm.h +2 -0

@@ -737,8 +737,10 @@ extern unsigned long shmem_get_unmapped_
 #endif
 
 extern int can_do_mlock(void);
+extern void acct_shm_lock(unsigned long, struct user_struct *);
 extern int user_shm_lock(size_t, struct user_struct *);
 extern void user_shm_unlock(size_t, struct user_struct *);
+extern spinlock_t shmlock_user_lock;
 
 /*
  * Parameter block passed down to zap_pte_range in exceptional cases.

Index: linux-2.6-tip/mm/mlock.c
===================================================================

mm/mlock.c +8 -3

@@ -637,7 +637,13 @@ SYSCALL_DEFINE0(munlockall)
  * Objects with different lifetime than processes (SHM_LOCK and SHM_HUGETLB
  * shm segments) get accounted against the user_struct instead.
  */
-static DEFINE_SPINLOCK(shmlock_user_lock);
+DEFINE_SPINLOCK(shmlock_user_lock);
+
+void acct_shm_lock(unsigned long pages, struct user_struct *user)
+{
+	get_uid(user);
+	user->locked_shm += pages;
+}
 
 int user_shm_lock(size_t size, struct user_struct *user)
 {

@@ -653,8 +659,7 @@ int user_shm_lock(size_t size, struct us
 	if (!allowed &&
 	    locked + user->locked_shm > lock_limit && !capable(CAP_IPC_LOCK))
 		goto out;
-	get_uid(user);
-	user->locked_shm += locked;
+	acct_shm_lock(locked, user);
 	allowed = 1;
 out:
 	spin_unlock(&shmlock_user_lock);

Index: linux-2.6-tip/Documentation/vm/hugetlbpage.txt
===================================================================

Documentation/vm/hugetlbpage.txt +0 -2

@@ -147,11 +147,12 @@ used to change the file attributes on hu
 
 Also, it is important to note that no such mount command is required if the
 applications are going to use only shmat/shmget system calls.  Users who
-wish to use hugetlb page via shared memory segment should be a member of
-a supplementary group and system admin needs to configure that gid into

On Wed, Feb 04, 2009 at 04:41:57PM -0800, Ravikiran G Thirumalai wrote:

========

Quoted 8 lines

Fix hugetlb subsystem so that non root users belonging to hugetlb_shm_group can actually allocate hugetlb backed shm.

Currently non root users cannot even map one large page using SHM_HUGETLB when they belong to the gid in /proc/sys/vm/hugetlb_shm_group. This is because allocation size is verified against RLIMIT_MEMLOCK resource limit even if the user belongs to hugetlb_shm_group.

Quoted 7 lines

This patch
1. Fixes hugetlb subsystem so that users with CAP_IPC_LOCK and users belonging to hugetlb_shm_group don't need to be restricted with RLIMIT_MEMLOCK resource limits
2. If a user has sufficient memlock limit he can still allocate the hugetlb shm segment.

Point 1 I'm happy with, point 2 less so. It alters the semantics of the locked rlimit beyond what is necessary to fix the problem - i.e. a user in the group should be allowed to use hugepages with shmget(). Minimally, there should be two separate patches.

Quoted 29 lines

Signed-off-by Ravikiran Thirumalai <[email protected]>

---

 Documentation/vm/hugetlbpage.txt |   11 ++++++-----
 fs/hugetlbfs/inode.c             |   18 ++++++++++++------
 include/linux/mm.h               |    2 ++
 mm/mlock.c                       |   11 ++++++++---
 4 files changed, 28 insertions(+), 14 deletions(-)

Index: linux-2.6-tip/fs/hugetlbfs/inode.c
===================================================================

fs/hugetlbfs/inode.c +8 -2

@@ -943,8 +943,15 @@ static struct vfsmount *hugetlbfs_vfsmou
 static int can_do_hugetlb_shm(void)
 {
 	return likely(capable(CAP_IPC_LOCK) ||
-			in_group_p(sysctl_hugetlb_shm_group) ||
-			can_do_mlock());
+			in_group_p(sysctl_hugetlb_shm_group));
+}
+
+static void acct_huge_shm_lock(size_t size, struct user_struct *user)
+{
+	unsigned long pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	spin_lock(&shmlock_user_lock);
+	acct_shm_lock(pages, user);

fs/hugetlbfs/inode.c static struct vfsmount *hugetlbfs_vfsmou · line 955

Quoted 2 lines

fs/hugetlbfs/inode.c +1 -0

+	spin_unlock(&shmlock_user_lock);
 }

This should be split into another patch (i.e. three in all). The first patch allows users in thh shm_group to use huge pages. The second that accounts for locked_shm properly. The third allows users with a high enough locked rlimit to use shmget() with hugepages. However, my feeling right now would be to ack 1, re-reread 2 and nak 3.

Quoted 6 lines

This should be split into another patch (i.e. three in all). The first patch allows users in thh shm_group to use huge pages. The second that accounts for locked_shm properly. The third allows users with a high enough locked rlimit to use shmget() with hugepages. However, my feeling right now would be to ack 1, re-reread 2 and nak 3.

I totally agree. In fact yesterday I was thinking of resending this patch to not account for shm memory when a user is not validated against rlimits (when he has CAP_IPC_LOCK or if he belongs to the sysctl_hugetlb_shm_group).

As I see it there must be two parts:
1. Free ticket to CAP_IPC_LOCK and users belonging to sysctl_hugetlb_shm_group
2. Patch to have users not having CAP_IPC_LOCK or sysctl_hugetlb_shm_group to check against memlock rlimits, and account it. Also mark this deprecated in feature-removal-schedule.txt
Would this be OK?

Quoted 4 lines · Ravikiran G Thirumalai

I totally agree. In fact yesterday I was thinking of resending this patch to not account for shm memory when a user is not validated against rlimits (when he has CAP_IPC_LOCK or if he belongs to the sysctl_hugetlb_shm_group).

Quoted 8 lines

As I see it there must be two parts:
1. Free ticket to CAP_IPC_LOCK and users belonging to sysctl_hugetlb_shm_group
2. Patch to have users not having CAP_IPC_LOCK or sysctl_hugetlb_shm_group to check against memlock rlimits, and account it. Also mark this deprecated in feature-removal-schedule.txt
Would this be OK?

This is the ideal scenario, except I thought the rlimit was destined to replace the other methods, not vice-versa. I don't really mind going this way, but maybe we should check in with the rlimit authors.

Thanks for your comments Mel.

Quoted 19 lines · Mel Gorman

On Wed, Feb 04, 2009 at 04:41:57PM -0800, Ravikiran G Thirumalai wrote:

========

Quoted 8 lines

Fix hugetlb subsystem so that non root users belonging to hugetlb_shm_group can actually allocate hugetlb backed shm.

Currently non root users cannot even map one large page using SHM_HUGETLB when they belong to the gid in /proc/sys/vm/hugetlb_shm_group. This is because allocation size is verified against RLIMIT_MEMLOCK resource limit even if the user belongs to hugetlb_shm_group.

Quoted 7 lines

This patch
1. Fixes hugetlb subsystem so that users with CAP_IPC_LOCK and users belonging to hugetlb_shm_group don't need to be restricted with RLIMIT_MEMLOCK resource limits
2. If a user has sufficient memlock limit he can still allocate the hugetlb shm segment.

Quoted 4 lines

Point 1 I'm happy with, point 2 less so. It alters the semantics of the locked rlimit beyond what is necessary to fix the problem - i.e. a user in the group should be allowed to use hugepages with shmget(). Minimally, there should be two separate patches.

I see your point, and I was initially leaning towards 1. only -- that is not validate against memlock rlimit at all. But, I kinda understand Bill's comments about still honoring the rlimit because this is the only way to map SHM_HUGETLB currently, and seems like all oracle users currently do that.

This is a compatibility issue and sysadmins will have to change from using /etc/security/limits.conf to a gid based sysctl in /etc/sysctl.conf (both based on distros) to let users use hugetlb backed shm. I agree this still keeps some inconsistency around, so how about letting people still use rlimit based checks, but marking it deprecated by adding this to feature-removal-schedule.txt?

Quoted 29 lines

Quoted 29 lines

Signed-off-by Ravikiran Thirumalai <[email protected]>

---

 Documentation/vm/hugetlbpage.txt |   11 ++++++-----
 fs/hugetlbfs/inode.c             |   18 ++++++++++++------
 include/linux/mm.h               |    2 ++
 mm/mlock.c                       |   11 ++++++++---
 4 files changed, 28 insertions(+), 14 deletions(-)

Index: linux-2.6-tip/fs/hugetlbfs/inode.c
===================================================================

fs/hugetlbfs/inode.c +8 -2

@@ -943,8 +943,15 @@ static struct vfsmount *hugetlbfs_vfsmou
 static int can_do_hugetlb_shm(void)
 {
 	return likely(capable(CAP_IPC_LOCK) ||
-			in_group_p(sysctl_hugetlb_shm_group) ||
-			can_do_mlock());
+			in_group_p(sysctl_hugetlb_shm_group));
+}
+
+static void acct_huge_shm_lock(size_t size, struct user_struct *user)
+{
+	unsigned long pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	spin_lock(&shmlock_user_lock);
+	acct_shm_lock(pages, user);

Quoted 8 lines

fs/hugetlbfs/inode.c static struct vfsmount *hugetlbfs_vfsmou · line 955

Quoted 2 lines

fs/hugetlbfs/inode.c +1 -0

+	spin_unlock(&shmlock_user_lock);
 }

This should be split into another patch (i.e. three in all). The first patch allows users in thh shm_group to use huge pages. The second that accounts for locked_shm properly. The third allows users with a high enough locked rlimit to use shmget() with hugepages. However, my feeling right now would be to ack 1, re-reread 2 and nak 3.

I totally agree. In fact yesterday I was thinking of resending this patch to not account for shm memory when a user is not validated against rlimits (when he has CAP_IPC_LOCK or if he belongs to the sysctl_hugetlb_shm_group).

As I see it there must be two parts:
1. Free ticket to CAP_IPC_LOCK and users belonging to sysctl_hugetlb_shm_group
2. Patch to have users not having CAP_IPC_LOCK or sysctl_hugetlb_shm_group to check against memlock rlimits, and account it. Also mark this deprecated in feature-removal-schedule.txt

Would this be OK?