Re: [PATCH resend] mm: get rid of CONFIG_STACK_GROWSUP ||CONFIG_IA64

[Hugh Dickins]

On Wed, 4 May 2011, Michal Hocko wrote:
> 
> This case is obscure enough already because we are using VM_GROWSUP to
> declare expand_stack_upwards in include/linux/mm.h

Ah yes, I didn't notice that it was already done that way there
(closer to the definitions of VM_GROWSUP so not as bad).

> while definition is guarded by CONFIG_STACK_GROWSUP||CONFIG_IA64. 
> What the patch does is just "make it consistent" thing. I think we
> should at least use CONFIG_STACK_GROWSUP||CONFIG_IA64 at both places if
> you do not like VM_GROWSUP misuse.

If it's worth changing anything, yes, that would be better.

> 
> > Not a nack: others may well disagree with me.
> > 
> > And, though I didn't find time to comment on your later "symmetrical"
> > patch before it went into mmotm, I didn't see how renaming expand_downwards
> > and expand_upwards to expand_stack_downwards and expand_stack_upwards was
> > helpful either - needless change, and you end up using expand_stack_upwards
> > on something which is not (what we usually call) the stack.
> 
> OK, I see your point. expand_stack_upwards in ia64_do_page_fault can be
> confusing as well. Maybe if we stick with the original expand_upwards
> and just make expand_downwards symmetrical without renameing to
> "_stack_" like the patch does? I can rework that patch if there is an
> interest. I would like to have it symmetrical, though, because the
> original code was rather confusing.

Yes, what I suggested before was an expand_upwards, an expand_downwards
and an expand_stack (with mod to fs/exec.c to replace its call to
expand_stack_downwards by direct call to expand_downwards).

But it's always going to be somewhat confusing and asymmetrical
because of the ia64 register backing store case.

Hugh
--
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/