[Xen-devel] [PATCH 5/7] xen-gntdev: Add reference counting to maps

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

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;
        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;
        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);
 
-- 
1.7.2.3


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