Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180601125321.30652-1-osalvador@techadventures.net> <20180601125321.30652-4-osalvador@techadventures.net>
In-Reply-To: <20180601125321.30652-4-osalvador@techadventures.net>
From:   Pavel Tatashin <pasha.tatashin@oracle.com>
Date:   Wed, 20 Jun 2018 22:35:18 -0400
Message-ID: <CAGM2reb6p-ffZ6-JDc5vqMkyDNDn9siWjEy9LnVJ1BtTKYhegA@mail.gmail.com>
Subject: Re: [PATCH 3/4] mm/memory_hotplug: Get rid of link_mem_sections
To:     osalvador@techadventures.net
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Michal Hocko <mhocko@suse.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Linux Memory Management List <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>, osalvador@suse.de
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 1, 2018 at 8:54 AM <osalvador@techadventures.net> wrote:
>
> From: Oscar Salvador <osalvador@suse.de>
>
> link_mem_sections() and walk_memory_range() share most of the code,
> so we can use walk_memory_range() with a callback to register_mem_sect_under_node()
> instead of using link_mem_sections().

Yes, their logic is indeed identical, so it is good to replace some
code with walk_memory_range().

>
> To control whether the node id must be check, two new functions has been added:
>
> register_mem_sect_under_node_nocheck_node()
> and
> register_mem_sect_under_node_check_node()

I do not like this, please see if my suggestion is better:

1. Revert all the changes outside of  link_mem_sections()
2. Remove check_nid argument from register_mem_sect_under_node
and link_mem_sections.
3. In register_mem_sect_under_node
Replace:

if (check_nid) {
}

With:
if (system_state == SYSTEM_BOOTING) {
}

4. Change register_mem_sect_under_node() prototype to match callback
of walk_memory_range()
5. Call walk_memory_range(... register_mem_sect_under_node ...) from
link_mem_sections

Thank you,
Pavel