On 2022/5/7 1:23, Bjorn Helgaas wrote:
Subject line convention looks like "numa: ..." On Fri, May 06, 2022 at 01:58:00AM +0000, Peng Liu wrote:Lots of code dosedoesnode != NUMA_NO_NODE && !node_online(node) or node == NUMA_NO_NODE || node_online(node) so create node_available to do this to simplify code.node_available()
Thanks.
I'm not really sure what meaning "node_available" conveys, though. Probably just because I don't understand NUMA. Should the test for NUMA_NO_NODE be folded into node_state() or node_online() directly instead of adding a new node_available() interface? NUMA_NO_NODE is -1. It's not clear to me that node_state()/ node_isset()/test_bit() would do the right thing given -1. I doubt all node_online() callers ensure they don't pass NUMA_NO_NODE.
I have tested node_online(NUMA_NO_NODE) on x86_64, arm64, arm32, the results are:
arch node_online(NUMA_NO_NODE)
------------------------------------------
x86_64 0
arm64 1
arm32 0
.
Hence, I think the behavior of node_state(NUMA_NO_NODE) is undefined which is due to
test_bit(NUMA_NO_NODE) is different between different arches.
For many times, callers could ensure they don't pass NUMA_NO_NODE, for example "nid < 0" is
checked before node_online() or some code only used in NUMA has node. So, it is not
suitable to test NUMA_NO_NODE in node_state() or node_online().
Unfortunately, there are actually some cases that test for NUMA_NO_NODE is needed but ignored.
Related issue: ab31c7fd2d37 ("sched/numa: Fix boot crash on arm64 systems")
--- a/include/linux/nodemask.h +++ b/include/linux/nodemask.h @@ -70,6 +70,7 @@ * * int node_online(node) Is some node online? * int node_possible(node) Is some node possible? + * int node_available(node) Is some node available(online or NUMA_NO_NODE)?Existing file generally fits in 80 columns; follow that lead unless you have a really good reason. E.g., maybe this? + * int node_available(node) Node online or NUMA_NO_NODE .
Thank you for your suggestion, I will send a revised patch with modified comments later.
