From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH] xen/gntdev,gntalloc: Remove unneeded VM flags Date: Mon, 7 Mar 2011 19:53:49 +0000 Message-ID: <1299527629.3731.15.camel@localhost.localdomain> References: <1299521976-8952-1-git-send-email-dgdegra@tycho.nsa.gov> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1299521976-8952-1-git-send-email-dgdegra@tycho.nsa.gov> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Daniel De Graaf Cc: "xen-devel@lists.xensource.com" , "konrad.wilk@oracle.com" List-Id: xen-devel@lists.xenproject.org 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 > --- > 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; >