2010-08-15 05:15:19

by Jovi Zhang

Subject: [PATCH] mm: code improvement of check_stack_guard_page

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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2010-08-15 17:59:30

by Linus Torvalds

Subject: Re: [PATCH] mm: code improvement of check_stack_guard_page

On Sat, Aug 14, 2010 at 10:15 PM, jovi zhang <[email protected]> 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

2010-08-16 01:32:06

by Jovi Zhang

Subject: Re: [PATCH] mm: code improvement of check_stack_guard_page

On Mon, Aug 16, 2010 at 1:58 AM, Linus Torvalds
<[email protected]> wrote:
> On Sat, Aug 14, 2010 at 10:15 PM, jovi zhang <[email protected]> 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.

2010-08-18 02:07:11

by Kamezawa Hiroyuki

Subject: Re: [PATCH] mm: code improvement of check_stack_guard_page

On Sun, 15 Aug 2010 13:07:56 +0800
jovi zhang <[email protected]> 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;
> }

2010-08-18 06:10:33

by Balbir Singh

Subject: Re: [PATCH] mm: code improvement of check_stack_guard_page

On Sun, Aug 15, 2010 at 10:45 AM, jovi zhang <[email protected]> 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