| Message ID | 20180919100819.25518-3-osalvador@techadventures.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor node_states_check_changes_online/offline | expand |
On 9/19/18 6:08 AM, Oscar Salvador wrote: > From: Oscar Salvador <osalvador@suse.de> > > Currently, when !CONFIG_HIGHMEM, status_change_nid_high is being set > to status_change_nid_normal, but on such systems N_HIGH_MEMORY falls > back to N_NORMAL_MEMORY. > That means that if status_change_nid_normal is not -1, > we will perform two calls to node_set_state for the same memory type. > > Set status_change_nid_high to -1 for !CONFIG_HIGHMEM, so we skip the > double call in node_states_set_node. > > The same goes for node_clear_state. > > Signed-off-by: Oscar Salvador <osalvador@suse.de> Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com> This is a rare case where I think comments are unnecessary as the code is self explanatory. So, I would remove the comments before: > + arg->status_change_nid_high = -1; Pavel > --- > mm/memory_hotplug.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 63facfc57224..c2c7359bd0a7 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -731,7 +731,11 @@ static void node_states_check_changes_online(unsigned long nr_pages, > else > arg->status_change_nid_high = -1; > #else > - arg->status_change_nid_high = arg->status_change_nid_normal; > + /* > + * When !CONFIG_HIGHMEM, N_HIGH_MEMORY equals N_NORMAL_MEMORY > + * so setting the node for N_NORMAL_MEMORY is enough. > + */ > + arg->status_change_nid_high = -1; > #endif > > /* > @@ -1555,7 +1559,11 @@ static void node_states_check_changes_offline(unsigned long nr_pages, > else > arg->status_change_nid_high = -1; > #else > - arg->status_change_nid_high = arg->status_change_nid_normal; > + /* > + * When !CONFIG_HIGHMEM, N_HIGH_MEMORY equals N_NORMAL_MEMORY > + * so clearing the node for N_NORMAL_MEMORY is enough. > + */ > + arg->status_change_nid_high = -1; > #endif > > /* >
On Thu, Sep 20, 2018 at 08:59:18PM +0000, Pasha Tatashin wrote: > This is a rare case where I think comments are unnecessary as the code > is self explanatory. So, I would remove the comments before: Fair enough. I just wanted to make clear why it was not needed. I will remove it in the next version. Thanks
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 63facfc57224..c2c7359bd0a7 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -731,7 +731,11 @@ static void node_states_check_changes_online(unsigned long nr_pages, else arg->status_change_nid_high = -1; #else - arg->status_change_nid_high = arg->status_change_nid_normal; + /* + * When !CONFIG_HIGHMEM, N_HIGH_MEMORY equals N_NORMAL_MEMORY + * so setting the node for N_NORMAL_MEMORY is enough. + */ + arg->status_change_nid_high = -1; #endif /* @@ -1555,7 +1559,11 @@ static void node_states_check_changes_offline(unsigned long nr_pages, else arg->status_change_nid_high = -1; #else - arg->status_change_nid_high = arg->status_change_nid_normal; + /* + * When !CONFIG_HIGHMEM, N_HIGH_MEMORY equals N_NORMAL_MEMORY + * so clearing the node for N_NORMAL_MEMORY is enough. + */ + arg->status_change_nid_high = -1; #endif /*