Re: [PATCH] Don't mlock guardpage if the stack is growing up

[Linus Torvalds]

On Sun, May 8, 2011 at 11:55 AM, Mikulas Patocka
<mikulas@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> This patch fixes lvm2 on PA-RISC (and possibly other architectures with
> up-growing stack). lvm2 calculates the number of used pages when locking
> and when unlocking and reports an internal error if the numbers mismatch.

This patch won't apply on current kernels (including stable) because
of commit a1fde08c74e9 that changed the test of "pages" to instead
just test "flags & FOLL_MLOCK".

That should be trivial to fix up.

However, I really don't much like this complex test:

>  static inline int stack_guard_page(struct vm_area_struct *vma, unsigned long addr)
>  {
> -       return (vma->vm_flags & VM_GROWSDOWN) &&
> +       return ((vma->vm_flags & VM_GROWSDOWN) &&
>                (vma->vm_start == addr) &&
> -               !vma_stack_continue(vma->vm_prev, addr);
> +               !vma_stack_continue(vma->vm_prev, addr)) ||
> +              ((vma->vm_flags & VM_GROWSUP) &&
> +               (vma->vm_end == addr + PAGE_SIZE) &&
> +               !vma_stack_growsup_continue(vma->vm_next, addr + PAGE_SIZE));
>  }

in that format. It gets really hard to read, and I think you'd be
better off writing it as two helper functions (or macros) for the two
cases, and then have

  static inline int stack_guard_page(struct vm_area_struct *vma,
unsigned long addr)
  {
    return stack_guard_page_growsdown(vma, addr) ||
      stack_guard_page_growsup(vma, addr);
  }

I'd also like to verify that it doesn't actually generate any extra
code for the common case (iirc VM_GROWSUP is 0 for the architectures
that don't need it, and so the compiler shouldn't generate any extra
code, but I'd like that mentioned and verified explicitly).

Hmm?

Other than that it looks ok to me.

That said, could we please fix LVM to not do that crazy sh*t in the
first place? The STACK_GROWSUP case is never going to have a lot of
testing, this is just sad.

                   Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/