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: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xen/gntdev, gntalloc: Remove unneeded VM flags
From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Date: Mon, 07 Mar 2011 15:01:46 -0500
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "konrad.wilk@xxxxxxxxxx" <konrad.wilk@xxxxxxxxxx>
Delivery-date: Mon, 07 Mar 2011 12:02:47 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1299527629.3731.15.camel@xxxxxxxxxxxxxxxxxxxxx>
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: National Security Agency
References: <1299521976-8952-1-git-send-email-dgdegra@xxxxxxxxxxxxx> <1299527629.3731.15.camel@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc13 Thunderbird/3.1.7
On 03/07/2011 02:53 PM, Ian Campbell wrote:
> 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.

PV guests use PTE modification in gntdev, so this does not make any changes
to them. I can clarify that in the commit message if it is helpful.

The reference counting changes are balanced by the already-existing close
functions. The open function was not needed before now because it is only
used when the VM area is being copied on fork, which was not possible. I
can add a note of this if it would clarify the commit.

>>
>> 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