Re: [PATCH] KVM: SVM: Consider NUMA affinity when allocating per-CPU save_area

[Sean Christopherson <seanjc@google.com>]

On Tue, Apr 23, 2024, Yosry Ahmed wrote:
> On Tue, Apr 23, 2024 at 6:30 AM Peter Gonda <pgonda@google.com> wrote:
> > > struct page *snp_safe_alloc_page(int cpu)
> > > {
> > >         unsigned long pfn;
> > >         struct page *p;
> > >         gfp_t gpf;
> > >         int node;
> > >
> > >         if (cpu >= 0) {
> > >                 node = cpu_to_node(cpu);
> > >                 gfp = GFP_KERNEL;
> > >         } else {
> > >                 node = NUMA_NO_NODE;
> > >                 gfp = GFP_KERNEL_ACCOUNT
> > >         }
> 
> FWIW, from the pov of someone who has zero familiarity with this code,
> passing @cpu only to make inferences about the GFP flags and numa
> nodes is confusing tbh.
> 
> Would it be clearer to pass in the gfp flags and node id directly? I
> think it would make it clearer why we choose to account the allocation
> and/or specify a node in some cases but not others.

Hmm, yeah, passing GFP directly makes sense, if only to align with alloc_page()
and not reinvent the wheel.  But forcing all callers to explicitly provide a node
ID is a net negative, i.e. having snp_safe_alloc_page() and snp_safe_alloc_page_node()
as originally suggested makes sense.

But snp_safe_alloc_page() should again flow alloc_pages() and pass numa_node_id(),
not NUMA_NO_NODE.