Re: [Xen-devel] [PATCH v7 01/21] xen: make two memory hypercalls vNUMA-aware

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Mar 09, 2015 at 01:52:28PM +0000, Jan Beulich wrote:
> >>> On 09.03.15 at 14:29, <wei.liu2@xxxxxxxxxx> wrote:
> > On Mon, Mar 09, 2015 at 01:22:55PM +0000, Jan Beulich wrote:
> >> >>> On 09.03.15 at 13:51, <wei.liu2@xxxxxxxxxx> wrote:
> >> > @@ -759,6 +784,14 @@ long do_memory_op(unsigned long cmd, 
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >> >              return start_extent;
> >> >          args.domain = d;
> >> >  
> >> > +        if ( construct_memop_from_reservation(d, &reservation, &args) )
> >> > +        {
> >> > +            rcu_unlock_domain(d);
> >> > +            return start_extent;
> >> > +        }
> >> > +        args.nr_done      = start_extent;
> >> > +        args.preempted    = 0;
> >> 
> >> Remembering Andrew's comment on your patch introducing
> >> construct_memop_from_reservation(), the setting of
> >> args.domain visible in the context above should either be
> >> moved past that call, or be used by the function instead of
> >> passing d explicitly (in which case it is pretty clear that the
> >> function isn't free to clobber that structure field).
> >> 
> > 
> > My understanding was that he wanted me to move setting args.nr_done and
> > args.preempted after calling construct_memop_from_reservation. But then
> > here I missed args.domain. :-/
> > 
> > I will just move "args.domain = d" after the function call, if that's OK
> > with you.
> 
> Yes, as said, either adjustment is fine with me.
> 
> But looking at this another time points out another issue with the
> patch - you now clobber MEMF_populate_on_demand possibly
> being set in args.memflags, i.e. you need to move down that part
> too.
> 

Sure, I will check that as well.

Wei.

> Jan

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