Re: [PATCH] mm/page_alloc: remove the static for local variable node_order

[Michal Hocko <mhocko@suse.com>]

On Wed 30-12-20 21:41:30, Muchun Song wrote:
> On Wed, Dec 30, 2020 at 8:45 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Wed, Dec 30, 2020 at 07:40:14PM +0800, Hui Su wrote:
> > > local variable node_order do not need the static here.
> >
> > It bloody well does.  It can be up to 2^10 entries on x86 (and larger
> > on others) That's 4kB which you've now moved onto the stack.
> 
> This is not the first time I have seen similar changes. So what
> do you think about adding a comment here to avoid another one
> do this in the feature?

Well, this is not an unusual technieque to reduce the stack space. I am
not really sure we really need to put an explicit comment about that.  I
would appreciate much more if patch submitters took an extra step when
creating seemingly trivial patches and either consult the history of the
respective code or look for a similar pattern elsewhere before sending
them.

I do agree with Willy that mm code is usually not a great place to
search for trivial patches. First of all most people tend to be pretty
busy with other reviewes and the code has grown rather delicate and
tricky so each review is non trivial.
-- 
Michal Hocko
SUSE Labs