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