Re: [Xen-devel] [PATCH] Improve performance of IOCTL_PRIVCMD_MMAPBATCH_V2

[Mats Petersson <mats.petersson@xxxxxxxxxx>]

On 21/11/12 10:29, Jan Beulich wrote:
> > > > On 16.11.12 at 15:45, Mats Petersson <mats.petersson@xxxxxxxxxx> wrote:
> > @@ -2526,12 +2540,25 @@ int xen_remap_domain_mfn_range(struct vm_area_struct 
> > *vma,
> >                 if (err)
> >                         goto out;
> >   
> > -		err = HYPERVISOR_mmu_update(mmu_update, batch, NULL, domid);
> > -               if (err < 0)
> > -                       goto out;
> > +               /* We record the error for each page that gives an error, but
> > +                * continue mapping until the whole set is done */
> > +               do {
> > +                       err = HYPERVISOR_mmu_update(&mmu_update[index],
> > +                                                   batch_left, &done, domid);
> > +                       if (err < 0) {
> > +                               if (err_ptr)
> > +                                       err_ptr[index] = err;
> Shouldn't you increment "done" here, in order to not retry the failed
> slot immediately?
Yes, good spot - for some reason, I have double checked the behaviour of 
"done", and it returns the index of the item which gave the error, not 
actually "how many were processed".
I have rewritten this part of code for V3 of this patch, but I think it 
still requires an increment of "done" to make it work correctly.
> > +                               else /* exit if error and no err_ptr */
> > +                                       goto out;
> > +                       }
> > +                       batch_left -= done;
> > +                       index += done;
> > +               } while (batch_left);
> >   
> >   		nr -= batch;
> >                 addr += range;
> > +               if (err_ptr)
> > +                       err_ptr += batch;
> >         }
> >   
> >   	err = 0;
> > @@ -303,6 +349,8 @@ static int mmap_return_errors_v1(void *data, void *state)
> >         return __put_user(*mfnp, st->user_mfn++);
> >   }
> >   
> > +
> > +
> ???
This is cleaned up in the V3 patch.
> >   static struct vm_operations_struct privcmd_vm_ops;
> >   
> >   static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> > @@ -319,6 +367,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata,
> > int version)
> >         if (!xen_initial_domain())
> >                 return -EPERM;
> >   
> > +
> > +
> ???
As above.

--
Mats
> Jan
>
> >         switch (version) {
> >         case 1:
> >                 if (copy_from_user(&m, udata, sizeof(struct privcmd_mmapbatch)))

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