Messages in this thread | | | Date | Sat, 28 Mar 2020 00:27:05 +0000 | From | Wei Yang <> | Subject | Re: [Patch v2 2/2] mm/page_alloc.c: define node_order with all zero |
| |
On Fri, Mar 27, 2020 at 08:18:20PM -0300, Jason Gunthorpe 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 >> > +++ 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... > >It is also redundant, all static data is 0 initialized in Linux and >should not be explicitly initialized so it can remain in .bss >
Yeah, you are right. I missed the static word.
>> > @@ -5595,7 +5595,6 @@ static void build_zonelists(pg_data_t *pgdat) >> > load = nr_online_nodes; >> > prev_node = local_node; >> > - memset(node_order, 0, sizeof(node_order)); >> >> ...and all subsequent callers are left with whatever debris is remaining in >> node_order. So this is not good. > >Indeed > >Jason
-- Wei Yang Help you, Help me
| |