2011-04-19 11:10:01

by Michal Hocko

[permalink] [raw]
Subject: [PATCH followup] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64

While I am in the cleanup mode. We should use VM_GROWSUP rather than
tricky CONFIG_STACK_GROWSUP||CONFIG_IA64.

What do you think?
---
>From fd832dd46b4918718901f2ebe994f4662f167999 Mon Sep 17 00:00:00 2001
From: Michal Hocko <[email protected]>
Date: Tue, 19 Apr 2011 11:11:41 +0200
Subject: [PATCH] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64

IA64 needs some trickery for Register Backing Store so we have to
export expand_stack_upwards for it even though the architecture expands
its stack downwards normally. To avoid
we have defined VM_GROWSUP which is defined only for the above
configuration.

We still have places which use the original ifdefs so let's get rid of
them finally.

Signed-off-by: Michal Hocko <[email protected]>
---
mm/mmap.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 29c68b0..3ff9edf 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1726,7 +1726,7 @@ static int acct_stack_growth(struct vm_area_struct *vma, unsigned long size, uns
return 0;
}

-#if defined(CONFIG_STACK_GROWSUP) || defined(CONFIG_IA64)
+#if VM_GROWSUP
/*
* PA-RISC uses this for its stack; IA64 for its Register Backing Store.
* vma is the last one with address > vma->vm_end. Have to extend vma.
@@ -1777,7 +1777,7 @@ int expand_stack_upwards(struct vm_area_struct *vma, unsigned long address)
khugepaged_enter_vma_merge(vma);
return error;
}
-#endif /* CONFIG_STACK_GROWSUP || CONFIG_IA64 */
+#endif /* VM_GROWSUP */

/*
* vma is the first one with address < vma->vm_start. Have to extend vma.
--
1.7.4.1

--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic


2011-04-20 00:33:33

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH followup] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64

> While I am in the cleanup mode. We should use VM_GROWSUP rather than
> tricky CONFIG_STACK_GROWSUP||CONFIG_IA64.
>
> What do you think?

Now, VM_GROWSUP share the same value with VM_NOHUGEPAGE.
(this trick use the fact that thp don't support any stack growup architecture)

So, No.
Sorry for that.



2011-04-20 06:59:47

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH followup] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64

Hi Kosaki,

On Wed 20-04-11 09:33:26, KOSAKI Motohiro wrote:
> > While I am in the cleanup mode. We should use VM_GROWSUP rather than
> > tricky CONFIG_STACK_GROWSUP||CONFIG_IA64.
> >
> > What do you think?
>
> Now, VM_GROWSUP share the same value with VM_NOHUGEPAGE.
> (this trick use the fact that thp don't support any stack growup architecture)

I am not sure I understand you. AFAICS, VM_GROWSUP is defined to non 0
only if CONFIG_STACK_GROWSUP||CONFIG_IA64 (include/linux/mm.h).
And we use it to determine whether expand_stack_growsup[*] should be
defined (in include/linux/mm.h).

The patch basically unifies the way how we export expand_stack_growsup
function and how define it (in mm/mmap.c).

So either we should use CONFIG_STACK_GROWSUP||CONFIG_IA64 at both places
or we should use VM_GROWSUP trick. I am for the later one.

Am I missing something?

---
[*] the previous patch renamed expand_growsup to expand_stack_growsup.
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic

2011-04-20 07:08:57

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH followup] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64

> Hi Kosaki,
>
> On Wed 20-04-11 09:33:26, KOSAKI Motohiro wrote:
> > > While I am in the cleanup mode. We should use VM_GROWSUP rather than
> > > tricky CONFIG_STACK_GROWSUP||CONFIG_IA64.
> > >
> > > What do you think?
> >
> > Now, VM_GROWSUP share the same value with VM_NOHUGEPAGE.
> > (this trick use the fact that thp don't support any stack growup architecture)
>
> I am not sure I understand you. AFAICS, VM_GROWSUP is defined to non 0
> only if CONFIG_STACK_GROWSUP||CONFIG_IA64 (include/linux/mm.h).
> And we use it to determine whether expand_stack_growsup[*] should be
> defined (in include/linux/mm.h).
>
> The patch basically unifies the way how we export expand_stack_growsup
> function and how define it (in mm/mmap.c).
>
> So either we should use CONFIG_STACK_GROWSUP||CONFIG_IA64 at both places
> or we should use VM_GROWSUP trick. I am for the later one.
>
> Am I missing something?
>
> ---
> [*] the previous patch renamed expand_growsup to expand_stack_growsup.

Right you are. sorry.


2011-04-26 07:59:25

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH followup] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64

Hi Andrew,
this one probably got lost in the follow up "parisc doesn't boot" email
storm.

On Tue 19-04-11 13:09:56, Michal Hocko wrote:
> While I am in the cleanup mode. We should use VM_GROWSUP rather than
> tricky CONFIG_STACK_GROWSUP||CONFIG_IA64.
>
> What do you think?
> ---
> From fd832dd46b4918718901f2ebe994f4662f167999 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <[email protected]>
> Date: Tue, 19 Apr 2011 11:11:41 +0200
> Subject: [PATCH] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64
>
> IA64 needs some trickery for Register Backing Store so we have to
> export expand_stack_upwards for it even though the architecture expands
> its stack downwards normally. To avoid
> we have defined VM_GROWSUP which is defined only for the above
> configuration.
>
> We still have places which use the original ifdefs so let's get rid of
> them finally.
>
> Signed-off-by: Michal Hocko <[email protected]>
> ---
> mm/mmap.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 29c68b0..3ff9edf 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1726,7 +1726,7 @@ static int acct_stack_growth(struct vm_area_struct *vma, unsigned long size, uns
> return 0;
> }
>
> -#if defined(CONFIG_STACK_GROWSUP) || defined(CONFIG_IA64)
> +#if VM_GROWSUP
> /*
> * PA-RISC uses this for its stack; IA64 for its Register Backing Store.
> * vma is the last one with address > vma->vm_end. Have to extend vma.
> @@ -1777,7 +1777,7 @@ int expand_stack_upwards(struct vm_area_struct *vma, unsigned long address)
> khugepaged_enter_vma_merge(vma);
> return error;
> }
> -#endif /* CONFIG_STACK_GROWSUP || CONFIG_IA64 */
> +#endif /* VM_GROWSUP */
>
> /*
> * vma is the first one with address < vma->vm_start. Have to extend vma.

--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic