- support inheritance of mlocks across fork/exec

PATCH/RFC - support inheritance of mlocks across fork/exec

Against; 2.6.28-rc5-mmotm-081121

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):
MCL_CURRENT|MCL_INHERIT - inherit memory locks across fork() MCL_FUTURE|MCL_INHERIT - inherit "MCL_FUTURE" semantics across fork() and exec().
MCL_RECURSIVE - inherit across future generations.

In support of a "lock prefix command"--e.g., mlock <cmd> <args> ...

PATCH/RFC - support inheritance of mlocks across fork/exec

Signed-off-by Lee Schermerhorn <[email protected]>

PATCH/RFC - support inheritance of mlocks across fork/exec

Against; 2.6.28-rc5-mmotm-081121

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):
MCL_CURRENT|MCL_INHERIT - inherit memory locks across fork() MCL_FUTURE|MCL_INHERIT - inherit "MCL_FUTURE" semantics across fork() and exec().
MCL_RECURSIVE - inherit across future generations.

In support of a "lock prefix command"--e.g., mlock <cmd> <args> ...

mm/mlock.c +1 -1

@@ -600,9 +603,15 @@ asmlinkage long sys_mlockall(int flags)
 	unsigned long lock_limit;
 	int ret = -EINVAL;
 
-	if (!flags || (flags & ~(MCL_CURRENT | MCL_FUTURE)))
+	if (!(flags & (MCL_CURRENT | MCL_FUTURE)))
 		goto out;

Against; 2.6.28-rc7-mmotm-081203

V02: rework vetting of flags argument as suggested by
Kosaki Motohiro.
enhance description as requested by Andrew Morton.

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):

MCL_CURRENT[|MCL_FUTURE]|MCL_INHERIT - inherit memory locks
[vmas' VM_LOCKED flags] across fork(), and inherit MCL_FUTURE behavior [mm's def_flags] across fork() and exec(). Behaves as if child and/or new task called mlockall(MCL_CURRENT|MCL_FUTURE) as first instruction.

MCL_RECURSIVE - inherit MCL_CURRENT|MCL_FUTURE|MCL_INHERIT
[vmas' VM_LOCKED flags for fork() and mm's def_flags and mcl_inherit across fork() and exec()] for all future generations of calling process's descendants. Behaves as if child and/or new task called mlockall(MCL_CURRENT|MCL_FUTURE|MCL_INHERIT|MCL_RECURSIVE) as the first instruction.

In support of a "lock prefix command"--e.g., mlock <cmd> <args> ... Analogous to taskset(1) for cpu affinity or numactl(8) for numa memory policy.

Together with patches to keep mlocked pages off the LRU, this will allow users/admins to lock down applications without modifying them, if their RLIMIT_MEMLOCK is sufficiently large, keeping their pages off the LRU and out of consideration for reclaim.

Potentially useful, as well, in real-time environments to force prefaulting and residency for applications that don't mlock themselves.

Jeff Sharkey at Montana State developed a similar patch for Linux [link no longer accessible], but apparently he never submitted the patch.

I submitted an earlier version of this patch around a year ago. I resurrected it to test the unevictable lru/mlocked pages patches-- e.g., by "mlock -r make -j<N*nr_cpus> all". This did shake out a few races and vmstat accounting bugs, but NOT something I'd recommend as general practice--for kernel builds, that is.

----

Define MCL_INHERIT, MCL_RECURSIVE in <asm-*/mman.h>.
+ x86 and ia64 versions included.
+ other arch can/will be created, if this patch deemed merge-worthy.

Similarly, I'll provide kernel man page update if/when needed.

Example "lock prefix command" in Documentation/vm/mlock.c

...

mm/mlock.c +0 -0

@@ -573,15 +573,18 @@ asmlinkage long sys_munlock(unsigned lon
 static int do_mlockall(int flags)

...

kernel/fork.c +6 -2

@@ -278,7 +278,8 @@ static int dup_mmap(struct mm_struct *mm
 	 */
 	down_write_nested(&mm->mmap_sem, SINGLE_DEPTH_NESTING);
 
-	mm->locked_vm = 0;
+	if (!mm->mcl_inherit)
+		mm->locked_vm = 0;
 	mm->mmap = NULL;
 	mm->mmap_cache = NULL;
 	mm->free_area_cache = oldmm->mmap_base;

@@ -316,7 +317,8 @@ static int dup_mmap(struct mm_struct *mm
 		if (IS_ERR(pol))
 			goto fail_nomem_policy;
 		vma_set_policy(tmp, pol);
-		tmp->vm_flags &= ~VM_LOCKED;
+		if (!mm->mcl_inherit)
+			tmp->vm_flags &= ~VM_LOCKED;
 		tmp->vm_mm = mm;
 		tmp->vm_next = NULL;
 		anon_vma_link(tmp);

@@ -406,6 +408,8 @@ __cacheline_aligned_in_smp DEFINE_SPINLO
 
 static struct mm_struct * mm_init(struct mm_struct * mm, struct task_struct *p)
 {
+	unsigned long def_flags = 0;
+
 	atomic_set(&mm->mm_users, 1);
 	atomic_set(&mm->mm_count, 1);
 	init_rwsem(&mm->mmap_sem);

kernel/fork.c +1 -1

 	if (likely(!mm_alloc_pgd(mm))) {
-		mm->def_flags = 0;
+		mm->def_flags = def_flags;
 		mmu_notifier_mm_init(mm);
 		return mm;
 	}

Index: linux-2.6.28-rc7-mmotm-081203/fs/binfmt_elf.c
===================================================================

fs/binfmt_elf.c +1 -0

@@ -585,6 +585,7 @@ static int load_elf_binary(struct linux_
 	unsigned long reloc_func_desc = 0;
 	int executable_stack = EXSTACK_DEFAULT;
 	unsigned long def_flags = 0;
+	int mcl_inherit = 0;
 	struct {
 		struct elfhdr elf_ex;
 		struct elfhdr interp_elf_ex;

fs/binfmt_elf.c +4 -0

+		if (current->mm->mcl_inherit & MCL_RECURSIVE)
+			mcl_inherit  = current->mm->mcl_inherit;
+	}
+
 	/* Flush all traces of the currently running executable */
 	retval = flush_old_exec(bprm);
 	if (retval)

Index: linux-2.6.28-rc7-mmotm-081203/arch/x86/include/asm/mman.h
===================================================================

arch/x86/include/asm/mman.h +3 -0

@@ -16,5 +16,8 @@
 
 #define MCL_CURRENT	1		/* lock all current mappings */
 #define MCL_FUTURE	2		/* lock all future mappings */
+#define MCL_INHERIT	4		/* inherit mlocks across fork */
+					/* inherit '_FUTURE flag across fork/exec */
+#define MCL_RECURSIVE	8		/* inherit mlocks recursively */
 
 #endif /* _ASM_X86_MMAN_H */

Index: linux-2.6.28-rc7-mmotm-081203/include/linux/mm_types.h
===================================================================

(cc linux-abi. Again.)

Quoted 69 lines

Against; 2.6.28-rc7-mmotm-081203

V02: rework vetting of flags argument as suggested by
Kosaki Motohiro.
enhance description as requested by Andrew Morton.

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):

MCL_CURRENT[|MCL_FUTURE]|MCL_INHERIT - inherit memory locks
[vmas' VM_LOCKED flags] across fork(), and inherit MCL_FUTURE behavior [mm's def_flags] across fork() and exec(). Behaves as if child and/or new task called mlockall(MCL_CURRENT|MCL_FUTURE) as first instruction.

MCL_RECURSIVE - inherit MCL_CURRENT|MCL_FUTURE|MCL_INHERIT
[vmas' VM_LOCKED flags for fork() and mm's def_flags and mcl_inherit across fork() and exec()] for all future generations of calling process's descendants. Behaves as if child and/or new task called mlockall(MCL_CURRENT|MCL_FUTURE|MCL_INHERIT|MCL_RECURSIVE) as the first instruction.

In support of a "lock prefix command"--e.g., mlock <cmd> <args> ... Analogous to taskset(1) for cpu affinity or numactl(8) for numa memory policy.

Together with patches to keep mlocked pages off the LRU, this will allow users/admins to lock down applications without modifying them, if their RLIMIT_MEMLOCK is sufficiently large, keeping their pages off the LRU and out of consideration for reclaim.

Potentially useful, as well, in real-time environments to force prefaulting and residency for applications that don't mlock themselves.

Jeff Sharkey at Montana State developed a similar patch for Linux [link no longer accessible], but apparently he never submitted the patch.

I submitted an earlier version of this patch around a year ago. I resurrected it to test the unevictable lru/mlocked pages patches-- e.g., by "mlock -r make -j<N*nr_cpus> all". This did shake out a few races and vmstat accounting bugs, but NOT something I'd recommend as general practice--for kernel builds, that is.

----

Define MCL_INHERIT, MCL_RECURSIVE in <asm-*/mman.h>.
+ x86 and ia64 versions included.
+ other arch can/will be created, if this patch deemed merge-worthy.

Similarly, I'll provide kernel man page update if/when needed.

Example "lock prefix command" in Documentation/vm/mlock.c

...

mm/mlock.c +1 -0

@@ -573,15 +573,18 @@ asmlinkage long sys_munlock(unsigned lon
 static int do_mlockall(int flags)
 {
 	struct vm_area_struct * vma, * prev = NULL;
+	struct mm_struct *mm = current->mm;
 	unsigned int def_flags = 0;
 

mm/mlock.c asmlinkage long sys_munlock(unsigned lon · line 580

Quoted 5 lines

mm/mlock.c +1 -3

 	if (flags & MCL_FUTURE)
-		def_flags = VM_LOCKED;
-	current->mm->def_flags = def_flags;
-	if (flags == MCL_FUTURE)
+		def_flags = VM_LOCKED;;

You get paid by the semicolon?

80 char line in x86 mman.h, but there was already a much longer line,

mm/mlock.c asmlinkage long sys_munlock(unsigned lon · line 583

Quoted 4 lines

mm/mlock.c +3 -0

+	mm->def_flags = def_flags;
+	if (flags & MCL_INHERIT)
+		mm->mcl_inherit = flags & (MCL_INHERIT | MCL_RECURSIVE);

aaaaaaarrrrrrggggggghhhhhhh!

So mm->mcl_inherit is _not_ a boolean meaning "this mm has MCL_INHERIT set", as I naively believed when I saw it. Is it in fact a composite of both MCL_INHERIT and MCL_RECURSIVE, given a badly misleading name and a waffly comment.

Can we please make this clearer?

Quoted 38 lines

...

kernel/fork.c +6 -2

@@ -278,7 +278,8 @@ static int dup_mmap(struct mm_struct *mm
 	 */
 	down_write_nested(&mm->mmap_sem, SINGLE_DEPTH_NESTING);
 
-	mm->locked_vm = 0;
+	if (!mm->mcl_inherit)
+		mm->locked_vm = 0;
 	mm->mmap = NULL;
 	mm->mmap_cache = NULL;
 	mm->free_area_cache = oldmm->mmap_base;

@@ -316,7 +317,8 @@ static int dup_mmap(struct mm_struct *mm
 		if (IS_ERR(pol))
 			goto fail_nomem_policy;
 		vma_set_policy(tmp, pol);
-		tmp->vm_flags &= ~VM_LOCKED;
+		if (!mm->mcl_inherit)
+			tmp->vm_flags &= ~VM_LOCKED;
 		tmp->vm_mm = mm;
 		tmp->vm_next = NULL;
 		anon_vma_link(tmp);

@@ -406,6 +408,8 @@ __cacheline_aligned_in_smp DEFINE_SPINLO
 
 static struct mm_struct * mm_init(struct mm_struct * mm, struct task_struct *p)
 {
+	unsigned long def_flags = 0;
+
 	atomic_set(&mm->mm_users, 1);
 	atomic_set(&mm->mm_count, 1);
 	init_rwsem(&mm->mmap_sem);

@@ -422,9 +426,14 @@ static struct mm_struct * mm_init(struct
 	mm->free_area_cache = TASK_UNMAPPED_BASE;
 	mm->cached_hole_size = ~0UL;

kernel/fork.c static struct mm_struct * mm_init(struct · line 433

Quoted 6 lines

kernel/fork.c +5 -0

 	mm_init_owner(mm, p);
+	if (current->mm && current->mm->mcl_inherit) {
+		def_flags = current->mm->def_flags & VM_LOCKED;
+		if (mm->mcl_inherit & MCL_RECURSIVE)
+			mm->mcl_inherit  = current->mm->mcl_inherit;
+	}

hm. That code looks familiar. Not worth a helper function?

Quoted 23 lines

kernel/fork.c +1 -1

 	if (likely(!mm_alloc_pgd(mm))) {
-		mm->def_flags = 0;
+		mm->def_flags = def_flags;
 		mmu_notifier_mm_init(mm);
 		return mm;
 	}

Index: linux-2.6.28-rc7-mmotm-081203/fs/binfmt_elf.c
===================================================================

fs/binfmt_elf.c +1 -0

@@ -585,6 +585,7 @@ static int load_elf_binary(struct linux_
 	unsigned long reloc_func_desc = 0;
 	int executable_stack = EXSTACK_DEFAULT;
 	unsigned long def_flags = 0;
+	int mcl_inherit = 0;
 	struct {
 		struct elfhdr elf_ex;
 		struct elfhdr interp_elf_ex;

@@ -749,6 +750,13 @@ static int load_elf_binary(struct linux_
 		SET_PERSONALITY(loc->elf_ex);
 	}
 

fs/binfmt_elf.c static int load_elf_binary(struct linux_ · line 754

Quoted 2 lines

fs/binfmt_elf.c +2 -0

+	/* Optionally inherit MCL_FUTURE state before destroying old mm */
+	if (current->mm && current->mm->mcl_inherit) {

umm, OK, it looks like current->mm can be null here.

fs/binfmt_elf.c static int load_elf_binary(struct linux_ · line 755

Quoted 2 lines

fs/binfmt_elf.c +1 -0

+		def_flags = current->mm->def_flags & VM_LOCKED;

ooh, we finally used local variable def_flags for something. That's been wasting space since 2.2.26 (or earlier).

Quoted 14 lines

fs/binfmt_elf.c +5 -0

+		if (current->mm->mcl_inherit & MCL_RECURSIVE)
+			mcl_inherit  = current->mm->mcl_inherit;
+	}
+
 	/* Flush all traces of the currently running executable */
 	retval = flush_old_exec(bprm);
 	if (retval)

@@ -757,6 +765,7 @@ static int load_elf_binary(struct linux_
 	/* OK, This is the point of no return */
 	current->flags &= ~PF_FORKNOEXEC;
 	current->mm->def_flags = def_flags;
+	current->mm->mcl_inherit = mcl_inherit;
 

fs/binfmt_elf.c static int load_elf_binary(struct linux_ · line 771

Quoted 2 lines

fs/binfmt_elf.c +0 -0

 	/* Do this immediately, since STACK_TOP as used in setup_arg_pages
 	   may depend on the personality.  */

Why is this done in binfmt_elf.c? Doesn't this preclude the operation of these new flags for other binary formats?

Quoted 22 lines

Index: linux-2.6.28-rc7-mmotm-081203/arch/x86/include/asm/mman.h
===================================================================

arch/x86/include/asm/mman.h +3 -0

@@ -16,5 +16,8 @@
 
 #define MCL_CURRENT	1		/* lock all current mappings */
 #define MCL_FUTURE	2		/* lock all future mappings */
+#define MCL_INHERIT	4		/* inherit mlocks across fork */
+					/* inherit '_FUTURE flag across fork/exec */
+#define MCL_RECURSIVE	8		/* inherit mlocks recursively */
 
 #endif /* _ASM_X86_MMAN_H */

Index: linux-2.6.28-rc7-mmotm-081203/include/linux/mm_types.h
===================================================================

include/linux/mm_types.h +0 -0

@@ -235,6 +235,8 @@ struct mm_struct {
 	unsigned int token_priority;
 	unsigned int last_interval;
 

include/linux/mm_types.h struct mm_struct { · line 240

Quoted 3 lines

include/linux/mm_types.h +2 -0

+	int mcl_inherit;		/* inherit current/future locks */
+
 	unsigned long flags; /* Must use atomic bitops to access the bits */

I was thinking that the boolean mcl_inherit could become a bit in mm.flags. That was before I unmisled myself about it.

Perhaps we could use two bits. That might get a bit nasty.

(cc linux-abi. Again.)

Quoted 68 lines · On Wed, 03 Dec 2008 14:04:29 -0500 Lee Schermerhorn…

Against; 2.6.28-rc7-mmotm-081203

V02: rework vetting of flags argument as suggested by
Kosaki Motohiro.
enhance description as requested by Andrew Morton.

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):

MCL_CURRENT[|MCL_FUTURE]|MCL_INHERIT - inherit memory locks
[vmas' VM_LOCKED flags] across fork(), and inherit MCL_FUTURE behavior [mm's def_flags] across fork() and exec(). Behaves as if child and/or new task called mlockall(MCL_CURRENT|MCL_FUTURE) as first instruction.

MCL_RECURSIVE - inherit MCL_CURRENT|MCL_FUTURE|MCL_INHERIT
[vmas' VM_LOCKED flags for fork() and mm's def_flags and mcl_inherit across fork() and exec()] for all future generations of calling process's descendants. Behaves as if child and/or new task called mlockall(MCL_CURRENT|MCL_FUTURE|MCL_INHERIT|MCL_RECURSIVE) as the first instruction.

In support of a "lock prefix command"--e.g., mlock <cmd> <args> ... Analogous to taskset(1) for cpu affinity or numactl(8) for numa memory policy.

Together with patches to keep mlocked pages off the LRU, this will allow users/admins to lock down applications without modifying them, if their RLIMIT_MEMLOCK is sufficiently large, keeping their pages off the LRU and out of consideration for reclaim.

Potentially useful, as well, in real-time environments to force prefaulting and residency for applications that don't mlock themselves.

Jeff Sharkey at Montana State developed a similar patch for Linux [link no longer accessible], but apparently he never submitted the patch.

I submitted an earlier version of this patch around a year ago. I resurrected it to test the unevictable lru/mlocked pages patches-- e.g., by "mlock -r make -j<N*nr_cpus> all". This did shake out a few races and vmstat accounting bugs, but NOT something I'd recommend as general practice--for kernel builds, that is.

----

Define MCL_INHERIT, MCL_RECURSIVE in <asm-*/mman.h>.
+ x86 and ia64 versions included.
+ other arch can/will be created, if this patch deemed merge-worthy.

Similarly, I'll provide kernel man page update if/when needed.

Example "lock prefix command" in Documentation/vm/mlock.c

...

mm/mlock.c +1 -0

@@ -573,15 +573,18 @@ asmlinkage long sys_munlock(unsigned lon
 static int do_mlockall(int flags)
 {
 	struct vm_area_struct * vma, * prev = NULL;
+	struct mm_struct *mm = current->mm;
 	unsigned int def_flags = 0;
 

mm/mlock.c asmlinkage long sys_munlock(unsigned lon · line 580

Quoted 5 lines

mm/mlock.c +1 -3

 	if (flags & MCL_FUTURE)
-		def_flags = VM_LOCKED;
-	current->mm->def_flags = def_flags;
-	if (flags == MCL_FUTURE)
+		def_flags = VM_LOCKED;;

You get paid by the semicolon?

80 char line in x86 mman.h, but there was already a much longer line,

mm/mlock.c asmlinkage long sys_munlock(unsigned lon · line 583

Quoted 4 lines

mm/mlock.c +3 -0

+	mm->def_flags = def_flags;
+	if (flags & MCL_INHERIT)
+		mm->mcl_inherit = flags & (MCL_INHERIT | MCL_RECURSIVE);

aaaaaaarrrrrrggggggghhhhhhh!

So mm->mcl_inherit is _not_ a boolean meaning "this mm has MCL_INHERIT set", as I naively believed when I saw it. Is it in fact a composite of both MCL_INHERIT and MCL_RECURSIVE, given a badly misleading name and a waffly comment.

Can we please make this clearer?

Quoted 38 lines

...

kernel/fork.c +6 -2

@@ -278,7 +278,8 @@ static int dup_mmap(struct mm_struct *mm
 	 */
 	down_write_nested(&mm->mmap_sem, SINGLE_DEPTH_NESTING);
 
-	mm->locked_vm = 0;
+	if (!mm->mcl_inherit)
+		mm->locked_vm = 0;
 	mm->mmap = NULL;
 	mm->mmap_cache = NULL;
 	mm->free_area_cache = oldmm->mmap_base;

@@ -316,7 +317,8 @@ static int dup_mmap(struct mm_struct *mm
 		if (IS_ERR(pol))
 			goto fail_nomem_policy;
 		vma_set_policy(tmp, pol);
-		tmp->vm_flags &= ~VM_LOCKED;
+		if (!mm->mcl_inherit)
+			tmp->vm_flags &= ~VM_LOCKED;
 		tmp->vm_mm = mm;
 		tmp->vm_next = NULL;
 		anon_vma_link(tmp);

@@ -406,6 +408,8 @@ __cacheline_aligned_in_smp DEFINE_SPINLO
 
 static struct mm_struct * mm_init(struct mm_struct * mm, struct task_struct *p)
 {
+	unsigned long def_flags = 0;
+
 	atomic_set(&mm->mm_users, 1);
 	atomic_set(&mm->mm_count, 1);
 	init_rwsem(&mm->mmap_sem);

@@ -422,9 +426,14 @@ static struct mm_struct * mm_init(struct
 	mm->free_area_cache = TASK_UNMAPPED_BASE;
 	mm->cached_hole_size = ~0UL;

kernel/fork.c static struct mm_struct * mm_init(struct · line 433

Quoted 6 lines

kernel/fork.c +5 -0

 	mm_init_owner(mm, p);
+	if (current->mm && current->mm->mcl_inherit) {
+		def_flags = current->mm->def_flags & VM_LOCKED;
+		if (mm->mcl_inherit & MCL_RECURSIVE)
+			mm->mcl_inherit  = current->mm->mcl_inherit;
+	}

hm. That code looks familiar. Not worth a helper function?

Quoted 23 lines

kernel/fork.c +1 -1

 	if (likely(!mm_alloc_pgd(mm))) {
-		mm->def_flags = 0;
+		mm->def_flags = def_flags;
 		mmu_notifier_mm_init(mm);
 		return mm;
 	}

Index: linux-2.6.28-rc7-mmotm-081203/fs/binfmt_elf.c
===================================================================

fs/binfmt_elf.c +1 -0

@@ -585,6 +585,7 @@ static int load_elf_binary(struct linux_
 	unsigned long reloc_func_desc = 0;
 	int executable_stack = EXSTACK_DEFAULT;
 	unsigned long def_flags = 0;
+	int mcl_inherit = 0;
 	struct {
 		struct elfhdr elf_ex;
 		struct elfhdr interp_elf_ex;

@@ -749,6 +750,13 @@ static int load_elf_binary(struct linux_
 		SET_PERSONALITY(loc->elf_ex);
 	}
 

fs/binfmt_elf.c static int load_elf_binary(struct linux_ · line 754

Quoted 2 lines

fs/binfmt_elf.c +2 -0

+	/* Optionally inherit MCL_FUTURE state before destroying old mm */
+	if (current->mm && current->mm->mcl_inherit) {

umm, OK, it looks like current->mm can be null here.

fs/binfmt_elf.c static int load_elf_binary(struct linux_ · line 755

Quoted 2 lines

fs/binfmt_elf.c +1 -0

+		def_flags = current->mm->def_flags & VM_LOCKED;

ooh, we finally used local variable def_flags for something. That's been wasting space since 2.2.26 (or earlier).

Quoted 14 lines

fs/binfmt_elf.c +5 -0

+		if (current->mm->mcl_inherit & MCL_RECURSIVE)
+			mcl_inherit  = current->mm->mcl_inherit;
+	}
+
 	/* Flush all traces of the currently running executable */
 	retval = flush_old_exec(bprm);
 	if (retval)

@@ -757,6 +765,7 @@ static int load_elf_binary(struct linux_
 	/* OK, This is the point of no return */
 	current->flags &= ~PF_FORKNOEXEC;
 	current->mm->def_flags = def_flags;
+	current->mm->mcl_inherit = mcl_inherit;
 

fs/binfmt_elf.c static int load_elf_binary(struct linux_ · line 771

Quoted 2 lines

fs/binfmt_elf.c +0 -0

 	/* Do this immediately, since STACK_TOP as used in setup_arg_pages
 	   may depend on the personality.  */

Why is this done in binfmt_elf.c? Doesn't this preclude the operation of these new flags for other binary formats?

Quoted 22 lines

Index: linux-2.6.28-rc7-mmotm-081203/arch/x86/include/asm/mman.h
===================================================================

arch/x86/include/asm/mman.h +3 -0

@@ -16,5 +16,8 @@
 
 #define MCL_CURRENT	1		/* lock all current mappings */
 #define MCL_FUTURE	2		/* lock all future mappings */
+#define MCL_INHERIT	4		/* inherit mlocks across fork */
+					/* inherit '_FUTURE flag across fork/exec */
+#define MCL_RECURSIVE	8		/* inherit mlocks recursively */
 
 #endif /* _ASM_X86_MMAN_H */

Index: linux-2.6.28-rc7-mmotm-081203/include/linux/mm_types.h
===================================================================

include/linux/mm_types.h +0 -0

@@ -235,6 +235,8 @@ struct mm_struct {
 	unsigned int token_priority;
 	unsigned int last_interval;
 

include/linux/mm_types.h struct mm_struct { · line 240

Quoted 3 lines

include/linux/mm_types.h +2 -0

+	int mcl_inherit;		/* inherit current/future locks */
+
 	unsigned long flags; /* Must use atomic bitops to access the bits */

I was thinking that the boolean mcl_inherit could become a bit in mm.flags. That was before I unmisled myself about it.

Perhaps we could use two bits. That might get a bit nasty.

Quoted 1 line

Quoted 1 line

In support of a "lock prefix command"--e.g., mlock <cmd>

<args> ...

Quoted 10 lines

Quoted 10 lines

Analogous to taskset(1) for cpu affinity or numactl(8) for numa memory policy.

Together with patches to keep mlocked pages off the LRU, this will allow users/admins to lock down applications without modifying them, if their RLIMIT_MEMLOCK is sufficiently large, keeping their pages off the LRU and out of consideration for reclaim.

Potentially useful, as well, in real-time environments to force prefaulting and residency for applications that don't mlock themselves.

Quoted 10 lines · Lee Schermerhorn

Quoted 1 line

Quoted 1 line

In support of a "lock prefix command"--e.g., mlock <cmd>

<args> ...

Quoted 8 lines

Quoted 8 lines

Analogous to taskset(1) for cpu affinity or numactl(8) for numa memory policy.

Together with patches to keep mlocked pages off the LRU, this will allow users/admins to lock down applications without modifying them, if their RLIMIT_MEMLOCK is sufficiently large, keeping their pages off the LRU and out of consideration for reclaim.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Potentially useful, as well, in real-time environments to force prefaulting and residency for applications that don't mlock themselves.

This is a bit scary to me. Privilege and mode inheritance across processes is the root of many nasty surprises, security and otherwise.

Here's a crazy alternative: add a flag to containers instead? I think this is a better match to what you're trying to do and will keep people from being surprised when an mlockall call in one thread causes a fork/exec in another thread to crash their box, but only sometimes.

Quoted 11 lines · Matt Mackall

Quoted 10 lines · Lee Schermerhorn

Quoted 1 line

Quoted 1 line

In support of a "lock prefix command"--e.g., mlock <cmd>

<args> ...

Quoted 8 lines

Quoted 8 lines

Analogous to taskset(1) for cpu affinity or numactl(8) for numa memory policy.

Together with patches to keep mlocked pages off the LRU, this will allow users/admins to lock down applications without modifying them, if their RLIMIT_MEMLOCK is sufficiently large, keeping their pages off the LRU and out of consideration for reclaim.

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Potentially useful, as well, in real-time environments to force prefaulting and residency for applications that don't mlock themselves.

This is a bit scary to me. Privilege and mode inheritance across processes is the root of many nasty surprises, security and otherwise.

Hi, Matt:

Could you explain more about this issue? I believe that the patch doesn't provide any privileges or capabilities that a process doesn't already have. It just allows one to cause a process and, optionally, its descendants, to behave as if one had modified the sources to invoke mlockall() directly. It is still subject to each individual task's resource limits. At least, that was my intent.

As far as "what I'm trying to do": I see this as exactly like using taskset to set the cpu affinity of a task, or numactl to set the task mempolicy without modifying the source.

If one had access to the
program source and wouldn't, e.g., void a support contract by modifying it, one could just insert the calls into the source itself.

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):

Quoted 2 lines

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):

FWIW, I really liked this patch series. And I think there is still value in a generic "mlock" wrapper utility that I can use. Sure, the later on containers suggestions are all wonderful in theory but I don't see that that went anywhere either (and I disagree that we can't trust people to use this right without doing silly things) - if I'm really right that this got dropped on the floor, can we resurrect it in .31 please?

Quoted 4 lines · Lee Schermerhorn

Quoted 2 lines

Add support for mlockall(MCL_INHERIT|MCL_RECURSIVE):

Quoted 6 lines

FWIW, I really liked this patch series. And I think there is still value in a generic "mlock" wrapper utility that I can use. Sure, the later on containers suggestions are all wonderful in theory but I don't see that that went anywhere either (and I disagree that we can't trust people to use this right without doing silly things) - if I'm really right that this got dropped on the floor, can we resurrect it in .31 please?

I guess Lee is really really busy now.

Can you make V3 patch instead?

only one nit.

mm/mlock.c asmlinkage long sys_mlockall(int flags) · line 607

Quoted 8 lines

mm/mlock.c +2 -1

@@ -599,7 +602,8 @@ asmlinkage long sys_mlockall(int flags)
 	unsigned long lock_limit;
 	int ret = -EINVAL;
 
-	if (!flags || (flags & ~(MCL_CURRENT | MCL_FUTURE)))
+	if (!flags || (flags & ~(MCL_CURRENT | MCL_FUTURE |
+				 MCL_INHERIT | MCL_RECURSIVE)))
 		goto out;

from patch description, I think mlockall(MCL_INHERIT) and mlockall(MCL_RECURSIVE) are incorrect. right?

if so, I think following likes error check is needed.

if (!(flags & (MCL_CURRENT | MCL_FUTURE)))

	goto out;

if ((flags & (MCL_INHERIT | MECL_RECURSIVE)) == MCL_RECURSIVE)

	goto out;

Hello, Kosaki-san:

Thanks for looking at this. I think you mean that:

1) don't allow MCL_INHERIT | MCL_RECURSIVE without either MCL_CURRENT or MCL_FUTURE, and

2) MCL_RECURSIVE without MCL_INHERIT does not make sense, either.

Is this correct?

I guess I agree with you. As is stands, my patch would allow MCL_INHERIT[|MCL_RECURSIVE] to sneak through with neither MCL_CURRENT nor MCL_FUTURE set. Looks like this would result in mlock_fixup() being called with a newflags that does not containing VM_LOCKED. This would
be treated as munlockall(). Not good. Your first check would catch
this.

The second condition would be a no-op, I think. We only look at look for MCL_RECURSIVE in mm->mcl_inherit when mcl_inherit is non-zero; and we only set mcl_inherit when MCL_INHERIT is specified. But, if the caller specified MCL_RECURSIVE, they probably intended something to happen, and since it won't, best to return an error.

I'll fix this up and send it out to the wider distribution that Andrew requested.

Thanks, again.