lkml.org 
[lkml]   [2008]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 2/3] binfmt_elf_fdpic: Convert initial stack alignment to arch_align_stack().
    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) */

    \
     
     \ /
      Last update: 2008-08-04 06:03    [W:0.026 / U:129.420 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site