Re: [PATCH] driver core: ensure a device has valid node id in device_add()

From: Michal Hocko <mhocko@kernel.org>
To: Yunsheng Lin <linyunsheng@huawei.com>
Cc: gregkh@linuxfoundation.org, rafael@kernel.org,
	linux-kernel@vger.kernel.org, peterz@infradead.org,
	mingo@kernel.org, linuxarm@huawei.com
Subject: Re: [PATCH] driver core: ensure a device has valid node id in device_add()
Date: Tue, 10 Sep 2019 09:24:25 +0200	[thread overview]
Message-ID: <20190910072425.GI2063@dhcp22.suse.cz> (raw)
In-Reply-To: <07576292-e129-5949-6a2e-45fff067ca5a@huawei.com>

Our emails crossed, sorry about that.

On Tue 10-09-19 15:08:20, Yunsheng Lin wrote:
> On 2019/9/10 2:50, Michal Hocko wrote:
> > On Mon 09-09-19 14:04:23, Yunsheng Lin wrote:
[...]
> >> Even if a device's numa node is not specified, the device really
> >> does belong to a node.
> > 
> > What does this mean?
> 
> It means some one need to guess the node id if the node is not
> specified.

I have asked about this in other email so let's not cross the
communication even more.

> >> This patch sets the device node to node 0 in device_add() if the
> >> device's node id is not specified and it either has no parent
> >> device, or the parent device also does not have a valid node id.
> > 
> > Why is node 0 special? I have seen platforms with node 0 missing or
> > being memory less. The changelog also lacks an actual problem
> 
> by node 0 missing, how do we know if node 0 is missing?
> by node_online(0)?

No, this is a dynamic situation. Node might get offline via hotremove.
In most cases it wouldn't because there will likely be some kernel
memory on node0 but you cannot really make any assumptions here. Besides
that nothing should really care.

> > descripton. Why do we even care about NUMA_NO_NODE? E.g. the page
> > allocator interprets NUMA_NO_NODE as the closest node with a memory.
> > And by closest it really means to the CPU which is performing the
> > allocation.
> 
> Yes, I should have mentioned that in the commit log.
> 
> I mentioned the below in the RFC, but somehow deleted when sending
> V1:
> "There may be explicit handling out there relying on NUMA_NO_NODE,
> like in nvme_probe()."

This code, and other doing similar things, is very likely bogus. Just
look at what the code does. It takes the node affinity from the dev and
uses it for an allocation. So far so good. But it tries to be clever
and special cases NUMA_NO_NODE to be first_node. So now the allocator
has used a proper fallback to the nearest node with memory for the
current CPU that is executing the code while dev will point to a
first_node which might be a completely different one. See the
discrepancy?
-- 
Michal Hocko
SUSE Labs