On 27.04.2022 05:52, Wei Chen wrote: >> From: Jan Beulich <jbeul...@suse.com> >> Sent: 2022年4月26日 22:42 >> >> On 26.04.2022 12:59, Wei Chen wrote: >>> On 2022/4/26 17:02, Jan Beulich wrote: >>>> On 18.04.2022 11:07, Wei Chen wrote: >>>>> 2. error: wrong type argument to unary exclamation mark. >>>>> This is because, the error-checking code contains !node_data[nid]. >>>>> But node_data is a data structure variable, it's not a pointer. >>>>> >>>>> So, in this patch, we use ASSERT instead of VIRTUAL_BUG_ON to >>>>> enable the two lines of error-checking code. And fix the left >>>>> compilation errors by replacing !node_data[nid] to >>>>> !node_data[nid].node_spanned_pages. >>>>> >>>>> Because when node_spanned_pages is 0, this node has no memory, >>>>> numa_scan_node will print warning message for such kind of nodes: >>>>> "Firmware Bug or mis-configured hardware?". >>>> >>>> This warning is bogus - nodes can have only processors. Therefore I'd >>>> like to ask that you don't use it for justification. And indeed you >>> >>> Yes, you're right, node can only has CPUs! I will remove it. >>> >>>> don't need to: phys_to_nid() is about translating an address. The >>>> input address can't be valid if it maps to a node with no memory. >>>> >>> >>> Can I understand your comment: >>> Any input address is invalid, when node_spanned_pages is zero, because >>> this node has no memory? >> >> It's getting close, but it's not exactly equivalent I think. A node >> with 0 bytes of memory might (at least in theory) have an entry in >> memnodemap[]. But finding a node ID for that address would still > > I have done a quick check in populate_memnodemap: > 74 spdx = paddr_to_pdx(nodes[i].start); > 75 epdx = paddr_to_pdx(nodes[i].end - 1) + 1; > 76 if ( spdx >= epdx ) > 77 continue; > > It seems that if node has no memory, start == end, then this function > will not populate memnodemap entry for this node. > >> not mean that at least one byte of memory at that address is present >> on the given node, because the node covers 0 bytes. >> > > And back to this patch, can I just drop the unnecessary justification > from the commit message?
Well, you'll want to have _some_ justification for that particular aspect of the patch. > And for the bogus warning message, can I update it to an INFO level > message in part#2 series, and just keep: > printk(KERN_INFO "SRAT: Node %u has no memory!\n", i); > but remove "BIOS Bug or mis-configured hardware?\n", i); ? This sounds at least plausible to do. Jan
