|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [PATCH 5/7] xen-gntdev: Add reference counting to maps
On 12/16/2010 04:17 PM, Daniel De Graaf wrote:
> This allows userspace to perform mmap() on the gntdev device and then
> immediately close the filehandle or remove the mapping using the
> remove ioctl, with the mapped area remaining valid until unmapped.
> This also fixes an infinite loop when a gntdev device is closed
> without first unmapping all areas.
>
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
> drivers/xen/gntdev.c | 67 +++++++++++++++++++++++--------------------------
> 1 files changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 6a3c9e4..f1fc8fa 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -66,16 +66,18 @@ struct granted_page {
>
> struct grant_map {
> struct list_head next;
> - struct gntdev_priv *priv;
> struct vm_area_struct *vma;
> int index;
> int count;
> + atomic_t users;
Does this need to be atomic? Won't it be happening under spinlock anyway?
> int is_mapped:1;
> int is_ro:1;
> struct page **pages;
> struct granted_page pginfo[0];
> };
>
> +static void unmap_grant_pages(struct grant_map *map, int offset, int pages);
> +
> /* ------------------------------------------------------------------ */
>
> static void gntdev_print_maps(struct gntdev_priv *priv,
> @@ -112,6 +114,7 @@ static struct grant_map *gntdev_alloc_map(int count,
> }
>
> add->index = 0;
> + atomic_set(&add->users, 1);
> add->count = count;
> for(i = 0; i < count; i++)
> add->pginfo[i].target = grants[i];
> @@ -144,7 +147,6 @@ static void gntdev_add_map(struct gntdev_priv *priv,
> struct grant_map *add)
> list_add_tail(&add->next, &priv->maps);
>
> done:
> - add->priv = priv;
> if (debug)
> gntdev_print_maps(priv, "[new]", add->index);
> spin_unlock(&priv->lock);
> @@ -165,33 +167,26 @@ static struct grant_map *gntdev_find_map_index(struct
> gntdev_priv *priv, int ind
> return NULL;
> }
>
> -static int gntdev_del_map(struct grant_map *map)
> +static void gntdev_put_map(struct grant_map *map)
> {
> int i;
>
> - if (map->vma)
> - return -EBUSY;
> - if (map->is_mapped)
> - for (i = 0; i < map->count; i++)
> - if (map->pginfo[i].handle)
> - return -EBUSY;
> + if (!map)
> + return;
>
> - atomic_sub(map->count, &pages_mapped);
> - list_del(&map->next);
> - return 0;
> -}
> + if (!atomic_dec_and_test(&map->users))
> + return;
>
> -static void gntdev_free_map(struct grant_map *map)
> -{
> - unsigned i;
> + atomic_sub(map->count, &pages_mapped);
>
> - if (!map)
> - return;
> + if (!use_ptemod)
> + unmap_grant_pages(map, 0, map->count);
>
> for (i = 0; i < map->count; i++) {
> if (map->pages[i])
> __free_page(map->pages[i]);
> }
> + kfree(map->pages);
> kfree(map);
> }
>
> @@ -310,6 +305,7 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
> map->is_mapped = 0;
> map->vma = NULL;
> vma->vm_private_data = NULL;
> + gntdev_put_map(map);
> }
>
> static int gntdev_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
> @@ -425,7 +421,6 @@ static int gntdev_release(struct inode *inode, struct
> file *flip)
> {
> struct gntdev_priv *priv = flip->private_data;
> struct grant_map *map;
> - int err;
>
> if (debug)
> printk("%s: priv %p\n", __FUNCTION__, priv);
> @@ -433,10 +428,8 @@ static int gntdev_release(struct inode *inode, struct
> file *flip)
> spin_lock(&priv->lock);
> while (!list_empty(&priv->maps)) {
> map = list_entry(priv->maps.next, struct grant_map, next);
> - err = gntdev_del_map(map);
> - if (WARN_ON(err))
> - gntdev_free_map(map);
> -
> + list_del(&map->next);
> + gntdev_put_map(map);
> }
> spin_unlock(&priv->lock);
>
> @@ -487,18 +480,14 @@ static long gntdev_ioctl_map_grant_ref(struct
> gntdev_priv *priv,
>
> if (copy_to_user(u, &op, sizeof(op))) {
> err = -EFAULT;
> - goto out_remove;
> + goto out_free;
> }
> err = 0;
> out_free:
> kfree(grants);
> return err;
> -out_remove:
> - spin_lock(&priv->lock);
> - gntdev_del_map(map);
> - spin_unlock(&priv->lock);
> out_free_map:
> - gntdev_free_map(map);
> + gntdev_put_map(map);
> goto out_free;
> }
>
> @@ -507,7 +496,7 @@ static long gntdev_ioctl_unmap_grant_ref(struct
> gntdev_priv *priv,
> {
> struct ioctl_gntdev_unmap_grant_ref op;
> struct grant_map *map;
> - int err = -EINVAL;
> + int err;
>
> if (copy_from_user(&op, u, sizeof(op)) != 0)
> return -EFAULT;
> @@ -517,11 +506,13 @@ static long gntdev_ioctl_unmap_grant_ref(struct
> gntdev_priv *priv,
>
> spin_lock(&priv->lock);
> map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT, op.count);
> - if (map)
> - err = gntdev_del_map(map);
> + if (map) {
> + list_del(&map->next);
> + gntdev_put_map(map);
> + err = 0;
> + } else
> + err = -EINVAL;
What prevents unmap_grant_ref being called multiple times?
> spin_unlock(&priv->lock);
> - if (!err)
> - gntdev_free_map(map);
> return err;
> }
>
> @@ -599,13 +590,15 @@ static int gntdev_mmap(struct file *flip, struct
> vm_area_struct *vma)
> map = gntdev_find_map_index(priv, index, count);
> if (!map)
> goto unlock_out;
> - if (map->vma)
> + if (use_ptemod && map->vma)
> goto unlock_out;
> if (priv->mm != vma->vm_mm) {
> printk("%s: Huh? Other mm?\n", __FUNCTION__);
> goto unlock_out;
> }
>
> + atomic_inc(&map->users);
> +
> vma->vm_ops = &gntdev_vmops;
>
> vma->vm_flags |= VM_RESERVED;
> @@ -614,7 +607,9 @@ static int gntdev_mmap(struct file *flip, struct
> vm_area_struct *vma)
> vma->vm_flags |= VM_PFNMAP;
>
> vma->vm_private_data = map;
> - map->vma = vma;
> +
> + if (use_ptemod)
> + map->vma = vma;
>
> map->is_ro = !(vma->vm_flags & VM_WRITE);
>
J
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |