[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/6] mm: New XENMEM, XENMEM_add_to_physmap_gmfn_range
At 13:32 +0000 on 08 Nov (1320759148), Tim Deegan wrote: > At 02:20 -0700 on 08 Nov (1320718835), Jan Beulich wrote: > > >>> On 07.11.11 at 19:25, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote: > > > > Sorry, noticed this only now, but neither title nor description of this > > are in sync with the actual patch. > > > Indeed; they should be updated. But otherwise: > Acked-by: Tim Deegan <tim@xxxxxxx> No, wait, that was the other patch, which I already acked! ENOCOFFEE. :( I have a few other comments on this patch... > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > index f75011e..cca64b8 100644 > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -4714,9 +4714,29 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) > return -EPERM; > } > > - xenmem_add_to_physmap(d, xatp); > + if ( xatp.space != XENMAPSPACE_gmfn_range ) > + xatp.size = 1; > + > + for ( ; xatp.size > 0; xatp.size-- ) > + { > + if ( hypercall_preempt_check() ) > + { > + rc = -EAGAIN; > + break; > + } > + xenmem_add_to_physmap(d, xatp); > + xatp.idx++; > + xatp.gpfn++; > + } Having moved XATP into its own function, this loop probably belongs in that function. While I'm looking at it, updating two loop vars explicitly and one in the header is a bit ugly - why not just use a while() and update all three together? > rcu_unlock_domain(d); > + if ( rc == -EAGAIN ) > + { > + if ( copy_to_guest(arg, &xatp, 1) ) > + return -EFAULT; > + rc = hypercall_create_continuation( > + __HYPERVISOR_memory_op, "ih", op, arg); > + } I think this might need some compat glue in arch/x86/x86_64/compat/mm.c for it to work with 32-bit callers. Tim. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |