2010-01-13 09:31:29

by Gleb Natapov

[permalink] [raw]
Subject: [PATCH v5] add MAP_UNLOCKED mmap flag

If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
file bigger than main memory or allocate big area of anonymous memory
in a thread safe manner. Sometimes it is desirable to lock everything
related to program execution into memory, but still be able to mmap
big file or allocate huge amount of memory and allow OS to swap them on
demand. MAP_UNLOCKED allows to do that.

Signed-off-by: Gleb Natapov <[email protected]>
---

I get reports that people find this useful, so resending.

v1->v2
- adding new flag to all archs
- fixing typo
v2->v3
- one more typo fix
v3->v4
- return error if MAP_LOCKED | MAP_UNLOCKED is specified
v4->v5
- rebase to latest head

diff --git a/arch/alpha/include/asm/mman.h b/arch/alpha/include/asm/mman.h
index 99c56d4..cfc51ac 100644
--- a/arch/alpha/include/asm/mman.h
+++ b/arch/alpha/include/asm/mman.h
@@ -30,6 +30,7 @@
#define MAP_NONBLOCK 0x40000 /* do not block on IO */
#define MAP_STACK 0x80000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x100000 /* create a huge page mapping */
+#define MAP_UNLOCKED 0x200000 /* force page unlocking */

#define MS_ASYNC 1 /* sync memory asynchronously */
#define MS_SYNC 2 /* synchronous memory sync */
diff --git a/arch/mips/include/asm/mman.h b/arch/mips/include/asm/mman.h
index c892bfb..3e4d108 100644
--- a/arch/mips/include/asm/mman.h
+++ b/arch/mips/include/asm/mman.h
@@ -48,6 +48,7 @@
#define MAP_NONBLOCK 0x20000 /* do not block on IO */
#define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x80000 /* create a huge page mapping */
+#define MAP_UNLOCKED 0x100000 /* force page unlocking */

/*
* Flags for msync
diff --git a/arch/parisc/include/asm/mman.h b/arch/parisc/include/asm/mman.h
index 9749c8a..4e8b9bf 100644
--- a/arch/parisc/include/asm/mman.h
+++ b/arch/parisc/include/asm/mman.h
@@ -24,6 +24,7 @@
#define MAP_NONBLOCK 0x20000 /* do not block on IO */
#define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x80000 /* create a huge page mapping */
+#define MAP_UNLOCKED 0x100000 /* force page unlocking */

#define MS_SYNC 1 /* synchronous memory sync */
#define MS_ASYNC 2 /* sync memory asynchronously */
diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index d4a7f64..7d33f01 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -27,6 +27,7 @@
#define MAP_NONBLOCK 0x10000 /* do not block on IO */
#define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x40000 /* create a huge page mapping */
+#define MAP_UNLOCKED 0x80000 /* force page unlocking */

#ifdef __KERNEL__
#ifdef CONFIG_PPC64
diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h
index c3029ad..f80d203 100644
--- a/arch/sparc/include/asm/mman.h
+++ b/arch/sparc/include/asm/mman.h
@@ -22,6 +22,7 @@
#define MAP_NONBLOCK 0x10000 /* do not block on IO */
#define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x40000 /* create a huge page mapping */
+#define MAP_UNLOCKED 0x80000 /* force page unlocking */

#ifdef __KERNEL__
#ifndef __ASSEMBLY__
diff --git a/arch/xtensa/include/asm/mman.h b/arch/xtensa/include/asm/mman.h
index fca4db4..c62bcd8 100644
--- a/arch/xtensa/include/asm/mman.h
+++ b/arch/xtensa/include/asm/mman.h
@@ -55,6 +55,7 @@
#define MAP_NONBLOCK 0x20000 /* do not block on IO */
#define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x80000 /* create a huge page mapping */
+#define MAP_UNLOCKED 0x100000 /* force page unlocking */

/*
* Flags for msync
diff --git a/include/asm-generic/mman.h b/include/asm-generic/mman.h
index 32c8bd6..59e0f29 100644
--- a/include/asm-generic/mman.h
+++ b/include/asm-generic/mman.h
@@ -12,6 +12,7 @@
#define MAP_NONBLOCK 0x10000 /* do not block on IO */
#define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x40000 /* create a huge page mapping */
+#define MAP_UNLOCKED 0x80000 /* force page unlocking */

#define MCL_CURRENT 1 /* lock all current mappings */
#define MCL_FUTURE 2 /* lock all future mappings */
diff --git a/mm/mmap.c b/mm/mmap.c
index ee22989..4bda220 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -962,6 +962,9 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
if (!can_do_mlock())
return -EPERM;

+ if (flags & MAP_UNLOCKED)
+ vm_flags &= ~VM_LOCKED;
+
/* mlock MCL_FUTURE? */
if (vm_flags & VM_LOCKED) {
unsigned long locked, lock_limit;
@@ -1050,7 +1053,10 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
struct file *file = NULL;
unsigned long retval = -EBADF;

- if (!(flags & MAP_ANONYMOUS)) {
+ if (unlikely((flags & (MAP_LOCKED | MAP_UNLOCKED)) ==
+ (MAP_LOCKED | MAP_UNLOCKED))) {
+ return -EINVAL;
+ } else if (!(flags & MAP_ANONYMOUS)) {
if (unlikely(flags & MAP_HUGETLB))
return -EINVAL;
file = fget(fd);
--
Gleb.


2010-01-13 14:34:16

by Cong Wang

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

On Wed, Jan 13, 2010 at 11:31:19AM +0200, Gleb Natapov wrote:
>If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
>file bigger than main memory or allocate big area of anonymous memory
>in a thread safe manner. Sometimes it is desirable to lock everything
>related to program execution into memory, but still be able to mmap
>big file or allocate huge amount of memory and allow OS to swap them on
>demand. MAP_UNLOCKED allows to do that.
>
>Signed-off-by: Gleb Natapov <[email protected]>

Thanks for keeping working on it.

This version looks fine for me.

Acked-by: WANG Cong <[email protected]>

>---
>
>I get reports that people find this useful, so resending.
>
> v1->v2
> - adding new flag to all archs
> - fixing typo
> v2->v3
> - one more typo fix
> v3->v4
> - return error if MAP_LOCKED | MAP_UNLOCKED is specified
> v4->v5
> - rebase to latest head
>
>diff --git a/arch/alpha/include/asm/mman.h b/arch/alpha/include/asm/mman.h
>index 99c56d4..cfc51ac 100644
>--- a/arch/alpha/include/asm/mman.h
>+++ b/arch/alpha/include/asm/mman.h
>@@ -30,6 +30,7 @@
> #define MAP_NONBLOCK 0x40000 /* do not block on IO */
> #define MAP_STACK 0x80000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x100000 /* create a huge page mapping */
>+#define MAP_UNLOCKED 0x200000 /* force page unlocking */
>
> #define MS_ASYNC 1 /* sync memory asynchronously */
> #define MS_SYNC 2 /* synchronous memory sync */
>diff --git a/arch/mips/include/asm/mman.h b/arch/mips/include/asm/mman.h
>index c892bfb..3e4d108 100644
>--- a/arch/mips/include/asm/mman.h
>+++ b/arch/mips/include/asm/mman.h
>@@ -48,6 +48,7 @@
> #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
>+#define MAP_UNLOCKED 0x100000 /* force page unlocking */
>
> /*
> * Flags for msync
>diff --git a/arch/parisc/include/asm/mman.h b/arch/parisc/include/asm/mman.h
>index 9749c8a..4e8b9bf 100644
>--- a/arch/parisc/include/asm/mman.h
>+++ b/arch/parisc/include/asm/mman.h
>@@ -24,6 +24,7 @@
> #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
>+#define MAP_UNLOCKED 0x100000 /* force page unlocking */
>
> #define MS_SYNC 1 /* synchronous memory sync */
> #define MS_ASYNC 2 /* sync memory asynchronously */
>diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
>index d4a7f64..7d33f01 100644
>--- a/arch/powerpc/include/asm/mman.h
>+++ b/arch/powerpc/include/asm/mman.h
>@@ -27,6 +27,7 @@
> #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
>+#define MAP_UNLOCKED 0x80000 /* force page unlocking */
>
> #ifdef __KERNEL__
> #ifdef CONFIG_PPC64
>diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h
>index c3029ad..f80d203 100644
>--- a/arch/sparc/include/asm/mman.h
>+++ b/arch/sparc/include/asm/mman.h
>@@ -22,6 +22,7 @@
> #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
>+#define MAP_UNLOCKED 0x80000 /* force page unlocking */
>
> #ifdef __KERNEL__
> #ifndef __ASSEMBLY__
>diff --git a/arch/xtensa/include/asm/mman.h b/arch/xtensa/include/asm/mman.h
>index fca4db4..c62bcd8 100644
>--- a/arch/xtensa/include/asm/mman.h
>+++ b/arch/xtensa/include/asm/mman.h
>@@ -55,6 +55,7 @@
> #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
>+#define MAP_UNLOCKED 0x100000 /* force page unlocking */
>
> /*
> * Flags for msync
>diff --git a/include/asm-generic/mman.h b/include/asm-generic/mman.h
>index 32c8bd6..59e0f29 100644
>--- a/include/asm-generic/mman.h
>+++ b/include/asm-generic/mman.h
>@@ -12,6 +12,7 @@
> #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
>+#define MAP_UNLOCKED 0x80000 /* force page unlocking */
>
> #define MCL_CURRENT 1 /* lock all current mappings */
> #define MCL_FUTURE 2 /* lock all future mappings */
>diff --git a/mm/mmap.c b/mm/mmap.c
>index ee22989..4bda220 100644
>--- a/mm/mmap.c
>+++ b/mm/mmap.c
>@@ -962,6 +962,9 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
> if (!can_do_mlock())
> return -EPERM;
>
>+ if (flags & MAP_UNLOCKED)
>+ vm_flags &= ~VM_LOCKED;
>+
> /* mlock MCL_FUTURE? */
> if (vm_flags & VM_LOCKED) {
> unsigned long locked, lock_limit;
>@@ -1050,7 +1053,10 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
> struct file *file = NULL;
> unsigned long retval = -EBADF;
>
>- if (!(flags & MAP_ANONYMOUS)) {
>+ if (unlikely((flags & (MAP_LOCKED | MAP_UNLOCKED)) ==
>+ (MAP_LOCKED | MAP_UNLOCKED))) {
>+ return -EINVAL;
>+ } else if (!(flags & MAP_ANONYMOUS)) {
> if (unlikely(flags & MAP_HUGETLB))
> return -EINVAL;
> file = fget(fd);
>--
> Gleb.
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/

--
Live like a child, think like the god.

2010-01-13 17:32:34

by Chris Wright

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

* Gleb Natapov ([email protected]) wrote:
> If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
> file bigger than main memory or allocate big area of anonymous memory
> in a thread safe manner. Sometimes it is desirable to lock everything
> related to program execution into memory, but still be able to mmap
> big file or allocate huge amount of memory and allow OS to swap them on
> demand. MAP_UNLOCKED allows to do that.
>
> Signed-off-by: Gleb Natapov <[email protected]>

Looks good to me.

Acked-by: Chris Wright <[email protected]>

thanks,
-chris

2010-01-14 00:31:11

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

> If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
> file bigger than main memory or allocate big area of anonymous memory
> in a thread safe manner. Sometimes it is desirable to lock everything
> related to program execution into memory, but still be able to mmap
> big file or allocate huge amount of memory and allow OS to swap them on
> demand. MAP_UNLOCKED allows to do that.
>
> Signed-off-by: Gleb Natapov <[email protected]>
> ---
>
> I get reports that people find this useful, so resending.

This description is still wrong. It doesn't describe why this patch is useful.


2010-01-14 06:50:18

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

On Thu, Jan 14, 2010 at 09:31:03AM +0900, KOSAKI Motohiro wrote:
> > If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
> > file bigger than main memory or allocate big area of anonymous memory
> > in a thread safe manner. Sometimes it is desirable to lock everything
> > related to program execution into memory, but still be able to mmap
> > big file or allocate huge amount of memory and allow OS to swap them on
> > demand. MAP_UNLOCKED allows to do that.
> >
> > Signed-off-by: Gleb Natapov <[email protected]>
> > ---
> >
> > I get reports that people find this useful, so resending.
>
> This description is still wrong. It doesn't describe why this patch is useful.
>
I think the text above describes the feature it adds and its use
case quite well. Can you elaborate what is missing in your opinion,
or suggest alternative text please?

--
Gleb.

2010-01-14 07:02:47

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

> On Thu, Jan 14, 2010 at 09:31:03AM +0900, KOSAKI Motohiro wrote:
> > > If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
> > > file bigger than main memory or allocate big area of anonymous memory
> > > in a thread safe manner. Sometimes it is desirable to lock everything
> > > related to program execution into memory, but still be able to mmap
> > > big file or allocate huge amount of memory and allow OS to swap them on
> > > demand. MAP_UNLOCKED allows to do that.
> > >
> > > Signed-off-by: Gleb Natapov <[email protected]>
> > > ---
> > >
> > > I get reports that people find this useful, so resending.
> >
> > This description is still wrong. It doesn't describe why this patch is useful.
> >
> I think the text above describes the feature it adds and its use
> case quite well. Can you elaborate what is missing in your opinion,
> or suggest alternative text please?

My point is, introducing mmap new flags need strong and clearly use-case.
All patch should have good benefit/cost balance. the code can describe the cost,
but the benefit can be only explained by the patch description.

I don't think this poor description explained bit benefit rather than cost.
you should explain why this patch is useful and not just pretty toy.

2010-01-14 07:23:14

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

On Thu, Jan 14, 2010 at 04:02:42PM +0900, KOSAKI Motohiro wrote:
> > On Thu, Jan 14, 2010 at 09:31:03AM +0900, KOSAKI Motohiro wrote:
> > > > If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
> > > > file bigger than main memory or allocate big area of anonymous memory
> > > > in a thread safe manner. Sometimes it is desirable to lock everything
> > > > related to program execution into memory, but still be able to mmap
> > > > big file or allocate huge amount of memory and allow OS to swap them on
> > > > demand. MAP_UNLOCKED allows to do that.
> > > >
> > > > Signed-off-by: Gleb Natapov <[email protected]>
> > > > ---
> > > >
> > > > I get reports that people find this useful, so resending.
> > >
> > > This description is still wrong. It doesn't describe why this patch is useful.
> > >
> > I think the text above describes the feature it adds and its use
> > case quite well. Can you elaborate what is missing in your opinion,
> > or suggest alternative text please?
>
> My point is, introducing mmap new flags need strong and clearly use-case.
> All patch should have good benefit/cost balance. the code can describe the cost,
> but the benefit can be only explained by the patch description.
>
> I don't think this poor description explained bit benefit rather than cost.
> you should explain why this patch is useful and not just pretty toy.
>
The benefit is that with this patch I can lock all of my application in
memory except some very big memory areas. My use case is that I want to
run virtual machine in such a way that everything related to machine
emulator is locked into the memory, but guest address space can be
swapped out at will. Guest address space is so huge that it is not
possible to allocated it locked and then unlock. I was very surprised
that current Linux API has no way to do it hence this patch. It may look
like a pretty toy to you until some day you need this and has no way to
do it.

--
Gleb.

2010-01-14 07:30:56

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

> On Thu, Jan 14, 2010 at 04:02:42PM +0900, KOSAKI Motohiro wrote:
> > > On Thu, Jan 14, 2010 at 09:31:03AM +0900, KOSAKI Motohiro wrote:
> > > > > If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
> > > > > file bigger than main memory or allocate big area of anonymous memory
> > > > > in a thread safe manner. Sometimes it is desirable to lock everything
> > > > > related to program execution into memory, but still be able to mmap
> > > > > big file or allocate huge amount of memory and allow OS to swap them on
> > > > > demand. MAP_UNLOCKED allows to do that.
> > > > >
> > > > > Signed-off-by: Gleb Natapov <[email protected]>
> > > > > ---
> > > > >
> > > > > I get reports that people find this useful, so resending.
> > > >
> > > > This description is still wrong. It doesn't describe why this patch is useful.
> > > >
> > > I think the text above describes the feature it adds and its use
> > > case quite well. Can you elaborate what is missing in your opinion,
> > > or suggest alternative text please?
> >
> > My point is, introducing mmap new flags need strong and clearly use-case.
> > All patch should have good benefit/cost balance. the code can describe the cost,
> > but the benefit can be only explained by the patch description.
> >
> > I don't think this poor description explained bit benefit rather than cost.
> > you should explain why this patch is useful and not just pretty toy.
> >
> The benefit is that with this patch I can lock all of my application in
> memory except some very big memory areas. My use case is that I want to
> run virtual machine in such a way that everything related to machine
> emulator is locked into the memory, but guest address space can be
> swapped out at will. Guest address space is so huge that it is not
> possible to allocated it locked and then unlock. I was very surprised
> that current Linux API has no way to do it hence this patch. It may look
> like a pretty toy to you until some day you need this and has no way to
> do it.

Hmm..
Your answer didn't match I wanted.
few additional questions.

- Why don't you change your application? It seems natural way than kernel change.
- Why do you want your virtual machine have mlockall? AFAIK, current majority
virtual machine doesn't.
- If this feature added, average distro user can get any benefit?

I mean, many application developrs want to add their specific feature
into kernel. but if we allow it unlimitedly, major syscall become
the trushbox of pretty toy feature soon.


2010-01-14 08:01:59

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

On Thu, Jan 14, 2010 at 04:30:51PM +0900, KOSAKI Motohiro wrote:
> > On Thu, Jan 14, 2010 at 04:02:42PM +0900, KOSAKI Motohiro wrote:
> > > > On Thu, Jan 14, 2010 at 09:31:03AM +0900, KOSAKI Motohiro wrote:
> > > > > > If application does mlockall(MCL_FUTURE) it is no longer possible to mmap
> > > > > > file bigger than main memory or allocate big area of anonymous memory
> > > > > > in a thread safe manner. Sometimes it is desirable to lock everything
> > > > > > related to program execution into memory, but still be able to mmap
> > > > > > big file or allocate huge amount of memory and allow OS to swap them on
> > > > > > demand. MAP_UNLOCKED allows to do that.
> > > > > >
> > > > > > Signed-off-by: Gleb Natapov <[email protected]>
> > > > > > ---
> > > > > >
> > > > > > I get reports that people find this useful, so resending.
> > > > >
> > > > > This description is still wrong. It doesn't describe why this patch is useful.
> > > > >
> > > > I think the text above describes the feature it adds and its use
> > > > case quite well. Can you elaborate what is missing in your opinion,
> > > > or suggest alternative text please?
> > >
> > > My point is, introducing mmap new flags need strong and clearly use-case.
> > > All patch should have good benefit/cost balance. the code can describe the cost,
> > > but the benefit can be only explained by the patch description.
> > >
> > > I don't think this poor description explained bit benefit rather than cost.
> > > you should explain why this patch is useful and not just pretty toy.
> > >
> > The benefit is that with this patch I can lock all of my application in
> > memory except some very big memory areas. My use case is that I want to
> > run virtual machine in such a way that everything related to machine
> > emulator is locked into the memory, but guest address space can be
> > swapped out at will. Guest address space is so huge that it is not
> > possible to allocated it locked and then unlock. I was very surprised
> > that current Linux API has no way to do it hence this patch. It may look
> > like a pretty toy to you until some day you need this and has no way to
> > do it.
>
> Hmm..
> Your answer didn't match I wanted.
Then I don't get what you want.

> few additional questions.
>
> - Why don't you change your application? It seems natural way than kernel change.
There is no way to change my application and achieve what I've described
in a multithreaded app.

> - Why do you want your virtual machine have mlockall? AFAIK, current majority
> virtual machine doesn't.
It is absolutely irrelevant for that patch, but just because you ask I
want to measure the cost of swapping out of a guest memory.

> - If this feature added, average distro user can get any benefit?
>
?! Is this some kind of new measure? There are plenty of much more
invasive features that don't bring benefits to an average distro user.
This feature can bring benefit to embedded/RT developers.

> I mean, many application developrs want to add their specific feature
> into kernel. but if we allow it unlimitedly, major syscall become
> the trushbox of pretty toy feature soon.
>
And if application developer wants to extend kernel in a way that it
will be possible to do something that was not possible before why is
this a bad thing? I would agree with you if for my problem was userspace
solution, but there is none. The mmap interface is asymmetric in regards
to mlock currently. There is MAP_LOCKED, but no MAP_UNLOCKED. Why
MAP_LOCKED is useful then?


--
Gleb.

2010-01-14 08:17:41

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

> > Hmm..
> > Your answer didn't match I wanted.
> Then I don't get what you want.

I want to know the benefit of the patch for patch reviewing.


> > few additional questions.
> >
> > - Why don't you change your application? It seems natural way than kernel change.
> There is no way to change my application and achieve what I've described
> in a multithreaded app.

Then, we don't recommend to use mlockall(). I don't hope to hear your conclusion,
it is not objectivization. I hope to hear why you reached such conclusion.


> > - Why do you want your virtual machine have mlockall? AFAIK, current majority
> > virtual machine doesn't.
> It is absolutely irrelevant for that patch, but just because you ask I
> want to measure the cost of swapping out of a guest memory.

No. if you stop to use mlockall, the issue is vanished.


> > - If this feature added, average distro user can get any benefit?
> >
> ?! Is this some kind of new measure? There are plenty of much more
> invasive features that don't bring benefits to an average distro user.
> This feature can bring benefit to embedded/RT developers.

I mean who get benifit?


> > I mean, many application developrs want to add their specific feature
> > into kernel. but if we allow it unlimitedly, major syscall become
> > the trushbox of pretty toy feature soon.
> >
> And if application developer wants to extend kernel in a way that it
> will be possible to do something that was not possible before why is
> this a bad thing? I would agree with you if for my problem was userspace
> solution, but there is none. The mmap interface is asymmetric in regards
> to mlock currently. There is MAP_LOCKED, but no MAP_UNLOCKED. Why
> MAP_LOCKED is useful then?

Why? Because this is formal LKML reviewing process. I'm reviewing your
patch for YOU.

If there is no objective reason, I don't want to continue reviewing.


2010-01-14 10:22:22

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

On Thu, Jan 14, 2010 at 05:17:36PM +0900, KOSAKI Motohiro wrote:
> > > Hmm..
> > > Your answer didn't match I wanted.
> > Then I don't get what you want.
>
> I want to know the benefit of the patch for patch reviewing.
>
>
> > > few additional questions.
> > >
> > > - Why don't you change your application? It seems natural way than kernel change.
> > There is no way to change my application and achieve what I've described
> > in a multithreaded app.
>
> Then, we don't recommend to use mlockall(). I don't hope to hear your conclusion,
> it is not objectivization. I hope to hear why you reached such conclusion.
So what do you recommend? Don't just wave hand on me saying "These
aren't the droids you're looking for". I explained you what I need to
achieve you seems to be trying to convince me I don't really need it
without proposing any alternatives. This is not constructive discussion.

>
>
> > > - Why do you want your virtual machine have mlockall? AFAIK, current majority
> > > virtual machine doesn't.
> > It is absolutely irrelevant for that patch, but just because you ask I
> > want to measure the cost of swapping out of a guest memory.
>
> No. if you stop to use mlockall, the issue is vanished.
>
And emulator parts will be swapped out too which is not what I want.

>
> > > - If this feature added, average distro user can get any benefit?
> > >
> > ?! Is this some kind of new measure? There are plenty of much more
> > invasive features that don't bring benefits to an average distro user.
> > This feature can bring benefit to embedded/RT developers.
>
> I mean who get benifit?
Someone who wants to mlock all application memory, but wants to be able
to mmap big file for reading and understand that access to that file can
cause major fault.

>
>
> > > I mean, many application developrs want to add their specific feature
> > > into kernel. but if we allow it unlimitedly, major syscall become
> > > the trushbox of pretty toy feature soon.
> > >
> > And if application developer wants to extend kernel in a way that it
> > will be possible to do something that was not possible before why is
> > this a bad thing? I would agree with you if for my problem was userspace
> > solution, but there is none. The mmap interface is asymmetric in regards
> > to mlock currently. There is MAP_LOCKED, but no MAP_UNLOCKED. Why
> > MAP_LOCKED is useful then?
>
> Why? Because this is formal LKML reviewing process. I'm reviewing your
> patch for YOU.
>
I appreciate that, but unfortunately it seems that you are trying to dismiss
my arguments on the basis that _you_ don't find that useful.

> If there is no objective reason, I don't want to continue reviewing.
>
--
Gleb.

2010-01-14 19:30:23

by Andrew C. Morrow

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

On Thu, Jan 14, 2010 at 3:17 AM, KOSAKI Motohiro
<[email protected]> wrote:
>> > Hmm..
>> > Your answer didn't match I wanted.
>> Then I don't get what you want.
>
> I want to know the benefit of the patch for patch reviewing.
>

The benefit of the patch is that it makes it possible for an
application which has previously called mlockall(MCL_FUTURE) to
selectively exempt new memory mappings from memory locking, on a
per-mmap-call basis. As was pointed out earlier, there is currently no
thread-safe way for an application to do this. The earlier proposed
workaround of toggling MCL_FUTURE around calls to mmap is racy in a
multi-threaded context. Other threads may manipulate the address space
during the window where MCL_FUTURE is off, subverting the programmers
intended memory locking semantics.

The ability to exempt specific memory mappings from memory locking is
necessary when the region to be mapped is larger than physical memory.
In such cases a call to mmap the region cannot succeed, unless
MAP_UNLOCKED is available.


>
>> > few additional questions.
>> >
>> > - Why don't you change your application? It seems natural way than kernel change.
>> There is no way to change my application and achieve what I've described
>> in a multithreaded app.
>
> Then, we don't recommend to use mlockall(). I don't hope to hear your conclusion,
> it is not objectivization. I hope to hear why you reached such conclusion.
>

I agree that mlockall is a big hammer and should be avoided in most
cases, but there are situations where it is exactly what is needed. In
Gleb's instance, it sounds like he is doing some finicky performance
measurement and major page faults skew his results. In my case, I have
a realtime process where the measured latency impact of major page
faults is unacceptable. In both of these cases, mlockall is a
reasonable approach to eliminating major faults.

However, Gleb and I have independently found ourselves unable to use
mlockall because we also need to create a very large memory mapping
(for which we don't care about major faults). The proposed
MAP_UNLOCKED flag would allow us to override MCL_FUTURE for that one
mapping.

>
>> > - Why do you want your virtual machine have mlockall? AFAIK, current majority
>> > ? virtual machine doesn't.
>> It is absolutely irrelevant for that patch, but just because you ask I
>> want to measure the cost of swapping out of a guest memory.
>
> No. if you stop to use mlockall, the issue is vanished.
>

And other issues arise. Gleb described a situation where the use of
mlockall is justified, identified an issue which prevents its use, and
provided a patch which resolves that issue. Why are you focusing on
the validity of using mlockall?

>
>> > - If this feature added, average distro user can get any benefit?
>> >
>> ?! Is this some kind of new measure? There are plenty of much more
>> invasive features that don't bring benefits to an average distro user.
>> This feature can bring benefit to embedded/RT developers.
>
> I mean who get benifit?
>
>
>> > I mean, many application developrs want to add their specific feature
>> > into kernel. but if we allow it unlimitedly, major syscall become
>> > the trushbox of pretty toy feature soon.
>> >
>> And if application developer wants to extend kernel in a way that it
>> will be possible to do something that was not possible before why is
>> this a bad thing? I would agree with you if for my problem was userspace
>> solution, but there is none. The mmap interface is asymmetric in regards
>> to mlock currently. There is MAP_LOCKED, but no MAP_UNLOCKED. Why
>> MAP_LOCKED is useful then?
>
> Why? Because this is formal LKML reviewing process. I'm reviewing your
> patch for YOU.
>
> If there is no objective reason, I don't want to continue reviewing.
>

There is an objective reason: the current interaction between
mlockall(MCL_FUTURE) and mmap has a deficiency. In 'normal' mode,
without MCL_FUTURE in force, the default is that new memory mappings
are not locked, but mmap provides MAP_LOCKED specifically to override
that default. However, with MCL_FUTURE toggled to on, there is no
analogous way to tell mmap to override the default. The proposed
MAP_UNLOCKED flag would resolve this deficiency.

Andrew

2010-01-18 03:23:21

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

> On Thu, Jan 14, 2010 at 3:17 AM, KOSAKI Motohiro
> <[email protected]> wrote:
> >> > Hmm..
> >> > Your answer didn't match I wanted.
> >> Then I don't get what you want.
> >
> > I want to know the benefit of the patch for patch reviewing.
> >
>
> The benefit of the patch is that it makes it possible for an
> application which has previously called mlockall(MCL_FUTURE) to
> selectively exempt new memory mappings from memory locking, on a
> per-mmap-call basis. As was pointed out earlier, there is currently no
> thread-safe way for an application to do this. The earlier proposed
> workaround of toggling MCL_FUTURE around calls to mmap is racy in a
> multi-threaded context. Other threads may manipulate the address space
> during the window where MCL_FUTURE is off, subverting the programmers
> intended memory locking semantics.
>
> The ability to exempt specific memory mappings from memory locking is
> necessary when the region to be mapped is larger than physical memory.
> In such cases a call to mmap the region cannot succeed, unless
> MAP_UNLOCKED is available.
>
>
> >
> >> > few additional questions.
> >> >
> >> > - Why don't you change your application? It seems natural way than kernel change.
> >> There is no way to change my application and achieve what I've described
> >> in a multithreaded app.
> >
> > Then, we don't recommend to use mlockall(). I don't hope to hear your conclusion,
> > it is not objectivization. I hope to hear why you reached such conclusion.
> >
>
> I agree that mlockall is a big hammer and should be avoided in most
> cases, but there are situations where it is exactly what is needed. In
> Gleb's instance, it sounds like he is doing some finicky performance
> measurement and major page faults skew his results. In my case, I have
> a realtime process where the measured latency impact of major page
> faults is unacceptable. In both of these cases, mlockall is a
> reasonable approach to eliminating major faults.
>
> However, Gleb and I have independently found ourselves unable to use
> mlockall because we also need to create a very large memory mapping
> (for which we don't care about major faults). The proposed
> MAP_UNLOCKED flag would allow us to override MCL_FUTURE for that one
> mapping.
>
> >
> >> > - Why do you want your virtual machine have mlockall? AFAIK, current majority
> >> > ? virtual machine doesn't.
> >> It is absolutely irrelevant for that patch, but just because you ask I
> >> want to measure the cost of swapping out of a guest memory.
> >
> > No. if you stop to use mlockall, the issue is vanished.
> >
>
> And other issues arise. Gleb described a situation where the use of
> mlockall is justified, identified an issue which prevents its use, and
> provided a patch which resolves that issue. Why are you focusing on
> the validity of using mlockall?
>
> >
> >> > - If this feature added, average distro user can get any benefit?
> >> >
> >> ?! Is this some kind of new measure? There are plenty of much more
> >> invasive features that don't bring benefits to an average distro user.
> >> This feature can bring benefit to embedded/RT developers.
> >
> > I mean who get benifit?
> >
> >
> >> > I mean, many application developrs want to add their specific feature
> >> > into kernel. but if we allow it unlimitedly, major syscall become
> >> > the trushbox of pretty toy feature soon.
> >> >
> >> And if application developer wants to extend kernel in a way that it
> >> will be possible to do something that was not possible before why is
> >> this a bad thing? I would agree with you if for my problem was userspace
> >> solution, but there is none. The mmap interface is asymmetric in regards
> >> to mlock currently. There is MAP_LOCKED, but no MAP_UNLOCKED. Why
> >> MAP_LOCKED is useful then?
> >
> > Why? Because this is formal LKML reviewing process. I'm reviewing your
> > patch for YOU.
> >
> > If there is no objective reason, I don't want to continue reviewing.
> >
>
> There is an objective reason: the current interaction between
> mlockall(MCL_FUTURE) and mmap has a deficiency. In 'normal' mode,
> without MCL_FUTURE in force, the default is that new memory mappings
> are not locked, but mmap provides MAP_LOCKED specifically to override
> that default. However, with MCL_FUTURE toggled to on, there is no
> analogous way to tell mmap to override the default. The proposed
> MAP_UNLOCKED flag would resolve this deficiency.

Very thank you, Andrew!

Your explanation help me lots rather than original patch description. OK, At least
MAP_UNLOCED have two users (you and gleb) and your explanation seems
makes sense.

So, if gleb resend this patch with rewrited description, I might take my reviewed-by tag to it, probagly.


Thanks.

2010-01-18 13:40:59

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v5] add MAP_UNLOCKED mmap flag

On Mon, Jan 18, 2010 at 12:23:09PM +0900, KOSAKI Motohiro wrote:
> > On Thu, Jan 14, 2010 at 3:17 AM, KOSAKI Motohiro
> > <[email protected]> wrote:
> > >> > Hmm..
> > >> > Your answer didn't match I wanted.
> > >> Then I don't get what you want.
> > >
> > > I want to know the benefit of the patch for patch reviewing.
> > >
> >
> > The benefit of the patch is that it makes it possible for an
> > application which has previously called mlockall(MCL_FUTURE) to
> > selectively exempt new memory mappings from memory locking, on a
> > per-mmap-call basis. As was pointed out earlier, there is currently no
> > thread-safe way for an application to do this. The earlier proposed
> > workaround of toggling MCL_FUTURE around calls to mmap is racy in a
> > multi-threaded context. Other threads may manipulate the address space
> > during the window where MCL_FUTURE is off, subverting the programmers
> > intended memory locking semantics.
> >
> > The ability to exempt specific memory mappings from memory locking is
> > necessary when the region to be mapped is larger than physical memory.
> > In such cases a call to mmap the region cannot succeed, unless
> > MAP_UNLOCKED is available.
> >
> >
> > >
> > >> > few additional questions.
> > >> >
> > >> > - Why don't you change your application? It seems natural way than kernel change.
> > >> There is no way to change my application and achieve what I've described
> > >> in a multithreaded app.
> > >
> > > Then, we don't recommend to use mlockall(). I don't hope to hear your conclusion,
> > > it is not objectivization. I hope to hear why you reached such conclusion.
> > >
> >
> > I agree that mlockall is a big hammer and should be avoided in most
> > cases, but there are situations where it is exactly what is needed. In
> > Gleb's instance, it sounds like he is doing some finicky performance
> > measurement and major page faults skew his results. In my case, I have
> > a realtime process where the measured latency impact of major page
> > faults is unacceptable. In both of these cases, mlockall is a
> > reasonable approach to eliminating major faults.
> >
> > However, Gleb and I have independently found ourselves unable to use
> > mlockall because we also need to create a very large memory mapping
> > (for which we don't care about major faults). The proposed
> > MAP_UNLOCKED flag would allow us to override MCL_FUTURE for that one
> > mapping.
> >
> > >
> > >> > - Why do you want your virtual machine have mlockall? AFAIK, current majority
> > >> > ? virtual machine doesn't.
> > >> It is absolutely irrelevant for that patch, but just because you ask I
> > >> want to measure the cost of swapping out of a guest memory.
> > >
> > > No. if you stop to use mlockall, the issue is vanished.
> > >
> >
> > And other issues arise. Gleb described a situation where the use of
> > mlockall is justified, identified an issue which prevents its use, and
> > provided a patch which resolves that issue. Why are you focusing on
> > the validity of using mlockall?
> >
> > >
> > >> > - If this feature added, average distro user can get any benefit?
> > >> >
> > >> ?! Is this some kind of new measure? There are plenty of much more
> > >> invasive features that don't bring benefits to an average distro user.
> > >> This feature can bring benefit to embedded/RT developers.
> > >
> > > I mean who get benifit?
> > >
> > >
> > >> > I mean, many application developrs want to add their specific feature
> > >> > into kernel. but if we allow it unlimitedly, major syscall become
> > >> > the trushbox of pretty toy feature soon.
> > >> >
> > >> And if application developer wants to extend kernel in a way that it
> > >> will be possible to do something that was not possible before why is
> > >> this a bad thing? I would agree with you if for my problem was userspace
> > >> solution, but there is none. The mmap interface is asymmetric in regards
> > >> to mlock currently. There is MAP_LOCKED, but no MAP_UNLOCKED. Why
> > >> MAP_LOCKED is useful then?
> > >
> > > Why? Because this is formal LKML reviewing process. I'm reviewing your
> > > patch for YOU.
> > >
> > > If there is no objective reason, I don't want to continue reviewing.
> > >
> >
> > There is an objective reason: the current interaction between
> > mlockall(MCL_FUTURE) and mmap has a deficiency. In 'normal' mode,
> > without MCL_FUTURE in force, the default is that new memory mappings
> > are not locked, but mmap provides MAP_LOCKED specifically to override
> > that default. However, with MCL_FUTURE toggled to on, there is no
> > analogous way to tell mmap to override the default. The proposed
> > MAP_UNLOCKED flag would resolve this deficiency.
>
> Very thank you, Andrew!
>
> Your explanation help me lots rather than original patch description. OK, At least
> MAP_UNLOCED have two users (you and gleb) and your explanation seems
> makes sense.
>
> So, if gleb resend this patch with rewrited description, I might take my reviewed-by tag to it, probagly.
>
Just did it. I hope the commit message is OK with you now. Its text is
taken from this Andrew's mail. Thanks.

--
Gleb.