Hi Jan,
On 13/12/2022 11:38, Jan Beulich wrote:
All callers convert frame numbers (perhaps in turn derived from struct
page_info pointers) to an address, just for the function to convert it
back to a frame number (as the first step of paddr_to_pdx()). Replace
the function by mfn_to_nid() plus a page_to_nid() wrapper macro. Replace
call sites by the respectively most suitable one.
While there also introduce a !NUMA stub, eliminating the need for Arm
(and potentially other ports) to carry one individually.
Signed-off-by: Jan Beulich <[email protected]>
---
At the top of free_heap_pages() mfn_to_nid() could also be used, since
the MFN is calculated immediately ahead. The choice of using
page_to_nid() (for now at least) was with the earlier patch's RFC in
mind, addressing of which may require to make mfn_to_nid() do weaker
checking than page_to_nid().
I haven't looked in details at the previous patch. However, I don't like
the idea of making mfn_to_nid() do weaker checking because this could
easily confuse the reader/developper.
If you want to use weaker check, then it would be better if a separate
helper is provided with a name reflecting its purpose.
--- a/xen/common/numa.c
+++ b/xen/common/numa.c
@@ -671,15 +671,15 @@ static void cf_check dump_numa(unsigned
for_each_online_node ( i )
{
- paddr_t pa = pfn_to_paddr(node_start_pfn(i) + 1);
+ mfn_t mfn = _mfn(node_start_pfn(i) + 1);
printk("NODE%u start->%lu size->%lu free->%lu\n",
i, node_start_pfn(i), node_spanned_pages(i),
avail_node_heap_pages(i));
- /* Sanity check phys_to_nid() */
- if ( phys_to_nid(pa) != i )
- printk("phys_to_nid(%"PRIpaddr") -> %d should be %u\n",
- pa, phys_to_nid(pa), i);
+ /* Sanity check mfn_to_nid() */
+ if ( node_spanned_pages(i) && mfn_to_nid(mfn) != i )
From the commit message, I would have expected that we would only
replace phys_to_nid() with either mfn_to_nid() or page_to_nid().
However, here you added node_spanned_pages(). Can you explain why?
Cheers,
--
Julien Grall