On 18.04.2022 11:07, Wei Chen wrote: > VIRTUAL_BUG_ON is an empty macro used in phys_to_nid. This > results in two lines of error-checking code in phys_to_nid > that is not actually working and causing two compilation > errors: > 1. error: "MAX_NUMNODES" undeclared (first use in this function). > This is because in the common header file, "MAX_NUMNODES" is > defined after the common header file includes the ARCH header > file, where phys_to_nid has attempted to use "MAX_NUMNODES". > This error was resolved when we moved the definition of > "MAX_NUMNODES" to x86 ARCH header file. And we reserve the > "MAX_NUMNODES" definition in common header file through a > conditional compilation for some architectures that don't > need to define "MAX_NUMNODES" in their ARCH header files.
No, that's setting up a trap for someone else to fall into, especially with the #ifdef around the original definition. Afaict all you need to do is to move that #define ahead of the #include in xen/numa.h. Unlike functions, #define-s can reference not-yet-defined identifiers. > 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 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. Jan
