linux-sh.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] binfmt_elf_fdpic: auxvec updates.
@ 2008-07-28 15:03 Paul Mundt
  2008-07-28 15:04 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string Paul Mundt
  2008-08-01 13:57 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string David Howells
  0 siblings, 2 replies; 12+ messages in thread
From: Paul Mundt @ 2008-07-28 15:03 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, linux-sh, linux-kernel

This is a tiny series of patches for bringing binfmt_elf_fdpic back in
line with binfmt_elf in terms of auxiliary vector layout, particularly
for the new entries that have popped up recently.

[PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string.
[PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
[PATCH 3/3] binfmt_elf_fdpic: Wire up AT_EXECFD, AT_EXECFN, AT_SECURE.

These depend on the refactored NEW_AUX_ENT(), so against -next or -mm
presently.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 fs/binfmt_elf_fdpic.c |   91 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 66 insertions(+), 25 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string.
  2008-07-28 15:03 [PATCH 0/3] binfmt_elf_fdpic: auxvec updates Paul Mundt
@ 2008-07-28 15:04 ` Paul Mundt
  2008-07-28 15:05   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() Paul Mundt
  2008-08-01 14:01   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() David Howells
  2008-08-01 13:57 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string David Howells
  1 sibling, 2 replies; 12+ messages in thread
From: Paul Mundt @ 2008-07-28 15:04 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, linux-sh, linux-kernel

Commit 483fad1c3fa1060d7e6710e84a065ad514571739 introduces
AT_BASE_PLATFORM, but only implements it for binfmt_elf. Given that
AT_VECTOR_SIZE_BASE is unconditionally enlarged for us, and it's only
optionally added in for the platforms that set ELF_BASE_PLATFORM, wire
it up for binfmt_elf_fdpic, too.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 fs/binfmt_elf_fdpic.c |   50 ++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 80c1f95..0955d03 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -455,6 +455,16 @@ error_kill:
 }
 
 /*****************************************************************************/
+
+#ifndef ELF_BASE_PLATFORM
+/*
+ * AT_BASE_PLATFORM indicates the "real" hardware/microarchitecture.
+ * If the arch defines ELF_BASE_PLATFORM (in asm/elf.h), the value
+ * will be copied to the user stack in the same manner as AT_PLATFORM.
+ */
+#define ELF_BASE_PLATFORM NULL
+#endif
+
 /*
  * present useful information to the program
  */
@@ -466,8 +476,8 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	unsigned long sp, csp, nitems;
 	elf_caddr_t __user *argv, *envp;
 	size_t platform_len = 0, len;
-	char *k_platform;
-	char __user *u_platform, *p;
+	char *k_platform, *k_base_platform;
+	char __user *u_platform, *u_base_platform, *p;
 	long hwcap;
 	int loop;
 	int nr;	/* reset for each csp adjustment */
@@ -483,11 +493,14 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 		return -EFAULT;
 #endif
 
-	/* get hold of platform and hardware capabilities masks for the machine
-	 * we are running on.  In some cases (Sparc), this info is impossible
-	 * to get, in others (i386) it is merely difficult.
-	 */
 	hwcap = ELF_HWCAP;
+
+	/*
+	 * If this architecture has a platform capability string, copy it
+	 * to userspace.  In some cases (Sparc), this info is impossible
+	 * for userspace to get any other way, in others (i386) it is
+	 * merely difficult.
+	 */
 	k_platform = ELF_PLATFORM;
 	u_platform = NULL;
 
@@ -499,6 +512,21 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 			return -EFAULT;
 	}
 
+	/*
+	 * If this architecture has a "base" platform capability
+	 * string, copy it to userspace.
+	 */
+	k_base_platform = ELF_BASE_PLATFORM;
+	u_base_platform = NULL;
+
+	if (k_base_platform) {
+		platform_len = strlen(k_base_platform) + 1;
+		sp -= platform_len;
+		u_base_platform = (char __user *) sp;
+		if (__copy_to_user(u_base_platform, k_base_platform, platform_len) != 0)
+			return -EFAULT;
+	}
+
 #if defined(__i386__) && defined(CONFIG_SMP)
 	/* in some cases (e.g. Hyper-Threading), we want to avoid L1 evictions
 	 * by the processes running on the same package. One thing we can do is
@@ -543,7 +571,8 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	/* force 16 byte _final_ alignment here for generality */
 #define DLINFO_ITEMS 13
 
-	nitems = 1 + DLINFO_ITEMS + (k_platform ? 1 : 0) + AT_VECTOR_SIZE_ARCH;
+	nitems = 1 + DLINFO_ITEMS + (k_platform ? 1 : 0) + \
+		(k_base_platform ? 1 : 0) + AT_VECTOR_SIZE_ARCH;
 
 	csp = sp;
 	sp -= nitems * 2 * sizeof(unsigned long);
@@ -575,6 +604,13 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 			    (elf_addr_t) (unsigned long) u_platform);
 	}
 
+	if (k_base_platform) {
+		nr = 0;
+		csp -= 2 * sizeof(unsigned long);
+		NEW_AUX_ENT(AT_BASE_PLATFORM,
+			    (elf_addr_t) (unsigned long) u_base_platform);
+	}
+
 	nr = 0;
 	csp -= DLINFO_ITEMS * 2 * sizeof(unsigned long);
 	NEW_AUX_ENT(AT_HWCAP,	hwcap);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
  2008-07-28 15:04 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string Paul Mundt
@ 2008-07-28 15:05   ` Paul Mundt
  2008-07-28 15:05     ` [PATCH 3/3] binfmt_elf_fdpic: Wire up AT_EXECFD, AT_EXECFN, AT_SECURE Paul Mundt
  2008-08-01 14:04     ` David Howells
  2008-08-01 14:01   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() David Howells
  1 sibling, 2 replies; 12+ messages in thread
From: Paul Mundt @ 2008-07-28 15:05 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, linux-sh, linux-kernel

binfmt_elf_fdpic seems to have grabbed a hard-coded hack from an ancient
version of binfmt_elf in order to try and fix up initial stack alignment
on multi-threaded x86, which while in addition to being unused, was also
pushed down beyond the first set of operations on the stack pointer,
negating the entire purpose.

These days, we have an architecture independent arch_align_stack(), so we
switch to using that instead. Move the initial alignment up before the
initial stores while we're at it.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 fs/binfmt_elf_fdpic.c |   27 ++++++++++-----------------
 1 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 0955d03..bc64f5f 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -482,11 +482,18 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	int loop;
 	int nr;	/* reset for each csp adjustment */
 
-	/* we're going to shovel a whole load of stuff onto the stack */
+	/*
+	 * we're going to shovel a whole load of stuff onto the stack
+	 *
+	 * In some cases (e.g. Hyper-Threading), we want to avoid L1
+	 * evictions by the processes running on the same package. One
+	 * thing we can do is to shuffle the initial stack for them, so
+	 * we give the architecture an opportunity to do so here.
+	 */
 #ifdef CONFIG_MMU
-	sp = bprm->p;
+	sp = arch_align_stack(bprm->p);
 #else
-	sp = mm->start_stack;
+	sp = arch_align_stack(mm->start_stack);
 
 	/* stack the program arguments and environment */
 	if (elf_fdpic_transfer_args_to_stack(bprm, &sp) < 0)
@@ -527,20 +534,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 			return -EFAULT;
 	}
 
-#if defined(__i386__) && defined(CONFIG_SMP)
-	/* in some cases (e.g. Hyper-Threading), we want to avoid L1 evictions
-	 * by the processes running on the same package. One thing we can do is
-	 * to shuffle the initial stack for them.
-	 *
-	 * the conditionals here are unneeded, but kept in to make the code
-	 * behaviour the same as pre change unless we have hyperthreaded
-	 * processors. This keeps Mr Marcelo Person happier but should be
-	 * removed for 2.5
-	 */
-	if (smp_num_siblings > 1)
-		sp = sp - ((current->pid % 64) << 7);
-#endif
-
 	sp &= ~7UL;
 
 	/* stack the load map(s) */

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] binfmt_elf_fdpic: Wire up AT_EXECFD, AT_EXECFN, AT_SECURE.
  2008-07-28 15:05   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() Paul Mundt
@ 2008-07-28 15:05     ` Paul Mundt
  2008-08-01 14:04     ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: Paul Mundt @ 2008-07-28 15:05 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, linux-sh, linux-kernel

These auxvec entries are the only ones left unhandled out of the current
base implementation. This syncs up binfmt_elf_fdpic with linux/auxvec.h
and current binfmt_elf.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 fs/binfmt_elf_fdpic.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index bc64f5f..f0dd83c 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -25,6 +25,7 @@
 #include <linux/fcntl.h>
 #include <linux/slab.h>
 #include <linux/pagemap.h>
+#include <linux/security.h>
 #include <linux/highmem.h>
 #include <linux/highuid.h>
 #include <linux/personality.h>
@@ -562,11 +563,14 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	}
 
 	/* force 16 byte _final_ alignment here for generality */
-#define DLINFO_ITEMS 13
+#define DLINFO_ITEMS 15
 
 	nitems = 1 + DLINFO_ITEMS + (k_platform ? 1 : 0) + \
 		(k_base_platform ? 1 : 0) + AT_VECTOR_SIZE_ARCH;
 
+	if (bprm->interp_flags & BINPRM_FLAGS_EXECFD)
+		nitems++;
+
 	csp = sp;
 	sp -= nitems * 2 * sizeof(unsigned long);
 	sp -= (bprm->envc + 1) * sizeof(char *);	/* envv[] */
@@ -604,6 +608,12 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 			    (elf_addr_t) (unsigned long) u_base_platform);
 	}
 
+	if (bprm->interp_flags & BINPRM_FLAGS_EXECFD) {
+		nr = 0;
+		csp -= 2 * sizeof(unsigned long);
+		NEW_AUX_ENT(AT_EXECFD, bprm->interp_data);
+	}
+
 	nr = 0;
 	csp -= DLINFO_ITEMS * 2 * sizeof(unsigned long);
 	NEW_AUX_ENT(AT_HWCAP,	hwcap);
@@ -619,6 +629,8 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	NEW_AUX_ENT(AT_EUID,	(elf_addr_t) current->euid);
 	NEW_AUX_ENT(AT_GID,	(elf_addr_t) current->gid);
 	NEW_AUX_ENT(AT_EGID,	(elf_addr_t) current->egid);
+	NEW_AUX_ENT(AT_SECURE,	security_bprm_secureexec(bprm));
+	NEW_AUX_ENT(AT_EXECFN,	bprm->exec);
 
 #ifdef ARCH_DLINFO
 	nr = 0;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string.
  2008-07-28 15:03 [PATCH 0/3] binfmt_elf_fdpic: auxvec updates Paul Mundt
  2008-07-28 15:04 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string Paul Mundt
@ 2008-08-01 13:57 ` David Howells
  2008-08-01 21:46   ` Paul Mundt
  1 sibling, 1 reply; 12+ messages in thread
From: David Howells @ 2008-08-01 13:57 UTC (permalink / raw)
  To: Paul Mundt; +Cc: dhowells, Andrew Morton, linux-sh, linux-kernel

Paul Mundt <lethal@linux-sh.org> wrote:

> +	nitems = 1 + DLINFO_ITEMS + (k_platform ? 1 : 0) + \
> +		(k_base_platform ? 1 : 0) + AT_VECTOR_SIZE_ARCH;

Why the backslash?

David

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
  2008-07-28 15:04 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string Paul Mundt
  2008-07-28 15:05   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() Paul Mundt
@ 2008-08-01 14:01   ` David Howells
  2008-08-01 21:44     ` Paul Mundt
  1 sibling, 1 reply; 12+ messages in thread
From: David Howells @ 2008-08-01 14:01 UTC (permalink / raw)
  To: Paul Mundt; +Cc: dhowells, Andrew Morton, linux-sh, linux-kernel

Paul Mundt <lethal@linux-sh.org> wrote:

> +	 * In some cases (e.g. Hyper-Threading), we want to avoid L1
> +	 * evictions by the processes running on the same package. One
> +	 * thing we can do is to shuffle the initial stack for them, so
> +	 * we give the architecture an opportunity to do so here.
> +	 */
>  #ifdef CONFIG_MMU
> -	sp = bprm->p;
> +	sp = arch_align_stack(bprm->p);
>  #else
> -	sp = mm->start_stack;
> +	sp = arch_align_stack(mm->start_stack);

Ummm...  You're calling arch_align_stack() under NOMMU...  Is that really a
good idea?

You can't necessarily move the stack pointer without exiting the allocated
region or shrinking the amount of stack space the executable asked for.  If
you want to do this sort of thing, you need to tell the memory allocator what
you're up to - or at the very least allocate some slack.

David

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] binfmt_elf_fdpic: Wire up AT_EXECFD, AT_EXECFN, AT_SECURE.
  2008-07-28 15:05   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() Paul Mundt
  2008-07-28 15:05     ` [PATCH 3/3] binfmt_elf_fdpic: Wire up AT_EXECFD, AT_EXECFN, AT_SECURE Paul Mundt
@ 2008-08-01 14:04     ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: David Howells @ 2008-08-01 14:04 UTC (permalink / raw)
  To: Paul Mundt; +Cc: dhowells, Andrew Morton, linux-sh, linux-kernel

Paul Mundt <lethal@linux-sh.org> wrote:

> These auxvec entries are the only ones left unhandled out of the current
> base implementation. This syncs up binfmt_elf_fdpic with linux/auxvec.h
> and current binfmt_elf.
> 
> Signed-off-by: Paul Mundt <lethal@linux-sh.org>

Acked-by: David Howells <dhowells@redhat.com>

Though I'm not sure bprm->exec is any use in NOMMU circumstances:-/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
  2008-08-01 14:01   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() David Howells
@ 2008-08-01 21:44     ` Paul Mundt
  2008-08-04  3:24       ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Mundt @ 2008-08-01 21:44 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, linux-sh, linux-kernel

On Fri, Aug 01, 2008 at 03:01:19PM +0100, David Howells wrote:
> Paul Mundt <lethal@linux-sh.org> wrote:
> 
> > +	 * In some cases (e.g. Hyper-Threading), we want to avoid L1
> > +	 * evictions by the processes running on the same package. One
> > +	 * thing we can do is to shuffle the initial stack for them, so
> > +	 * we give the architecture an opportunity to do so here.
> > +	 */
> >  #ifdef CONFIG_MMU
> > -	sp = bprm->p;
> > +	sp = arch_align_stack(bprm->p);
> >  #else
> > -	sp = mm->start_stack;
> > +	sp = arch_align_stack(mm->start_stack);
> 
> Ummm...  You're calling arch_align_stack() under NOMMU...  Is that really a
> good idea?
> 
Not particularly, no.

> You can't necessarily move the stack pointer without exiting the allocated
> region or shrinking the amount of stack space the executable asked for.  If
> you want to do this sort of thing, you need to tell the memory allocator what
> you're up to - or at the very least allocate some slack.
> 
Yes, that's a good point, and one that probably ought to be documented
alongside the initial alignment. I'll send an updated patch.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string.
  2008-08-01 13:57 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string David Howells
@ 2008-08-01 21:46   ` Paul Mundt
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Mundt @ 2008-08-01 21:46 UTC (permalink / raw)
  To: David Howells; +Cc: Andrew Morton, linux-sh, linux-kernel

On Fri, Aug 01, 2008 at 02:57:01PM +0100, David Howells wrote:
> Paul Mundt <lethal@linux-sh.org> wrote:
> 
> > +	nitems = 1 + DLINFO_ITEMS + (k_platform ? 1 : 0) + \
> > +		(k_base_platform ? 1 : 0) + AT_VECTOR_SIZE_ARCH;
> 
> Why the backslash?
> 
Habitual behavioural patterns?

If you feel strongly about it it can just be edited out of the patch.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
  2008-08-01 21:44     ` Paul Mundt
@ 2008-08-04  3:24       ` Mike Frysinger
  2008-08-04  4:00         ` Paul Mundt
  2008-08-04 10:08         ` David Howells
  0 siblings, 2 replies; 12+ messages in thread
From: Mike Frysinger @ 2008-08-04  3:24 UTC (permalink / raw)
  To: Paul Mundt, Wu, Bryan
  Cc: David Howells, Andrew Morton, linux-sh, linux-kernel

On Fri, Aug 1, 2008 at 5:44 PM, Paul Mundt wrote:
> On Fri, Aug 01, 2008 at 03:01:19PM +0100, David Howells wrote:
>> Paul Mundt <lethal@linux-sh.org> wrote:
>> > +    * In some cases (e.g. Hyper-Threading), we want to avoid L1
>> > +    * evictions by the processes running on the same package. One
>> > +    * thing we can do is to shuffle the initial stack for them, so
>> > +    * we give the architecture an opportunity to do so here.
>> > +    */
>> >  #ifdef CONFIG_MMU
>> > -   sp = bprm->p;
>> > +   sp = arch_align_stack(bprm->p);
>> >  #else
>> > -   sp = mm->start_stack;
>> > +   sp = arch_align_stack(mm->start_stack);
>>
>> Ummm...  You're calling arch_align_stack() under NOMMU...  Is that really a
>> good idea?
>
> Not particularly, no.
>
>> You can't necessarily move the stack pointer without exiting the allocated
>> region or shrinking the amount of stack space the executable asked for.  If
>> you want to do this sort of thing, you need to tell the memory allocator what
>> you're up to - or at the very least allocate some slack.
>
> Yes, that's a good point, and one that probably ought to be documented
> alongside the initial alignment. I'll send an updated patch.

fs/binfmt_elf_fdpic.c: In function 'create_elf_fdpic_tables':
fs/binfmt_elf_fdpic.c:497: error: implicit declaration of function
'arch_align_stack'
make[1]: *** [fs/binfmt_elf_fdpic.o] Error 1
make: *** [fs] Error 2

Blackfin lacks a arch_align_stack(x) in asm-blackfin/system.h ... i
imagine a stub will work for us like every other arch though

feel like doing that Bryan ?  i'm guessing Paul doesnt want to ... ;)
-mike

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
  2008-08-04  3:24       ` Mike Frysinger
@ 2008-08-04  4:00         ` Paul Mundt
  2008-08-04 10:08         ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: Paul Mundt @ 2008-08-04  4:00 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Wu, Bryan, David Howells, Andrew Morton, linux-sh, linux-kernel

On Sun, Aug 03, 2008 at 11:24:21PM -0400, Mike Frysinger wrote:
> On Fri, Aug 1, 2008 at 5:44 PM, Paul Mundt wrote:
> > On Fri, Aug 01, 2008 at 03:01:19PM +0100, David Howells wrote:
> >> Paul Mundt <lethal@linux-sh.org> wrote:
> >> > +    * In some cases (e.g. Hyper-Threading), we want to avoid L1
> >> > +    * evictions by the processes running on the same package. One
> >> > +    * thing we can do is to shuffle the initial stack for them, so
> >> > +    * we give the architecture an opportunity to do so here.
> >> > +    */
> >> >  #ifdef CONFIG_MMU
> >> > -   sp = bprm->p;
> >> > +   sp = arch_align_stack(bprm->p);
> >> >  #else
> >> > -   sp = mm->start_stack;
> >> > +   sp = arch_align_stack(mm->start_stack);
> >>
> >> Ummm...  You're calling arch_align_stack() under NOMMU...  Is that really a
> >> good idea?
> >
> > Not particularly, no.
> >
> >> You can't necessarily move the stack pointer without exiting the allocated
> >> region or shrinking the amount of stack space the executable asked for.  If
> >> you want to do this sort of thing, you need to tell the memory allocator what
> >> you're up to - or at the very least allocate some slack.
> >
> > Yes, that's a good point, and one that probably ought to be documented
> > alongside the initial alignment. I'll send an updated patch.
> 
> fs/binfmt_elf_fdpic.c: In function 'create_elf_fdpic_tables':
> fs/binfmt_elf_fdpic.c:497: error: implicit declaration of function
> 'arch_align_stack'
> make[1]: *** [fs/binfmt_elf_fdpic.o] Error 1
> make: *** [fs] Error 2
> 
> Blackfin lacks a arch_align_stack(x) in asm-blackfin/system.h ... i
> imagine a stub will work for us like every other arch though
> 
> feel like doing that Bryan ?  i'm guessing Paul doesnt want to ... ;)

There's no need, as David pointed out, that was the wrong thing to do for
the nommu case anyways. Here's an updated patch to replace the previous
one:

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 80c1f95..10c6cb8 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -472,9 +472,16 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	int loop;
 	int nr;	/* reset for each csp adjustment */
 
-	/* we're going to shovel a whole load of stuff onto the stack */
 #ifdef CONFIG_MMU
-	sp = bprm->p;
+	/*
+	 * we're going to shovel a whole load of stuff onto the stack
+	 *
+	 * In some cases (e.g. Hyper-Threading), we want to avoid L1
+	 * evictions by the processes running on the same package. One
+	 * thing we can do is to shuffle the initial stack for them, so
+	 * we give the architecture an opportunity to do so here.
+	 */
+	sp = arch_align_stack(bprm->p);
 #else
 	sp = mm->start_stack;
 
@@ -499,20 +506,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 			return -EFAULT;
 	}
 
-#if defined(__i386__) && defined(CONFIG_SMP)
-	/* in some cases (e.g. Hyper-Threading), we want to avoid L1 evictions
-	 * by the processes running on the same package. One thing we can do is
-	 * to shuffle the initial stack for them.
-	 *
-	 * the conditionals here are unneeded, but kept in to make the code
-	 * behaviour the same as pre change unless we have hyperthreaded
-	 * processors. This keeps Mr Marcelo Person happier but should be
-	 * removed for 2.5
-	 */
-	if (smp_num_siblings > 1)
-		sp = sp - ((current->pid % 64) << 7);
-#endif
-
 	sp &= ~7UL;
 
 	/* stack the load map(s) */

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
  2008-08-04  3:24       ` Mike Frysinger
  2008-08-04  4:00         ` Paul Mundt
@ 2008-08-04 10:08         ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: David Howells @ 2008-08-04 10:08 UTC (permalink / raw)
  To: Paul Mundt
  Cc: dhowells, Mike Frysinger, Wu, Bryan, Andrew Morton, linux-sh,
	linux-kernel

Paul Mundt <lethal@linux-sh.org> wrote:

> There's no need, as David pointed out, that was the wrong thing to do for
> the nommu case anyways. Here's an updated patch to replace the previous
> one:

That's better, however:

> -	/* we're going to shovel a whole load of stuff onto the stack */
>  #ifdef CONFIG_MMU
> -	sp = bprm->p;
> +	/*
> +	 * we're going to shovel a whole load of stuff onto the stack

I think that comment shouldn't be moved inside the conditional section since
it applies to NOMMU as well.  Picky, I know, but...  It might actually be
better to attach it to the banner comment for the whole function.

David
---
[PATCH] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack()

From: Paul Mundt <lethal@linux-sh.org>

binfmt_elf_fdpic seems to have grabbed a hard-coded hack from an ancient
version of binfmt_elf in order to try and fix up initial stack alignment
on multi-threaded x86, which while in addition to being unused, was also
pushed down beyond the first set of operations on the stack pointer,
negating the entire purpose.

These days, we have an architecture independent arch_align_stack(), so we
switch to using that instead. Move the initial alignment up before the
initial stores while we're at it.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/binfmt_elf_fdpic.c |   25 ++++++++-----------------
 1 files changed, 8 insertions(+), 17 deletions(-)


diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 7c83605..f9d6eaa 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -466,7 +466,8 @@ error_kill:
 #endif
 
 /*
- * present useful information to the program
+ * present useful information to the program by shovelling it onto the new
+ * process's stack
  */
 static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 				   struct mm_struct *mm,
@@ -482,9 +483,13 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	int loop;
 	int nr;	/* reset for each csp adjustment */
 
-	/* we're going to shovel a whole load of stuff onto the stack */
 #ifdef CONFIG_MMU
-	sp = bprm->p;
+	/* In some cases (e.g. Hyper-Threading), we want to avoid L1 evictions
+	 * by the processes running on the same package. One thing we can do is
+	 * to shuffle the initial stack for them, so we give the architecture
+	 * an opportunity to do so here.
+	 */
+	sp = arch_align_stack(bprm->p);
 #else
 	sp = mm->start_stack;
 
@@ -527,20 +532,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 			return -EFAULT;
 	}
 
-#if defined(__i386__) && defined(CONFIG_SMP)
-	/* in some cases (e.g. Hyper-Threading), we want to avoid L1 evictions
-	 * by the processes running on the same package. One thing we can do is
-	 * to shuffle the initial stack for them.
-	 *
-	 * the conditionals here are unneeded, but kept in to make the code
-	 * behaviour the same as pre change unless we have hyperthreaded
-	 * processors. This keeps Mr Marcelo Person happier but should be
-	 * removed for 2.5
-	 */
-	if (smp_num_siblings > 1)
-		sp = sp - ((current->pid % 64) << 7);
-#endif
-
 	sp &= ~7UL;
 
 	/* stack the load map(s) */

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2008-08-04 10:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-28 15:03 [PATCH 0/3] binfmt_elf_fdpic: auxvec updates Paul Mundt
2008-07-28 15:04 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string Paul Mundt
2008-07-28 15:05   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() Paul Mundt
2008-07-28 15:05     ` [PATCH 3/3] binfmt_elf_fdpic: Wire up AT_EXECFD, AT_EXECFN, AT_SECURE Paul Mundt
2008-08-01 14:04     ` David Howells
2008-08-01 14:01   ` [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack() David Howells
2008-08-01 21:44     ` Paul Mundt
2008-08-04  3:24       ` Mike Frysinger
2008-08-04  4:00         ` Paul Mundt
2008-08-04 10:08         ` David Howells
2008-08-01 13:57 ` [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string David Howells
2008-08-01 21:46   ` Paul Mundt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).