Re: [PATCH 4/5] mm/memory_hotplug: Simplify node_states_check_changes_online

[Oscar Salvador <osalvador@techadventures.net>]

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