All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oscar Salvador <osalvador@techadventures.net>
To: Pasha Tatashin <Pavel.Tatashin@microsoft.com>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"mhocko@suse.com" <mhocko@suse.com>,
	"dan.j.williams@intel.com" <dan.j.williams@intel.com>,
	"david@redhat.com" <david@redhat.com>,
	"Jonathan.Cameron@huawei.com" <Jonathan.Cameron@huawei.com>,
	"yasu.isimatu@gmail.com" <yasu.isimatu@gmail.com>,
	"malat@debian.org" <malat@debian.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Oscar Salvador <osalvador@suse.de>
Subject: Re: [PATCH 4/5] mm/memory_hotplug: Simplify node_states_check_changes_online
Date: Fri, 21 Sep 2018 12:30:29 +0200	[thread overview]
Message-ID: <20180921103029.GA15555@techadventures.net> (raw)
In-Reply-To: <71676241-8aa5-2b58-b2fa-706bf21b9cfb@microsoft.com>

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

  reply	other threads:[~2018-09-21 10:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19 10:08 [PATCH 0/5] Refactor node_states_check_changes_online/offline Oscar Salvador
2018-09-19 10:08 ` [PATCH 1/5] mm/memory_hotplug: Spare unnecessary calls to node_set_state Oscar Salvador
2018-09-20 20:53   ` Pasha Tatashin
2018-09-19 10:08 ` [PATCH 2/5] mm/memory_hotplug: Avoid node_set/clear_state(N_HIGH_MEMORY) when !CONFIG_HIGHMEM Oscar Salvador
2018-09-20 20:59   ` Pasha Tatashin
2018-09-21 10:31     ` Oscar Salvador
2018-09-19 10:08 ` [PATCH 3/5] mm/memory_hotplug: Tidy up node_states_clear_node Oscar Salvador
2018-09-20 23:40   ` Pasha Tatashin
2018-09-19 10:08 ` [PATCH 4/5] mm/memory_hotplug: Simplify node_states_check_changes_online Oscar Salvador
2018-09-21  0:15   ` Pasha Tatashin
2018-09-21 10:30     ` Oscar Salvador [this message]
2018-09-19 10:08 ` [PATCH 5/5] mm/memory_hotplug: Clean up node_states_check_changes_offline Oscar Salvador
2018-09-21  0:38   ` Pasha Tatashin

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=20180921103029.GA15555@techadventures.net \
    --to=osalvador@techadventures.net \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=Pavel.Tatashin@microsoft.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=malat@debian.org \
    --cc=mhocko@suse.com \
    --cc=osalvador@suse.de \
    --cc=yasu.isimatu@gmail.com \
    --subject='Re: [PATCH 4/5] mm/memory_hotplug: Simplify node_states_check_changes_online' \
    /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

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.