[PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Kirill A. Shutemov]

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 arch/x86/kernel/sys_x86_64.c |   15 +++++++++++----
 include/asm-x86/elf.h        |    4 +++-
 include/asm-x86/processor.h  |    6 ++++--
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 3b360ef..d6ac928 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -48,7 +48,9 @@ out:
 static void find_start_end(unsigned long flags, unsigned long *begin,
 			   unsigned long *end)
 {
-	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT)) {
+	if (!test_thread_flag(TIF_IA32) &&
+	    ((flags & MAP_32BIT) ||
+	     (current->personality & ADDR_LIMIT_32BIT))) {
 		unsigned long new_begin;
 		/* This is usually used needed to map code in small
 		   model, so it needs to be in the first 31bit. Limit
@@ -94,7 +96,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 		    (!vma || addr + len <= vma->vm_start))
 			return addr;
 	}
-	if (((flags & MAP_32BIT) || test_thread_flag(TIF_IA32))
+	if (((flags & MAP_32BIT) || test_thread_flag(TIF_IA32) ||
+	     (current->personality & ADDR_LIMIT_32BIT))
 	    && len <= mm->cached_hole_size) {
 	        mm->cached_hole_size = 0;
 		mm->free_area_cache = begin;
@@ -150,8 +153,12 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	if (flags & MAP_FIXED)
 		return addr;
 
-	/* for MAP_32BIT mappings we force the legact mmap base */
-	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT))
+	/* for MAP_32BIT mappings and ADDR_LIMIT_32BIT personality we force the
+	 * legact mmap base
+	 */
+	if (!test_thread_flag(TIF_IA32) &&
+	    ((flags & MAP_32BIT) ||
+	     (current->personality & ADDR_LIMIT_32BIT)))
 		goto bottomup;
 
 	/* requesting a specific address */
diff --git a/include/asm-x86/elf.h b/include/asm-x86/elf.h
index 7be4733..fa39e10 100644
--- a/include/asm-x86/elf.h
+++ b/include/asm-x86/elf.h
@@ -298,7 +298,9 @@ do {									\
 #define VDSO_HIGH_BASE		0xffffe000U /* CONFIG_COMPAT_VDSO address */
 
 /* 1GB for 64bit, 8MB for 32bit */
-#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
+#define STACK_RND_MASK ((test_thread_flag(TIF_IA32) || \
+			 current->personality & ADDR_LIMIT_32BIT ) ? \
+			0x7ff : 0x3fffff)
 
 #define ARCH_DLINFO							\
 do {									\
diff --git a/include/asm-x86/processor.h b/include/asm-x86/processor.h
index 4df3e2f..6d7f2f9 100644
--- a/include/asm-x86/processor.h
+++ b/include/asm-x86/processor.h
@@ -904,7 +904,8 @@ extern unsigned long thread_saved_pc(struct task_struct *tsk);
 #define TASK_SIZE_OF(child)	((test_tsk_thread_flag(child, TIF_IA32)) ? \
 					IA32_PAGE_OFFSET : TASK_SIZE64)
 
-#define STACK_TOP		TASK_SIZE
+#define STACK_TOP		(current->personality & ADDR_LIMIT_32BIT ? \
+					 0x80000000 : TASK_SIZE)
 #define STACK_TOP_MAX		TASK_SIZE64
 
 #define INIT_THREAD  { \
@@ -932,7 +933,8 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
  * This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
  */
-#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 3))
+#define TASK_UNMAPPED_BASE	(current->personality & ADDR_LIMIT_32BIT ? \
+					0x40000000 : PAGE_ALIGN(TASK_SIZE / 3))
 
 #define KSTK_EIP(task)		(task_pt_regs(task)->ip)
 
-- 
1.5.6.5.GIT

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Ingo Molnar]

* Kirill A. Shutemov <kirill@shutemov.name> wrote:

> -	/* for MAP_32BIT mappings we force the legact mmap base */
> -	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT))
> +	/* for MAP_32BIT mappings and ADDR_LIMIT_32BIT personality we force the
> +	 * legact mmap base
> +	 */

please use the customary multi-line comment style:

  /*
   * Comment .....
   * ...... goes here:
   */

and you might use the opportunity to fix the s/legact/legacy typo as 
well.

but more generally, we already have ADDR_LIMIT_3GB support on x86. Why 
should support for ADDR_LIMIT_32BIT be added?

	Ingo

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]

On Fri, Oct 03, 2008 at 10:02:44AM +0200, Ingo Molnar wrote:
> 
> * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> 
> > -	/* for MAP_32BIT mappings we force the legact mmap base */
> > -	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT))
> > +	/* for MAP_32BIT mappings and ADDR_LIMIT_32BIT personality we force the
> > +	 * legact mmap base
> > +	 */
> 
> please use the customary multi-line comment style:
> 
>   /*
>    * Comment .....
>    * ...... goes here:
>    */
> 
> and you might use the opportunity to fix the s/legact/legacy typo as 
> well.

Ok, I'll fix it.

> 
> but more generally, we already have ADDR_LIMIT_3GB support on x86.

Does ADDR_LIMIT_3GB really work?

$ cat 1.c
#include <stdio.h>
#include <sys/personality.h>
#include <sys/ipc.h>
#include <sys/shm.h>

#define ADDR_LIMIT_3GB 0x8000000

int main(void)
{
        int id;
        void *shm;

        personality(ADDR_LIMIT_3GB);

        id = shmget(0x123456, 1, IPC_CREAT | 0600);
        shm = shmat(id, NULL, 0);
        printf("shm: %p\n", shm);
        shmdt(shm);

        return 0;
}
$ gcc -Wall 1.c
$ sudo ./a.out 
shm: 0x7f4fca755000

> Why 
> should support for ADDR_LIMIT_32BIT be added?

It's useful for user mode qemu when you try emulate 32-bit target on 
x86_64. For example, if shmat(2) return addres above 32-bit, target will
get SIGSEGV on access to it.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Kirill A. Shutemov]

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 arch/x86/kernel/sys_x86_64.c |   16 ++++++++++++----
 include/asm-x86/elf.h        |    4 +++-
 include/asm-x86/processor.h  |    6 ++++--
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 3b360ef..7f8672d 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -48,7 +48,9 @@ out:
 static void find_start_end(unsigned long flags, unsigned long *begin,
 			   unsigned long *end)
 {
-	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT)) {
+	if (!test_thread_flag(TIF_IA32) &&
+	    ((flags & MAP_32BIT) ||
+	     (current->personality & ADDR_LIMIT_32BIT))) {
 		unsigned long new_begin;
 		/* This is usually used needed to map code in small
 		   model, so it needs to be in the first 31bit. Limit
@@ -94,7 +96,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 		    (!vma || addr + len <= vma->vm_start))
 			return addr;
 	}
-	if (((flags & MAP_32BIT) || test_thread_flag(TIF_IA32))
+	if (((flags & MAP_32BIT) || test_thread_flag(TIF_IA32) ||
+	     (current->personality & ADDR_LIMIT_32BIT))
 	    && len <= mm->cached_hole_size) {
 	        mm->cached_hole_size = 0;
 		mm->free_area_cache = begin;
@@ -150,8 +153,13 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
 	if (flags & MAP_FIXED)
 		return addr;
 
-	/* for MAP_32BIT mappings we force the legact mmap base */
-	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT))
+	/*
+	 * for MAP_32BIT mappings and ADDR_LIMIT_32BIT personality we force the
+	 * legacy mmap base
+	 */
+	if (!test_thread_flag(TIF_IA32) &&
+	    ((flags & MAP_32BIT) ||
+	     (current->personality & ADDR_LIMIT_32BIT)))
 		goto bottomup;
 
 	/* requesting a specific address */
diff --git a/include/asm-x86/elf.h b/include/asm-x86/elf.h
index 7be4733..fa39e10 100644
--- a/include/asm-x86/elf.h
+++ b/include/asm-x86/elf.h
@@ -298,7 +298,9 @@ do {									\
 #define VDSO_HIGH_BASE		0xffffe000U /* CONFIG_COMPAT_VDSO address */
 
 /* 1GB for 64bit, 8MB for 32bit */
-#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
+#define STACK_RND_MASK ((test_thread_flag(TIF_IA32) || \
+			 current->personality & ADDR_LIMIT_32BIT ) ? \
+			0x7ff : 0x3fffff)
 
 #define ARCH_DLINFO							\
 do {									\
diff --git a/include/asm-x86/processor.h b/include/asm-x86/processor.h
index 4df3e2f..6d7f2f9 100644
--- a/include/asm-x86/processor.h
+++ b/include/asm-x86/processor.h
@@ -904,7 +904,8 @@ extern unsigned long thread_saved_pc(struct task_struct *tsk);
 #define TASK_SIZE_OF(child)	((test_tsk_thread_flag(child, TIF_IA32)) ? \
 					IA32_PAGE_OFFSET : TASK_SIZE64)
 
-#define STACK_TOP		TASK_SIZE
+#define STACK_TOP		(current->personality & ADDR_LIMIT_32BIT ? \
+					 0x80000000 : TASK_SIZE)
 #define STACK_TOP_MAX		TASK_SIZE64
 
 #define INIT_THREAD  { \
@@ -932,7 +933,8 @@ extern void start_thread(struct pt_regs *regs, unsigned long new_ip,
  * This decides where the kernel will search for a free chunk of vm
  * space during mmap's.
  */
-#define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 3))
+#define TASK_UNMAPPED_BASE	(current->personality & ADDR_LIMIT_32BIT ? \
+					0x40000000 : PAGE_ALIGN(TASK_SIZE / 3))
 
 #define KSTK_EIP(task)		(task_pt_regs(task)->ip)
 
-- 
1.5.6.5.GIT

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Arjan van de Ven]

On Fri, 3 Oct 2008 12:25:52 +0300
"Kirill A. Shutemov" <kirill@shutemov.name> wrote:

> On Fri, Oct 03, 2008 at 10:02:44AM +0200, Ingo Molnar wrote:
> > 
> > * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> > 
> > > -	/* for MAP_32BIT mappings we force the legact mmap base
> > > */
> > > -	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT))
> > > +	/* for MAP_32BIT mappings and ADDR_LIMIT_32BIT
> > > personality we force the
> > > +	 * legact mmap base
> > > +	 */
> > 
> > please use the customary multi-line comment style:
> > 
> >   /*
> >    * Comment .....
> >    * ...... goes here:
> >    */
> > 
> > and you might use the opportunity to fix the s/legact/legacy typo
> > as well.
> 
> Ok, I'll fix it.
> 
> > 
> > but more generally, we already have ADDR_LIMIT_3GB support on x86.
> 
> Does ADDR_LIMIT_3GB really work?

if it's broken we should fix it.... not invent a new one.
Also, traditionally often personalities only start at exec() time iirc.
(but I could be wrong on that)

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]

On Fri, Oct 03, 2008 at 05:44:31AM -0700, Arjan van de Ven wrote:
> On Fri, 3 Oct 2008 12:25:52 +0300
> "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> 
> > On Fri, Oct 03, 2008 at 10:02:44AM +0200, Ingo Molnar wrote:
> > > 
> > > * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> > > 
> > > > -	/* for MAP_32BIT mappings we force the legact mmap base
> > > > */
> > > > -	if (!test_thread_flag(TIF_IA32) && (flags & MAP_32BIT))
> > > > +	/* for MAP_32BIT mappings and ADDR_LIMIT_32BIT
> > > > personality we force the
> > > > +	 * legact mmap base
> > > > +	 */
> > > 
> > > please use the customary multi-line comment style:
> > > 
> > >   /*
> > >    * Comment .....
> > >    * ...... goes here:
> > >    */
> > > 
> > > and you might use the opportunity to fix the s/legact/legacy typo
> > > as well.
> > 
> > Ok, I'll fix it.
> > 
> > > 
> > > but more generally, we already have ADDR_LIMIT_3GB support on x86.
> > 
> > Does ADDR_LIMIT_3GB really work?
> 
> if it's broken we should fix it.... not invent a new one.
> Also, traditionally often personalities only start at exec() time iirc.
> (but I could be wrong on that)

What is difference beetween ADDR_LIMIT_3GB and ADDR_LIMIT_32BIT? Probably,
I implement ADDR_LIMIT_3GB, not ADDR_LIMIT_32BIT...

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Andi Kleen]

"Kirill A. Shutemov" <kirill@shutemov.name> writes:
>
>> 
>> but more generally, we already have ADDR_LIMIT_3GB support on x86.
>
> Does ADDR_LIMIT_3GB really work?

As Arjan pointed out it only takes effect on exec()

andi@basil:~/tsrc> cat tstack2.c
#include <stdio.h>
int main(void)
{
        void *p = &p;
        printf("%p\n", &p);
        return 0;
}
andi@basil:~/tsrc> gcc -m32 tstack2.c  -o tstack2
andi@basil:~/tsrc> ./tstack2 
0xff807d70
andi@basil:~/tsrc> linux32 --3gb ./tstack2 
0xbfae2840

>> Why 
>> should support for ADDR_LIMIT_32BIT be added?
>
> It's useful for user mode qemu when you try emulate 32-bit target on 
> x86_64. For example, if shmat(2) return addres above 32-bit, target will
> get SIGSEGV on access to it.

The traditional way in mmap() to handle this is to give it a search
hint < 4GB and then free the memory again/fail if the result was >4GB.

Unfortunately that doesn't work for shmat() because the address argument
is not a search hint, but a fixed address. 

I presume you need this for the qemu syscall emulation. For a standard
application I would just recommend to use mmap with tmpfs instead
(sysv shm is kind of obsolete). For shmat() emulation the cleanest way
would be probably to add a new flag to shmat() that says that address
is a search hint, not a fixed address. Then implement it the way recommended
above.

-Andi

-- 
ak@linux.intel.com

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

On Mon, Oct 06, 2008 at 08:13:19AM +0200, Andi Kleen wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
> >
> >> 
> >> but more generally, we already have ADDR_LIMIT_3GB support on x86.
> >
> > Does ADDR_LIMIT_3GB really work?
> 
> As Arjan pointed out it only takes effect on exec()
> 
> andi@basil:~/tsrc> cat tstack2.c
> #include <stdio.h>
> int main(void)
> {
>         void *p = &p;
>         printf("%p\n", &p);
>         return 0;
> }
> andi@basil:~/tsrc> gcc -m32 tstack2.c  -o tstack2
> andi@basil:~/tsrc> ./tstack2 
> 0xff807d70
> andi@basil:~/tsrc> linux32 --3gb ./tstack2 
> 0xbfae2840

Which kernel do you use?
Does it work only when compiled with -m32?

$ cat 1.c
#include <stdio.h>
int main(void)
{
        void *p = &p;
        printf("%p\n", &p);
        return 0;
}
$ gcc 1.c
$ linux32 --3gb ./a.out
0x7fffa667e7b8

> >> Why 
> >> should support for ADDR_LIMIT_32BIT be added?
> >
> > It's useful for user mode qemu when you try emulate 32-bit target on 
> > x86_64. For example, if shmat(2) return addres above 32-bit, target will
> > get SIGSEGV on access to it.
> 
> The traditional way in mmap() to handle this is to give it a search
> hint < 4GB and then free the memory again/fail if the result was >4GB.

mmap() has MAP_32BIT flag on x86_64.

> Unfortunately that doesn't work for shmat() because the address argument
> is not a search hint, but a fixed address. 
> 
> I presume you need this for the qemu syscall emulation. For a standard
> application I would just recommend to use mmap with tmpfs instead
> (sysv shm is kind of obsolete). For shmat() emulation the cleanest way
> would be probably to add a new flag to shmat() that says that address
> is a search hint, not a fixed address. Then implement it the way recommended
> above.

I prefer one handle to switch application to 32-bit address mode. Why is it
wrong?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Andi Kleen]

On Mon, Oct 06, 2008 at 11:17:23AM +0300, Kirill A. Shutemov wrote:
> On Mon, Oct 06, 2008 at 08:13:19AM +0200, Andi Kleen wrote:
> > "Kirill A. Shutemov" <kirill@shutemov.name> writes:
> > >
> > >> 
> > >> but more generally, we already have ADDR_LIMIT_3GB support on x86.
> > >
> > > Does ADDR_LIMIT_3GB really work?
> > 
> > As Arjan pointed out it only takes effect on exec()
> > 
> > andi@basil:~/tsrc> cat tstack2.c
> > #include <stdio.h>
> > int main(void)
> > {
> >         void *p = &p;
> >         printf("%p\n", &p);
> >         return 0;
> > }
> > andi@basil:~/tsrc> gcc -m32 tstack2.c  -o tstack2
> > andi@basil:~/tsrc> ./tstack2 
> > 0xff807d70
> > andi@basil:~/tsrc> linux32 --3gb ./tstack2 
> > 0xbfae2840
> 
> Which kernel do you use?

This was 2.6.26 (+ some irrelevant patches)

> Does it work only when compiled with -m32?

Yes. For 64bit processes you use the method described below for mmap.

> mmap() has MAP_32BIT flag on x86_64.

MAP_32BIT is just a short form for this, it's internally the same.
But it's actually MAP_31BIT. If you want the full 4GB you should not use it.

> 
> > Unfortunately that doesn't work for shmat() because the address argument
> > is not a search hint, but a fixed address. 
> > 
> > I presume you need this for the qemu syscall emulation. For a standard
> > application I would just recommend to use mmap with tmpfs instead
> > (sysv shm is kind of obsolete). For shmat() emulation the cleanest way
> > would be probably to add a new flag to shmat() that says that address
> > is a search hint, not a fixed address. Then implement it the way recommended
> > above.
> 
> I prefer one handle to switch application to 32-bit address mode. Why is it
> wrong?

"32 bit mode" really has to be set at exec time, otherwise it is not
(e.g. the stack will be beyond).

And personality() is not thread local/safe, so it's not a particularly
good interface to use later. Per system call switches are preferable
and more flexible.

-Andi
-- 
ak@linux.intel.com

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

On Mon, Oct 06, 2008 at 10:42:46AM +0200, Andi Kleen wrote:
> On Mon, Oct 06, 2008 at 11:17:23AM +0300, Kirill A. Shutemov wrote:
> > On Mon, Oct 06, 2008 at 08:13:19AM +0200, Andi Kleen wrote:
> > > Unfortunately that doesn't work for shmat() because the address argument
> > > is not a search hint, but a fixed address. 
> > > 
> > > I presume you need this for the qemu syscall emulation. For a standard
> > > application I would just recommend to use mmap with tmpfs instead
> > > (sysv shm is kind of obsolete). For shmat() emulation the cleanest way
> > > would be probably to add a new flag to shmat() that says that address
> > > is a search hint, not a fixed address. Then implement it the way recommended
> > > above.
> > 
> > I prefer one handle to switch application to 32-bit address mode. Why is it
> > wrong?
> 
> "32 bit mode" really has to be set at exec time, otherwise it is not
> (e.g. the stack will be beyond).

Stack isn't a problem for qemu. qemu allocate stack for target application
by itself.

> And personality() is not thread local/safe, so it's not a particularly
> good interface to use later.

qemu can call personality() before any threads will be created.

> Per system call switches are preferable
> and more flexible.

Per syscall switches are cool, but I don't see any advantage from it for 
qemu.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Andi Kleen]

> > And personality() is not thread local/safe, so it's not a particularly
> > good interface to use later.
> 
> qemu can call personality() before any threads will be created.

It still makes it unsuitable for a lot of other applications.
e.g. a JVM using 32bit pointers couldn't use it because it would
affect native C threads running in the same process.

> 
> > Per system call switches are preferable
> > and more flexible.
> 
> Per syscall switches are cool, but I don't see any advantage from it for 
> qemu.

Linux interfaces are not supposed to be "interfaces for qemu" but generally
applicable interfaces.

-Andi
-- 
ak@linux.intel.com

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On Mon, Oct 06, 2008 at 11:56:28AM +0200, Andi Kleen wrote:
> > > And personality() is not thread local/safe, so it's not a particularly
> > > good interface to use later.
> > 
> > qemu can call personality() before any threads will be created.
> 
> It still makes it unsuitable for a lot of other applications.
> e.g. a JVM using 32bit pointers couldn't use it because it would
> affect native C threads running in the same process.
> 
> > 
> > > Per system call switches are preferable
> > > and more flexible.
> > 
> > Per syscall switches are cool, but I don't see any advantage from it for 
> > qemu.
> 
> Linux interfaces are not supposed to be "interfaces for qemu" but generally
> applicable interfaces.

I know. What about adding both personality() and flag for shmat()? I can
prepare patch that implement flag for shmat().

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] x86_64: Implement personality ADDR_LIMIT_32BIT

[Andi Kleen]

> > Linux interfaces are not supposed to be "interfaces for qemu" but generally
> > applicable interfaces.
> 
> I know. What about adding both personality() and flag for shmat()? I can
> prepare patch that implement flag for shmat().

It would be better to just fix all calls in qemu than
to add a new personality. There aren't that many anyways.

personality is really more a kludge for bug-to-bug compatibility
with old binaries (that is where the 3GB personality came from
to work around bugs in some old JVMs that could not deal with a full 4GB
address space), it shouldn't be really used for anything new. 

-Andi
-- 
ak@linux.intel.com

[PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

It allows interpret attach address as a hint, not as exact address.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arjan van de Ven <arjan@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/shm.h |    1 +
 ipc/shm.c           |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/shm.h b/include/linux/shm.h
index eca6235..2a637b8 100644
--- a/include/linux/shm.h
+++ b/include/linux/shm.h
@@ -55,6 +55,7 @@ struct shmid_ds {
 #define	SHM_RND		020000	/* round attach address to SHMLBA boundary */
 #define	SHM_REMAP	040000	/* take-over region on attach */
 #define	SHM_EXEC	0100000	/* execution access */
+#define	SHM_MAP_HINT	0200000	/* interpret attach address as a hint */
 
 /* super user shmctl commands */
 #define SHM_LOCK 	11
diff --git a/ipc/shm.c b/ipc/shm.c
index e77ec69..19462bb 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -819,7 +819,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
 	if (shmid < 0)
 		goto out;
 	else if ((addr = (ulong)shmaddr)) {
-		if (addr & (SHMLBA-1)) {
+		if (!(shmflg & SHM_MAP_HINT) && (addr & (SHMLBA-1))) {
 			if (shmflg & SHM_RND)
 				addr &= ~(SHMLBA-1);	   /* round down */
 			else
@@ -828,7 +828,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
 #endif
 					goto out;
 		}
-		flags = MAP_SHARED | MAP_FIXED;
+		flags = (shmflg & SHM_MAP_HINT ? 0 : MAP_FIXED) | MAP_SHARED;
 	} else {
 		if ((shmflg & SHM_REMAP))
 			goto out;
-- 
1.5.6.5.GIT

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Andi Kleen]

On Mon, Oct 06, 2008 at 05:37:59PM +0300, Kirill A. Shutemov wrote:
> It allows interpret attach address as a hint, not as exact address.

First you should also do a patch for the manpage and send to 
the manpage maintainer.


>  #define SHM_LOCK 	11
> diff --git a/ipc/shm.c b/ipc/shm.c
> index e77ec69..19462bb 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -819,7 +819,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
>  	if (shmid < 0)
>  		goto out;
>  	else if ((addr = (ulong)shmaddr)) {
> -		if (addr & (SHMLBA-1)) {
> +		if (!(shmflg & SHM_MAP_HINT) && (addr & (SHMLBA-1))) {
>  			if (shmflg & SHM_RND)
>  				addr &= ~(SHMLBA-1);	   /* round down */
>  			else
> @@ -828,7 +828,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
>  #endif
>  					goto out;
>  		}
> -		flags = MAP_SHARED | MAP_FIXED;
> +		flags = (shmflg & SHM_MAP_HINT ? 0 : MAP_FIXED) | MAP_SHARED;


IMHO you need at least make the

   if (find_vma_intersection(current->mm, addr, addr + size))
                        goto invalid;

test above conditional too.

-Andi

-- 
ak@linux.intel.com

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

On Mon, Oct 06, 2008 at 09:29:23PM +0200, Andi Kleen wrote:
> On Mon, Oct 06, 2008 at 05:37:59PM +0300, Kirill A. Shutemov wrote:
> > It allows interpret attach address as a hint, not as exact address.
> 
> First you should also do a patch for the manpage and send to 
> the manpage maintainer.

I'll do it if the patch is ok.

> >  #define SHM_LOCK 	11
> > diff --git a/ipc/shm.c b/ipc/shm.c
> > index e77ec69..19462bb 100644
> > --- a/ipc/shm.c
> > +++ b/ipc/shm.c
> > @@ -819,7 +819,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
> >  	if (shmid < 0)
> >  		goto out;
> >  	else if ((addr = (ulong)shmaddr)) {
> > -		if (addr & (SHMLBA-1)) {
> > +		if (!(shmflg & SHM_MAP_HINT) && (addr & (SHMLBA-1))) {
> >  			if (shmflg & SHM_RND)
> >  				addr &= ~(SHMLBA-1);	   /* round down */
> >  			else
> > @@ -828,7 +828,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
> >  #endif
> >  					goto out;
> >  		}
> > -		flags = MAP_SHARED | MAP_FIXED;
> > +		flags = (shmflg & SHM_MAP_HINT ? 0 : MAP_FIXED) | MAP_SHARED;
> 
> 
> IMHO you need at least make the
> 
>    if (find_vma_intersection(current->mm, addr, addr + size))
>                         goto invalid;
> 
> test above conditional too.

Since it's a hint, we shouldn't call find_vma_intersection() at all.

I'll send fixed patch soon.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH, RFC, v2] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

It allows interpret attach address as a hint, not as exact address.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arjan van de Ven <arjan@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/shm.h |    1 +
 ipc/shm.c           |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/shm.h b/include/linux/shm.h
index eca6235..2a637b8 100644
--- a/include/linux/shm.h
+++ b/include/linux/shm.h
@@ -55,6 +55,7 @@ struct shmid_ds {
 #define	SHM_RND		020000	/* round attach address to SHMLBA boundary */
 #define	SHM_REMAP	040000	/* take-over region on attach */
 #define	SHM_EXEC	0100000	/* execution access */
+#define	SHM_MAP_HINT	0200000	/* interpret attach address as a hint */
 
 /* super user shmctl commands */
 #define SHM_LOCK 	11
diff --git a/ipc/shm.c b/ipc/shm.c
index e77ec69..765de74 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -819,7 +819,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
 	if (shmid < 0)
 		goto out;
 	else if ((addr = (ulong)shmaddr)) {
-		if (addr & (SHMLBA-1)) {
+		if (!(shmflg & SHM_MAP_HINT) && (addr & (SHMLBA-1))) {
 			if (shmflg & SHM_RND)
 				addr &= ~(SHMLBA-1);	   /* round down */
 			else
@@ -828,7 +828,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
 #endif
 					goto out;
 		}
-		flags = MAP_SHARED | MAP_FIXED;
+		flags = (shmflg & SHM_MAP_HINT ? 0 : MAP_FIXED) | MAP_SHARED;
 	} else {
 		if ((shmflg & SHM_REMAP))
 			goto out;
@@ -892,7 +892,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
 	sfd->vm_ops = NULL;
 
 	down_write(&current->mm->mmap_sem);
-	if (addr && !(shmflg & SHM_REMAP)) {
+	if (addr && !(shmflg & (SHM_REMAP|SHM_MAP_HINT))) {
 		err = -EINVAL;
 		if (find_vma_intersection(current->mm, addr, addr + size))
 			goto invalid;
-- 
1.5.6.5.GIT

Re: [PATCH, RFC, v2] shmat: introduce flag SHM_MAP_HINT

[Andi Kleen]

On Tue, Oct 07, 2008 at 09:57:50AM +0300, Kirill A. Shutemov wrote:
> It allows interpret attach address as a hint, not as exact address.

Please expand the description a bit. Rationale. etc.

> @@ -55,6 +55,7 @@ struct shmid_ds {
>  #define	SHM_RND		020000	/* round attach address to SHMLBA boundary */
>  #define	SHM_REMAP	040000	/* take-over region on attach */
>  #define	SHM_EXEC	0100000	/* execution access */
> +#define	SHM_MAP_HINT	0200000	/* interpret attach address as a hint */

search hint

> @@ -892,7 +892,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
>  	sfd->vm_ops = NULL;
>  
>  	down_write(&current->mm->mmap_sem);
> -	if (addr && !(shmflg & SHM_REMAP)) {
> +	if (addr && !(shmflg & (SHM_REMAP|SHM_MAP_HINT))) {

I think you were right earlier that it can be just deleted, so why don't
you just do that?

-Andi

-- 
ak@linux.intel.com

Re: [PATCH, RFC, v2] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1232 bytes --]

On Tue, Oct 07, 2008 at 10:20:30AM +0200, Andi Kleen wrote:
> On Tue, Oct 07, 2008 at 09:57:50AM +0300, Kirill A. Shutemov wrote:
> > It allows interpret attach address as a hint, not as exact address.
> 
> Please expand the description a bit. Rationale. etc.
> 
> > @@ -55,6 +55,7 @@ struct shmid_ds {
> >  #define	SHM_RND		020000	/* round attach address to SHMLBA boundary */
> >  #define	SHM_REMAP	040000	/* take-over region on attach */
> >  #define	SHM_EXEC	0100000	/* execution access */
> > +#define	SHM_MAP_HINT	0200000	/* interpret attach address as a hint */
> 
> search hint

Ok.

> > @@ -892,7 +892,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
> >  	sfd->vm_ops = NULL;
> >  
> >  	down_write(&current->mm->mmap_sem);
> > -	if (addr && !(shmflg & SHM_REMAP)) {
> > +	if (addr && !(shmflg & (SHM_REMAP|SHM_MAP_HINT))) {
> 
> I think you were right earlier that it can be just deleted, so why don't
> you just do that?

I want say that we shouldn't do this check if shmaddr is a search hint.
I'm not sure that check is unneeded if shmadd is the exact address.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[KOSAKI Motohiro]

> It allows interpret attach address as a hint, not as exact address.
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arjan van de Ven <arjan@infradead.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  include/linux/shm.h |    1 +
>  ipc/shm.c           |    4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/shm.h b/include/linux/shm.h
> index eca6235..2a637b8 100644
> --- a/include/linux/shm.h
> +++ b/include/linux/shm.h
> @@ -55,6 +55,7 @@ struct shmid_ds {
>  #define	SHM_RND		020000	/* round attach address to SHMLBA boundary */
>  #define	SHM_REMAP	040000	/* take-over region on attach */
>  #define	SHM_EXEC	0100000	/* execution access */
> +#define	SHM_MAP_HINT	0200000	/* interpret attach address as a hint */

hmm..
Honestly, I don't like that qemu specific feature insert into shmem core.
At least, this patch is too few comments.
Therefore, an develpper can't understand why SHM_MAP_HINT exist.

I think this patch description is too short and too poor.
I don't like increasing mysterious interface.

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Andi Kleen]

> Honestly, I don't like that qemu specific feature insert into shmem core.

I wouldn't say it's a qemu specific interface.  While qemu would 
be the first user I would expect more in the future. It's a pretty
obvious extension. In fact it nearly should be default, if the
risk of breaking old applications wasn't too high.

-Andi

-- 
ak@linux.intel.com

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

On Tue, Oct 07, 2008 at 01:21:19PM +0200, Andi Kleen wrote:
> > Honestly, I don't like that qemu specific feature insert into shmem core.
> 
> I wouldn't say it's a qemu specific interface.  While qemu would 
> be the first user I would expect more in the future. It's a pretty
> obvious extension. In fact it nearly should be default, if the
> risk of breaking old applications wasn't too high.

It's bad idea. It will break POSIX compatible.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH, RFC, v2] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

On Tue, Oct 07, 2008 at 01:26:31PM +0200, Andi Kleen wrote:
> > I want say that we shouldn't do this check if shmaddr is a search hint.
> > I'm not sure that check is unneeded if shmadd is the exact address.
> 
> mmap should fail in this case because it does the same check for 
> MAP_FIXED. Obviously it cannot succeed when there is already something
> else there.

We can do it in separate patch, I think.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

On Tue, Oct 07, 2008 at 08:08:19PM +0900, KOSAKI Motohiro wrote:
> > It allows interpret attach address as a hint, not as exact address.
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> > Cc: Andi Kleen <andi@firstfloor.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Arjan van de Ven <arjan@infradead.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >  include/linux/shm.h |    1 +
> >  ipc/shm.c           |    4 ++--
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/shm.h b/include/linux/shm.h
> > index eca6235..2a637b8 100644
> > --- a/include/linux/shm.h
> > +++ b/include/linux/shm.h
> > @@ -55,6 +55,7 @@ struct shmid_ds {
> >  #define	SHM_RND		020000	/* round attach address to SHMLBA boundary */
> >  #define	SHM_REMAP	040000	/* take-over region on attach */
> >  #define	SHM_EXEC	0100000	/* execution access */
> > +#define	SHM_MAP_HINT	0200000	/* interpret attach address as a hint */
> 
> hmm..
> Honestly, I don't like that qemu specific feature insert into shmem core.
> At least, this patch is too few comments.
> Therefore, an develpper can't understand why SHM_MAP_HINT exist.
> 
> I think this patch description is too short and too poor.
> I don't like increasing mysterious interface.

Sorry for it. I'll fix it in next patch version.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[KOSAKI Motohiro]

> > Honestly, I don't like that qemu specific feature insert into shmem core.
> 
> I wouldn't say it's a qemu specific interface.  While qemu would 
> be the first user I would expect more in the future. It's a pretty
> obvious extension. In fact it nearly should be default, if the
> risk of breaking old applications wasn't too high.

hm, ok, i understand your intension.
however, I think following code isn't self describing.

	addr = shmat(shmid, addr, SHM_MAP_HINT);

because HINT is too generic word.
I think we should find better word.

SHM_MAP_NO_FIXED ?


In addision, I still think current patch has too poor description and too 
few comments.

Re: [PATCH, RFC, v2] shmat: introduce flag SHM_MAP_HINT

[Andi Kleen]

> I want say that we shouldn't do this check if shmaddr is a search hint.
> I'm not sure that check is unneeded if shmadd is the exact address.

mmap should fail in this case because it does the same check for 
MAP_FIXED. Obviously it cannot succeed when there is already something
else there.

-Andi

-- 
ak@linux.intel.com

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 799 bytes --]

On Tue, Oct 07, 2008 at 08:26:03PM +0900, KOSAKI Motohiro wrote:
> > > Honestly, I don't like that qemu specific feature insert into shmem core.
> > 
> > I wouldn't say it's a qemu specific interface.  While qemu would 
> > be the first user I would expect more in the future. It's a pretty
> > obvious extension. In fact it nearly should be default, if the
> > risk of breaking old applications wasn't too high.
> 
> hm, ok, i understand your intension.
> however, I think following code isn't self describing.
> 
> 	addr = shmat(shmid, addr, SHM_MAP_HINT);
> 
> because HINT is too generic word.
> I think we should find better word.
> 
> SHM_MAP_NO_FIXED ?

I like it.
Andi?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Andi Kleen]

On Tue, Oct 07, 2008 at 02:30:51PM +0300, Kirill A. Shutemov wrote:
> On Tue, Oct 07, 2008 at 08:26:03PM +0900, KOSAKI Motohiro wrote:
> > > > Honestly, I don't like that qemu specific feature insert into shmem core.
> > > 
> > > I wouldn't say it's a qemu specific interface.  While qemu would 
> > > be the first user I would expect more in the future. It's a pretty
> > > obvious extension. In fact it nearly should be default, if the
> > > risk of breaking old applications wasn't too high.
> > 
> > hm, ok, i understand your intension.
> > however, I think following code isn't self describing.
> > 
> > 	addr = shmat(shmid, addr, SHM_MAP_HINT);
> > 
> > because HINT is too generic word.
> > I think we should find better word.
> > 
> > SHM_MAP_NO_FIXED ?
> 
> I like it.
> Andi?

SHM_MAP_NOT_FIXED perhaps.

I personally would call it SHM_MAP_SEARCH_HINT

But to be honest I have no strong opinion on the naming. Perhaps others have.

-Andi
-- 
ak@linux.intel.com

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Alan Cox]

> > Honestly, I don't like that qemu specific feature insert into shmem core.
> > At least, this patch is too few comments.
> > Therefore, an develpper can't understand why SHM_MAP_HINT exist.
> > 
> > I think this patch description is too short and too poor.
> > I don't like increasing mysterious interface.
> 
> Sorry for it. I'll fix it in next patch version.

I also don't see the point of this interface. We have POSIX shared memory
objects in Linux which are much cleaner and neater. They support mmap()
and mmap supports address hints.

There seems to be no reason at all to add further hacks to the historical
ugly SYS5 interface.

Alan

Re: [PATCH, RFC] shmat: introduce flag SHM_MAP_HINT

[Andi Kleen]

> I also don't see the point of this interface. We have POSIX shared memory
> objects in Linux which are much cleaner and neater. They support mmap()
> and mmap supports address hints.
> 
> There seems to be no reason at all to add further hacks to the historical
> ugly SYS5 interface.

Typically it's because some other parts of the interfaces that
cannot be easily changed (X shm would come to mind) need it.

He also needs it for the qemu syscall emulation. Even when he uses the compat
entry point shmat() will still only follow the personality.

-Andi
-- 
ak@linux.intel.com

Re: [PATCH, RFC, v2] shmat: introduce flag SHM_MAP_HINT

[Hugh Dickins]

On Tue, 7 Oct 2008, Andi Kleen wrote:
> > I want say that we shouldn't do this check if shmaddr is a search hint.
> > I'm not sure that check is unneeded if shmadd is the exact address.
> 
> mmap should fail in this case because it does the same check for 
> MAP_FIXED. Obviously it cannot succeed when there is already something
> else there.

I'm not really following this, so forgive me if I'm reading you
out of context, but I think you're wrong on that...

The dangerous feature of mmap MAP_FIXED (why we don't usually use
it except within an address range we've already staked out earlier)
is that it does unmap whatever stands in its way.  See the early
	if (flags & MAP_FIXED)
		return addr;
in arch_get_unmapped_area(), and the do_munmap() in mmap_region().

Hugh

Re: [PATCH, RFC, v2] shmat: introduce flag SHM_MAP_HINT

[Andi Kleen]

On Tue, Oct 07, 2008 at 03:38:44PM +0100, Hugh Dickins wrote:
> On Tue, 7 Oct 2008, Andi Kleen wrote:
> > > I want say that we shouldn't do this check if shmaddr is a search hint.
> > > I'm not sure that check is unneeded if shmadd is the exact address.
> > 
> > mmap should fail in this case because it does the same check for 
> > MAP_FIXED. Obviously it cannot succeed when there is already something
> > else there.
> 
> I'm not really following this, so forgive me if I'm reading you
> out of context, but I think you're wrong on that...

You're right, Hugh, I was confused here. The earlier check
is indeed needed and cannot be dropped. Thanks for the reality check.

-Andi