On Fri, Sep 21, 2018 at 12:15:53AM +0000, Pasha Tatashin wrote: Hi Pavel, > But what if that changes, will this function need to change as well? That's true. > Should not we have: > else > arg->status_change_nid_high = -1; ? > > > + } else > > + arg->status_change_nid_high = -1; Yes, I forgot about that else. > I think it is simpler to have something like this: > > int nid = zone_to_nid(zone); > > arg->status_change_nid_high = -1; > arg->status_change_nid = -1; > arg->status_change_nid = -1; > > if (!node_state(nid, N_MEMORY)) > arg->status_change_nid = nid; > if (zone_idx(zone) <= ZONE_NORMAL && !node_state(nid, N_NORMAL_MEMORY)) > arg->status_change_nid_normal = nid; > #ifdef CONFIG_HIGHMEM > if (zone_idx(zone) <= N_HIGH_MEMORY && !node_state(nid, N_HIGH_MEMORY)) > arg->status_change_nid_high = nid; > #endif I can write it that way, I also like it more. I will send it in V2. Thanks for reviewing it! -- Oscar Salvador SUSE L3