[PATCH] mm: code improvement of check_stack_guard_page

[PATCH] mm: code improvement of check_stack_guard_page

[jovi zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 1488 bytes --]

little code improvement of check_stack_guard_page function.
this commit is on top of commit "mm: keep a guard page below a
grow-down stack segment" of linus.

diff --git a/mm/memory.c b/mm/memory.c
index 9b3b73f..643b112 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2768,13 +2768,15 @@ out_release:
  */
 static inline int check_stack_guard_page(struct vm_area_struct *vma,
unsigned long address)
 {
-       address &= PAGE_MASK;
-       if ((vma->vm_flags & VM_GROWSDOWN) && address == vma->vm_start) {
-               address -= PAGE_SIZE;
-               if (find_vma(vma->vm_mm, address) != vma)
-                       return -ENOMEM;
-
-               expand_stack(vma, address);
+       if (vma->vm_flags & VM_GROWSDOWN) {
+               address &= PAGE_MASK;
+               if(address == vma->vm_start) {
+                       address -= PAGE_SIZE;
+                       if (unlikely(find_vma(vma->vm_mm, address) != vma))
+                               return -ENOMEM;
+
+                       expand_stack(vma, address);
+               }
        }
        return 0;
 }
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] mm: code improvement of check_stack_guard_page

[Linus Torvalds]

On Sat, Aug 14, 2010 at 10:15 PM, jovi zhang <bookjovi@gmail.com> wrote:
>
> little code improvement of check_stack_guard_page function.
> this commit is on top of commit "mm: keep a guard page below a
> grow-down stack segment" of linus.

I don't think your patch is in any way wrong, but I just checked, and
it makes absolutely zero difference to the code generation at least
for my version of gcc. The resulting assembly is identical except for
the line numbers that changed (BUG_ON() line/file information and
debug information).

So I think I prefer the shallower indentation.

                       Linus

Re: [PATCH] mm: code improvement of check_stack_guard_page

[jovi zhang]

On Mon, Aug 16, 2010 at 1:58 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sat, Aug 14, 2010 at 10:15 PM, jovi zhang <bookjovi@gmail.com> wrote:
>>
>> little code improvement of check_stack_guard_page function.
>> this commit is on top of commit "mm: keep a guard page below a
>> grow-down stack segment" of linus.
>
> I don't think your patch is in any way wrong, but I just checked, and
> it makes absolutely zero difference to the code generation at least
> for my version of gcc. The resulting assembly is identical except for
> the line numbers that changed (BUG_ON() line/file information and
> debug information).
>
> So I think I prefer the shallower indentation.
>
>                       Linus
>

That's fine. I havn't consider gcc optimization.

Re: [PATCH] mm: code improvement of check_stack_guard_page

[KAMEZAWA Hiroyuki]

On Sun, 15 Aug 2010 13:07:56 +0800
jovi zhang <bookjovi@gmail.com> wrote:

> little code improvement of check_stack_guard_page function.
> this commit is on top of commit "mm: keep a guard page below a grow-down
> stack segment" of linus.
> 

Hmm. difference in binary code finally ?

-Kame

> diff --git a/mm/memory.c b/mm/memory.c
> index 9b3b73f..643b112 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2768,13 +2768,15 @@ out_release:
>   */
>  static inline int check_stack_guard_page(struct vm_area_struct *vma,
> unsigned long address)
>  {
> -       address &= PAGE_MASK;
> -       if ((vma->vm_flags & VM_GROWSDOWN) && address == vma->vm_start) {
> -               address -= PAGE_SIZE;
> -               if (find_vma(vma->vm_mm, address) != vma)
> -                       return -ENOMEM;
> -
> -               expand_stack(vma, address);
> +       if (vma->vm_flags & VM_GROWSDOWN) {
> +               address &= PAGE_MASK;
> +               if(address == vma->vm_start) {
> +                       address -= PAGE_SIZE;
> +                       if (unlikely(find_vma(vma->vm_mm, address) != vma))
> +                               return -ENOMEM;
> +
> +                       expand_stack(vma, address);
> +               }
>         }
>         return 0;
>  }

Re: [PATCH] mm: code improvement of check_stack_guard_page

[KAMEZAWA Hiroyuki]

On Sun, 15 Aug 2010 13:07:56 +0800
jovi zhang <bookjovi@gmail.com> wrote:

> little code improvement of check_stack_guard_page function.
> this commit is on top of commit "mm: keep a guard page below a grow-down
> stack segment" of linus.
> 

Hmm. difference in binary code finally ?

-Kame

> diff --git a/mm/memory.c b/mm/memory.c
> index 9b3b73f..643b112 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2768,13 +2768,15 @@ out_release:
>   */
>  static inline int check_stack_guard_page(struct vm_area_struct *vma,
> unsigned long address)
>  {
> -       address &= PAGE_MASK;
> -       if ((vma->vm_flags & VM_GROWSDOWN) && address == vma->vm_start) {
> -               address -= PAGE_SIZE;
> -               if (find_vma(vma->vm_mm, address) != vma)
> -                       return -ENOMEM;
> -
> -               expand_stack(vma, address);
> +       if (vma->vm_flags & VM_GROWSDOWN) {
> +               address &= PAGE_MASK;
> +               if(address == vma->vm_start) {
> +                       address -= PAGE_SIZE;
> +                       if (unlikely(find_vma(vma->vm_mm, address) != vma))
> +                               return -ENOMEM;
> +
> +                       expand_stack(vma, address);
> +               }
>         }
>         return 0;
>  }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: code improvement of check_stack_guard_page

[Balbir Singh]

On Sun, Aug 15, 2010 at 10:45 AM, jovi zhang <bookjovi@gmail.com> wrote:
> little code improvement of check_stack_guard_page function.
> this commit is on top of commit "mm: keep a guard page below a
> grow-down stack segment" of linus.
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 9b3b73f..643b112 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2768,13 +2768,15 @@ out_release:
>   */
>  static inline int check_stack_guard_page(struct vm_area_struct *vma,
> unsigned long address)
>  {
> -       address &= PAGE_MASK;
> -       if ((vma->vm_flags & VM_GROWSDOWN) && address == vma->vm_start) {
> -               address -= PAGE_SIZE;
> -               if (find_vma(vma->vm_mm, address) != vma)
> -                       return -ENOMEM;
> -
> -               expand_stack(vma, address);
> +       if (vma->vm_flags & VM_GROWSDOWN) {
> +               address &= PAGE_MASK;
> +               if(address == vma->vm_start) {
                    ^^^ coding style is broken, did u run it through
scripts/checkpatch.pl?

> +                       address -= PAGE_SIZE;
> +                       if (unlikely(find_vma(vma->vm_mm, address) != vma))
> +                               return -ENOMEM;
> +
> +                       expand_stack(vma, address);
> +               }
>         }
>         return 0;
>  }
>

The main benefit I see is the new branch hint being passed to
find_vma, is my understanding correct?

Balbir