[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 3/4] libxc: rework vnuma bits in setup_guest
On Wed, Jun 03, 2015 at 11:57:33AM +0100, Ian Campbell wrote: > On Wed, 2015-06-03 at 11:39 +0100, Wei Liu wrote: > > On Wed, Jun 03, 2015 at 11:34:49AM +0100, Ian Campbell wrote: > > > On Mon, 2015-06-01 at 16:18 -0400, Boris Ostrovsky wrote: > > > > On 06/01/2015 06:19 AM, Wei Liu wrote: > > > > > Make the setup process similar to PV counterpart. That is, to > > > > > allocate a > > > > > P2M array that covers the whole memory range and start from there. > > > > > This > > > > > is clearer than using an array with no holes in it. > > > > > > > > > > Also the dummy layout should take MMIO hole into consideration. We > > > > > might > > > > > end up having two vmemranges in the dummy layout. > > > > > > > > > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx> > > > > > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx> > > > > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> > > > > > > > > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> > > > > > > + my ack on all 4 -> applied. > > > > > > > with a couple of nits below that you might consider. > > > > > > These can be done in followups, so I applied. > > > > > > I have a passing comment on one of the nits: > > > > > > > > + for ( vmemid = 0; vmemid < args->nr_vmemranges; vmemid++ ) > > > > > + { > > > > > + uint64_t pfn; > > > > > + > > > > > + for ( pfn = args->vmemranges[vmemid].start >> PAGE_SHIFT; > > > > > + pfn < args->vmemranges[vmemid].end >> PAGE_SHIFT; > > > > > + pfn++ ) > > > > > + page_array[pfn] = pfn; > > > > > + } > > > > > > > > > > /* > > > > > * Try to claim pages for early warning of insufficient memory > > > > > available. > > > > > @@ -645,6 +679,12 @@ static int setup_guest(xc_interface *xch, > > > > > error_out: > > > > > rc = -1; > > > > > out: > > > > > + if ( use_dummy ) > > > > > > > > Or 'if (args->vmemranges == dummy_vmemrange)' and drop use_dummy > > > > variable. > > > > > > FWIW a more normal approach would be to do all the calculations on local > > > variables and propagate them to the caller in the !use_dummy case. I > > > don't know if that implies some huge restructure of this function > > > though. > > > > There is no calculation with regard to vNUMA that needs to be propagated > > to the caller in !use_dummy case. So this point is moot. > > The code returns with args->{nr_vnodes,vmemranges,vnode_to_pnode} having > been updated, if that information should not be propoagated to the > caller then surely it should be blown away irrespective of the dummy or > not (and in that case why is it being done in that struct at all and not > in some local variables freed on exit) > That's because the code below references those fields in args and nothing more. I can refactor this function to have local pointers either point to dummy or args->*. Wei. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |