Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 21 Feb 2022 10:47:44 +0100
From:   Oscar Salvador <osalvador@suse.de>
To:     Michal Hocko <mhocko@suse.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>,
        Rafael Aquini <raquini@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Wei Yang <richard.weiyang@gmail.com>,
        Dennis Zhou <dennis@kernel.org>,
        Alexey Makhalov <amakhalov@vmware.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] arch/x86/mm/numa: Do not initialize nodes twice
Message-ID: <YhNfwCO3xTIavi5v@localhost.localdomain>
References: <20220218224302.5282-1-osalvador@suse.de>
 <20220218224302.5282-2-osalvador@suse.de>
 <YhNZQgGSZglGQvcg@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YhNZQgGSZglGQvcg@dhcp22.suse.cz>
Precedence: bulk

On Mon, Feb 21, 2022 at 10:20:02AM +0100, Michal Hocko wrote:
> On Fri 18-02-22 23:43:02, Oscar Salvador wrote:
> > Why setting the node online saves us then? Well, simply because
> > __try_online_node() backs off when the node is online, meaning
> > we do not end up calling register_one_node() in the first place.
> 
> This is really a mess and a house built on sand. Thanks for looking into
> it and hopefully this can get cleaned up to a saner state.

Yes, I am willing to have a deep look into that and see how we can
improve the situation.

> This sha1 is from linux-next very likely so it won't be persistent.
> Please drop it.

Yes, it is. I guess it is fine to not have a "Fixes" tag here, so I will
remove it then.

> I would stick a TODO here.
> 			/*
> 			 * Exclude this node from 
> 			 * bringup_nonboot_cpus
> 			 *  cpu_up
> 			 *    __try_online_node
> 			 *      register_one_node
> 			 * because node_subsys is not initialized yet
> 			 * TODO remove dependency on node_online()
> 			 */

Sure, will do.

Thanks!

-- 
Oscar Salvador
SUSE Labs