Re: [Xen-devel] [PATCH RFC 1/2] linux/vnuma: vnuma support for pv guest

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On gio, 2013-08-29 at 10:51 -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Aug 29, 2013 at 03:32:13PM +0100, George Dunlap wrote:
> > On 29/08/13 15:23, Konrad Rzeszutek Wilk wrote:
> > >  - Why not re-use existing code? As in, if both PV and HVM can use
> > >    SRAT, why not do it that way? This way you are exercising the same
> > >    code path in both guests and it means less bugs to chase.
> > >
> > >    Incidententaly this also means that the mechanism to fetch the
> > >    NUMA information from the hypervisor and construct the SRAT tables
> > >    from can be done the same in both hvmloader and Linux kernel.
> > 
> > If the SRAT tables are built by hvmloader for HVM guests, couldn't
> > hvmloader make this hypercall and construct the tables, instead of
> > having the domain builder do it?  That would mean that most of the
> > codepath is the same; HVM just has the extra step of encoding and
> > decoding the information in SRAT form.
> 
> Correct. 
>
Indeed. Also, Matt mentioned the HVM implementation does the most of the
work in libxc and hvmloader, is that the case? (Matt, I knew about the
"old" HVM-NUMA series from Andre, but I don't have the details fresh
enough right now... Will take another look ASAP.)

If yes, please, consider that, when talking about PV-vNUMA, although
right now the series addresses DomU only, we plan to make it possible
for Dom0 to have a virtual NUMA topology too. In that case, I don't
think any code from libxc and hvmloader could be shared, could it?

So, to me, it sort of looks like we risk to introduce more code
duplication than what we're trying to avoid! :-P

> > >I think short term Elena's option is better one as it gets it going and
> > >we can experiement with it. Long term I think stashing the data in ACPI
> > >SRAT/SLIT is right. But Elena might not get to it in the next couple of
> > >months - which means somebody else will have to sign up for that.
> > 
> > I definitely think that Elena needs to continue on the same path for
> > now, so she can actually have closure on the project in a reasonable
> > amount of time.
> 
> I concur. The caveat is that it could mean that the x86 maintainers might
> object to the generic path having the special case for Xen and that
> particular patch part won't be upstreamed until it has been taken care of.
> 
> If Elena is OK with that possiblity  - then that is fine with me.
>
Sorry, I'm not sure I understand what you mean here. When talking about
PV-NUMA, with Elena's approach, we have a Xen special case in
numa_init(). With the fake table approach, we'd have a Xen special case
in the ACPI parsing code (Matt was talking about something like
acpi_os_get_root_pointer() ), wouldn't we?

So, either way, we'll have a Xen special case, the only difference I see
is that, in the latter, we'd probably have to deal with the ACPI
maintainers instead than with the x86 ones. :-)

FWIW, x86_numa_init() already looks like this:

 621 void __init x86_numa_init(void)
 622 {
 623         if (!numa_off) {
 624 #ifdef CONFIG_X86_NUMAQ
 625                 if (!numa_init(numaq_numa_init))
 626                         return;
 627 #endif
 628 #ifdef CONFIG_ACPI_NUMA
 629                 if (!numa_init(x86_acpi_numa_init))
 630                         return;
 631 #endif
 632 #ifdef CONFIG_AMD_NUMA
 633                 if (!numa_init(amd_numa_init))
 634                         return;
 635 #endif
 636         }
 637 
 638         numa_init(dummy_numa_init);
 639 }

I.e., quite a bit of architecture/implementation special casing already!
So, perhaps, having some more `#ifdef CONFIG_XEN' and/or `if
(xen_domain())' in there won't be that much upsetting after all. :-)

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel