Re: [Xen-devel] [PATCH] Xen: Fix retry calls into PRIVCMD_MMAPBATCH*.

[David Vrabel <david.vrabel@xxxxxxxxxx>]

On 09/08/13 15:11, Andres Lagar-Cavilla wrote:
> From: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> 
> When a foreign mapper attempts to map guest frames that are paged out,
> the mapper receives an ENOENT response and will have to try again
> while a helper process pages the target frame back in.
> 
> Gating checks on PRIVCMD_MMAPBATCH* ioctl args were preventing retries
> of mapping calls.
> 
> V2: Fixed autotranslated physmap mode breakage introduced by V1.
> V3: Enforce full range mapping for first ioctl call on a given VMA.
> 
> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> ---
>  drivers/xen/privcmd.c |   58 +++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index f8e5dd7..938e6e6 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -43,9 +43,12 @@ MODULE_LICENSE("GPL");
>  
>  #define PRIV_VMA_LOCKED ((void *)1)
>  
> -#ifndef HAVE_ARCH_PRIVCMD_MMAP
>  static int privcmd_enforce_singleshot_mapping(struct vm_area_struct *vma);
> -#endif
> +
> +static int privcmd_enforce_singleshot_mapping_granular(
> +               struct vm_area_struct *vma,
> +               unsigned long addr,
> +               unsigned long nr_pages);
>  
>  static long privcmd_ioctl_hypercall(void __user *udata)
>  {
> @@ -422,19 +425,26 @@ static long privcmd_ioctl_mmap_batch(void __user 
> *udata, int version)
>       vma = find_vma(mm, m.addr);
>       if (!vma ||
>           vma->vm_ops != &privcmd_vm_ops ||
> -         (m.addr != vma->vm_start) ||
> -         ((m.addr + (nr_pages << PAGE_SHIFT)) != vma->vm_end) ||
> -         !privcmd_enforce_singleshot_mapping(vma)) {
> +         (m.addr < vma->vm_start) ||
> +         ((m.addr + (nr_pages << PAGE_SHIFT)) > vma->vm_end) ||
> +         !privcmd_enforce_singleshot_mapping_granular(vma, m.addr, 
> nr_pages)) {
>               up_write(&mm->mmap_sem);
>               ret = -EINVAL;
>               goto out;

I think this unnecessarily calls
privcmd_enforce_singleshot_mapping_granular() on the first call.

If you change the tests to the following, does it work for you?

/* Suitable VMA? */
if (!vma || vma->vm_ops != &privcmd_vm_ops)
        // error

if (!privcmd_enforce_singleshot_mapping(..)) {
        /* Already populated into this VMA once, are we populating a
           subset that is not yet populated? */
        if (is_subset_of_vma(vma, m,addr, nr_pages)
            && privcmd_enforce_single_shot_mapping_granular())
                // ok
        else
                // error
} else {
        /* Nothing populated in this VMA, must try to fully populate
           it. */
        if (is_all_of_vma(vma, m.addr, nr_pages))
                // ok
        else
                // error
}


Thanks.

David

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