Date:   Thu, 16 Feb 2023 08:51:11 +0100
From:   Michal Hocko <mhocko@suse.com>
To:     Qi Zheng <zhengqi.arch@bytedance.com>
Cc:     akpm@linux-foundation.org, vbabka@suse.cz, david@redhat.com,
        rppt@kernel.org, willy@infradead.org, mgorman@techsingularity.net,
        osalvador@suse.de, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, Muchun Song <muchun.song@linux.dev>
Subject: Re: [PATCH 0/2] handle memoryless nodes more appropriately
Message-ID: <Y+3gb/blCDJnQ0Ik@dhcp22.suse.cz>
References: <20230215152412.13368-1-zhengqi.arch@bytedance.com>
 <Y+0KKnN8BU6ky6oP@dhcp22.suse.cz>
 <3426457c-99bf-9f7c-f663-c29474d9fa73@bytedance.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3426457c-99bf-9f7c-f663-c29474d9fa73@bytedance.com>
Precedence: bulk

On Thu 16-02-23 07:11:19, Qi Zheng wrote:
> 
> 
> On 2023/2/16 00:36, Michal Hocko wrote:
> > On Wed 15-02-23 23:24:10, Qi Zheng wrote:
> > > Hi all,
> > > 
> > > Currently, in the process of initialization or offline memory, memoryless
> > > nodes will still be built into the fallback list of itself or other nodes.
> > > 
> > > This is not what we expected, so this patch series removes memoryless
> > > nodes from the fallback list entirely.
> > > 
> > > Comments and suggestions are welcome.
> 
> Hi Michal,
> 
> > 
> > This is a tricky area full of surprises and it is really easy to
> 
> Would you mind giving an example of a "new problem"?

The initialization is spread over several places and it is quite easy to
introduce bugs because it is hard to review this area. Been there done
that. Just look into the git log.

> > introduce new problems. What kind of problem/issue are you trying to
> > solve/handle by these changes?
> 
> IIUC, I think there are two reasons:
> 
> Firstly, as mentioned in commit message, the memoryless node has no
> memory to allocate (If it can be allocated, it may also cause the panic
> I mentioned in [1]), so we should not continue to traverse it when
> allocating memory at runtime, which will have a certain overhead.

Sure that is not the most optimal implementation but does this matter in
practice? Can you observe any actual measurable performance penalty?
Currently we are just sacrificing some tiny performance for a
simplicity.
 
> Secondly, from the perspective of semantic correctness, why do we remove
> the memoryless node from the fallback list of other normal nodes
> (N_MEMORY), but not from its own fallback list (PATCH[1/2])? Why should
> an upcoming memoryless node continue exist in the fallback list of
> itself and other normal nodes (PATCH[2/2])?

I am not sure I follow. What is the semantic correctness issue?

-- 
Michal Hocko
SUSE Labs