From: Baoquan He <bhe@redhat.com>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: John Hubbard <jhubbard@nvidia.com>,
akpm@linux-foundation.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, jgg@ziepe.ca, david@redhat.com
Subject: Re: [Patch v2 2/2] mm/page_alloc.c: define node_order with all zero
Date: Sat, 28 Mar 2020 08:51:44 +0800 [thread overview]
Message-ID: <20200328005144.GQ3039@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20200328002616.kjtf7dpkqbugyzi2@master>
On 03/28/20 at 12:26am, Wei Yang wrote:
> On Fri, Mar 27, 2020 at 03:37:57PM -0700, John Hubbard wrote:
> >On 3/27/20 3:01 PM, Wei Yang wrote:
> >> Since we always clear node_order before getting it, we can leverage
> >> compiler to do this instead of at run time.
> >>
> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> >> ---
> >> mm/page_alloc.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >> index dfcf2682ed40..49dd1f25c000 100644
> >> --- a/mm/page_alloc.c
> >> +++ b/mm/page_alloc.c
> >> @@ -5585,7 +5585,7 @@ static void build_thisnode_zonelists(pg_data_t *pgdat)
> >> static void build_zonelists(pg_data_t *pgdat)
> >> {
> >> - static int node_order[MAX_NUMNODES];
> >> + static int node_order[MAX_NUMNODES] = {0};
> >
> >
> >Looks wrong: now the single instance of node_order is initialized just once by
> >the compiler. And that means that only the first caller of this function
> >gets a zeroed node_order array...
> >
>
> What a shame on me. You are right, I miss the static word.
>
> Well, then I am curious about why we want to define it as static. Each time we
> call this function, node_order would be set to 0 and find_next_best_node()
> would sort a proper value into it. I don't see the reason to reserve it in a
> global area and be used next time.
>
> My suggestion is to remove the static and define it {0} instead of memset
> every time. Is my understanding correct here?
Removing static looks good, the node_order is calculated on the basis of
each node, it's useless for other node. It may be inherited from the old
code where it's a static global variable.
next prev parent reply other threads:[~2020-03-28 0:51 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-27 22:01 [Patch v2 1/2] mm/page_alloc.c: use NODE_MASK_NONE define used_mask Wei Yang
2020-03-27 22:01 ` [Patch v2 2/2] mm/page_alloc.c: define node_order with all zero Wei Yang
2020-03-27 22:37 ` John Hubbard
2020-03-27 23:18 ` Jason Gunthorpe
2020-03-28 0:27 ` Wei Yang
2020-03-28 0:26 ` Wei Yang
2020-03-28 0:51 ` Baoquan He [this message]
2020-03-28 0:59 ` John Hubbard
2020-03-28 1:10 ` Wei Yang
2020-03-28 1:28 ` John Hubbard
2020-03-28 2:56 ` Wei Yang
2020-03-29 1:30 ` John Hubbard
2020-03-29 2:31 ` Wei Yang
2020-03-28 11:25 ` Baoquan He
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200328005144.GQ3039@MiWiFi-R3L-srv \
--to=bhe@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=jgg@ziepe.ca \
--cc=jhubbard@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=richard.weiyang@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.