WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xen/gntdev,gntalloc: Remove unneeded VM flags

To: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xen/gntdev,gntalloc: Remove unneeded VM flags
From: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Date: Mon, 7 Mar 2011 19:53:49 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "konrad.wilk@xxxxxxxxxx" <konrad.wilk@xxxxxxxxxx>
Delivery-date: Mon, 07 Mar 2011 11:54:37 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1299521976-8952-1-git-send-email-dgdegra@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <1299521976-8952-1-git-send-email-dgdegra@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Mon, 2011-03-07 at 18:19 +0000, Daniel De Graaf wrote:
> The only time when granted pages need to be treated specially is when
> using Xen's PTE modification for grant mappings owned by another domain.
> Otherwise, the area does not require VM_DONTCOPY and VM_PFNMAP, since it
> can be accessed just like any other page of RAM.

This needs clarifying that it only applies to gntalloc and/or HVM
guests, I don't think it applies to gntdev in PV guests, right?

The actual patch seems to make some reference counting changes which
don't seem to be covered by the above description, and they don't seem
to be balanced (I see increments but no decrements).

Ian.

> 
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
>  drivers/xen/gntalloc.c |   14 ++++++++++++--
>  drivers/xen/gntdev.c   |   16 ++++++++++++++--
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c
> index a7ffdfe..f6832f4 100644
> --- a/drivers/xen/gntalloc.c
> +++ b/drivers/xen/gntalloc.c
> @@ -427,6 +427,17 @@ static long gntalloc_ioctl(struct file *filp, unsigned 
> int cmd,
>       return 0;
>  }
>  
> +static void gntalloc_vma_open(struct vm_area_struct *vma)
> +{
> +     struct gntalloc_gref *gref = vma->vm_private_data;
> +     if (!gref)
> +             return;
> +
> +     spin_lock(&gref_lock);
> +     gref->users++;
> +     spin_unlock(&gref_lock);
> +}
> +
>  static void gntalloc_vma_close(struct vm_area_struct *vma)
>  {
>       struct gntalloc_gref *gref = vma->vm_private_data;
> @@ -441,6 +452,7 @@ static void gntalloc_vma_close(struct vm_area_struct *vma)
>  }
>  
>  static struct vm_operations_struct gntalloc_vmops = {
> +     .open = gntalloc_vma_open,
>       .close = gntalloc_vma_close,
>  };
>  
> @@ -471,8 +483,6 @@ static int gntalloc_mmap(struct file *filp, struct 
> vm_area_struct *vma)
>       vma->vm_private_data = gref;
>  
>       vma->vm_flags |= VM_RESERVED;
> -     vma->vm_flags |= VM_DONTCOPY;
> -     vma->vm_flags |= VM_PFNMAP | VM_PFN_AT_MMAP;
>  
>       vma->vm_ops = &gntalloc_vmops;
>  
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 2faf797..69c787a 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -334,17 +334,26 @@ static int unmap_grant_pages(struct grant_map *map, int 
> offset, int pages)
>  
>  /* ------------------------------------------------------------------ */
>  
> +static void gntdev_vma_open(struct vm_area_struct *vma)
> +{
> +     struct grant_map *map = vma->vm_private_data;
> +
> +     pr_debug("gntdev_vma_open %p\n", vma);
> +     atomic_inc(&map->users);
> +}
> +
>  static void gntdev_vma_close(struct vm_area_struct *vma)
>  {
>       struct grant_map *map = vma->vm_private_data;
>  
> -     pr_debug("close %p\n", vma);
> +     pr_debug("gntdev_vma_close %p\n", vma);
>       map->vma = NULL;
>       vma->vm_private_data = NULL;
>       gntdev_put_map(map);
>  }
>  
>  static struct vm_operations_struct gntdev_vmops = {
> +     .open = gntdev_vma_open,
>       .close = gntdev_vma_close,
>  };
>  
> @@ -656,7 +665,10 @@ static int gntdev_mmap(struct file *flip, struct 
> vm_area_struct *vma)
>  
>       vma->vm_ops = &gntdev_vmops;
>  
> -     vma->vm_flags |= VM_RESERVED|VM_DONTCOPY|VM_DONTEXPAND|VM_PFNMAP;
> +     vma->vm_flags |= VM_RESERVED|VM_DONTEXPAND;
> +
> +     if (use_ptemod)
> +             vma->vm_flags |= VM_DONTCOPY|VM_PFNMAP;
>  
>       vma->vm_private_data = map;
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel