On 12/14/2010 06:55 AM, Daniel De Graaf wrote:
> This should be faster if many mappings exist,
Yes, seems reasonable.
> and also removes
> the only user of vma that is not related to PTE modification.
What do you mean? Oh, you mean map->vma?
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
> drivers/xen/gntdev.c | 34 ++++++++++------------------------
> 1 files changed, 10 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 773b76c..a73f07c 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -74,6 +74,8 @@ struct grant_map {
> struct granted_page pages[0];
> };
>
> +static struct vm_operations_struct gntdev_vmops;
Is it OK for this to be all NULL?
> +
> /* ------------------------------------------------------------------ */
>
> static void gntdev_print_maps(struct gntdev_priv *priv,
> @@ -142,23 +144,6 @@ static struct grant_map *gntdev_find_map_index(struct
> gntdev_priv *priv, int ind
> return NULL;
> }
>
> -static struct grant_map *gntdev_find_map_vaddr(struct gntdev_priv *priv,
> - unsigned long vaddr)
> -{
> - struct grant_map *map;
> -
> - list_for_each_entry(map, &priv->maps, next) {
> - if (!map->vma)
> - continue;
> - if (vaddr < map->vma->vm_start)
> - continue;
> - if (vaddr >= map->vma->vm_end)
> - continue;
> - return map;
> - }
> - return NULL;
> -}
> -
> static int gntdev_del_map(struct grant_map *map)
> {
> int i;
> @@ -510,6 +495,7 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct
> gntdev_priv *priv,
> struct
> ioctl_gntdev_get_offset_for_vaddr __user *u)
> {
> struct ioctl_gntdev_get_offset_for_vaddr op;
> + struct vm_area_struct *vma;
> struct grant_map *map;
>
> if (copy_from_user(&op, u, sizeof(op)) != 0)
> @@ -518,16 +504,16 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct
> gntdev_priv *priv,
> printk("%s: priv %p, offset for vaddr %lx\n", __FUNCTION__,
> priv,
> (unsigned long)op.vaddr);
>
> - spin_lock(&priv->lock);
> - map = gntdev_find_map_vaddr(priv, op.vaddr);
> - if (map == NULL ||
> - map->vma->vm_start != op.vaddr) {
> - spin_unlock(&priv->lock);
> + vma = find_vma(current->mm, op.vaddr);
> + if (!vma)
> return -EINVAL;
> - }
> +
> + map = vma->vm_private_data;
> + if (vma->vm_ops != &gntdev_vmops || !map)
> + return -EINVAL;
I know this is perfectly OK, but I'd be happier if you don't refer to
vm_private_data as "map" until the vma has been confirmed to be a gntdev
one.
> +
> op.offset = map->index << PAGE_SHIFT;
> op.count = map->count;
> - spin_unlock(&priv->lock);
>
> if (copy_to_user(u, &op, sizeof(op)) != 0)
> return -EFAULT;
J
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home