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 2/6] xen-gntdev: Change page limit to be global i

To: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 2/6] xen-gntdev: Change page limit to be global instead of per-open
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Wed, 15 Dec 2010 09:50:32 +0000
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 15 Dec 2010 01:51:10 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D07E4B8.7010406@xxxxxxxxxxxxx>
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: Citrix Systems, Inc.
References: <1292338553-20575-1-git-send-email-dgdegra@xxxxxxxxxxxxx> <1292338553-20575-3-git-send-email-dgdegra@xxxxxxxxxxxxx> <4D07DDB3.1070304@xxxxxxxx> <4D07E4B8.7010406@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Tue, 2010-12-14 at 21:42 +0000, Daniel De Graaf wrote:
> On 12/14/2010 04:12 PM, Jeremy Fitzhardinge wrote:
> > On 12/14/2010 06:55 AM, Daniel De Graaf wrote:
> >> Because there is no limitation on how many times a user can open a
> >> given device file, an per-file-description limit on the number of
> >> pages granted offers little to no benefit. Change to a global limit
> >> and remove the ioctl() as the parameter can now be changed via sysfs.
> > 
> > Does anyone use this ioctl?  Wouldn't it be safer to replace it with a
> > no-op version?
> > 
> >     J
> > 
> 
> I do not know of any users. If it's preferred to replace with a noop ioctl
> instead of the current -ENOSYS return, that's easy to do.

It's called by xc_gnttab_set_max_grants in libxc, although I don't see
any callers of that function.

We should probably also remove the library function.

Ian.

> >> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> >> ---
> >>  drivers/xen/gntdev.c |   48 
> >> ++++++++++++++----------------------------------
> >>  1 files changed, 14 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> >> index c582804..62639af 100644
> >> --- a/drivers/xen/gntdev.c
> >> +++ b/drivers/xen/gntdev.c
> >> @@ -44,13 +44,12 @@ MODULE_DESCRIPTION("User-space granted page access 
> >> driver");
> >>  
> >>  static int debug = 0;
> >>  module_param(debug, int, 0644);
> >> -static int limit = 1024;
> >> +static int limit = 1024*1024;
> >>  module_param(limit, int, 0644);
> >> +static atomic_t pages_mapped = ATOMIC_INIT(0);
> >>  
> >>  struct gntdev_priv {
> >>    struct list_head maps;
> >> -  uint32_t used;
> >> -  uint32_t limit;
> >>    spinlock_t lock;
> >>    struct mm_struct *mm;
> >>    struct mmu_notifier mn;
> >> @@ -76,8 +75,8 @@ static void gntdev_print_maps(struct gntdev_priv *priv,
> >>  {
> >>    struct grant_map *map;
> >>  
> >> -  printk("%s: maps list (priv %p, usage %d/%d)\n",
> >> -         __FUNCTION__, priv, priv->used, priv->limit);
> >> +  printk("%s: maps list (priv %p)\n",
> >> +         __FUNCTION__, priv);
> >>    list_for_each_entry(map, &priv->maps, next)
> >>            printk("  index %2d, count %2d %s\n",
> >>                   map->index, map->count,
> >> @@ -104,9 +103,6 @@ static struct grant_map *gntdev_alloc_map(struct 
> >> gntdev_priv *priv, int count)
> >>    add->count = count;
> >>    add->priv  = priv;
> >>  
> >> -  if (add->count + priv->used > priv->limit)
> >> -          goto err;
> >> -
> >>    return add;
> >>  
> >>  err:
> >> @@ -131,7 +127,6 @@ static void gntdev_add_map(struct gntdev_priv *priv, 
> >> struct grant_map *add)
> >>    list_add_tail(&add->next, &priv->maps);
> >>  
> >>  done:
> >> -  priv->used += add->count;
> >>    if (debug)
> >>            gntdev_print_maps(priv, "[new]", add->index);
> >>  }
> >> @@ -178,7 +173,7 @@ static int gntdev_del_map(struct grant_map *map)
> >>            if (map->unmap_ops[i].handle)
> >>                    return -EBUSY;
> >>  
> >> -  map->priv->used -= map->count;
> >> +  atomic_sub(map->count, &pages_mapped);
> >>    list_del(&map->next);
> >>    return 0;
> >>  }
> >> @@ -360,7 +355,6 @@ static int gntdev_open(struct inode *inode, struct 
> >> file *flip)
> >>  
> >>    INIT_LIST_HEAD(&priv->maps);
> >>    spin_lock_init(&priv->lock);
> >> -  priv->limit = limit;
> >>  
> >>    priv->mm = get_task_mm(current);
> >>    if (!priv->mm) {
> >> @@ -416,19 +410,26 @@ static long gntdev_ioctl_map_grant_ref(struct 
> >> gntdev_priv *priv,
> >>                   op.count);
> >>    if (unlikely(op.count <= 0))
> >>            return -EINVAL;
> >> -  if (unlikely(op.count > priv->limit))
> >> -          return -EINVAL;
> >>  
> >>    err = -ENOMEM;
> >>    map = gntdev_alloc_map(priv, op.count);
> >>    if (!map)
> >>            return err;
> >> +
> >>    if (copy_from_user(map->grants, &u->refs,
> >>                       sizeof(map->grants[0]) * op.count) != 0) {
> >>            gntdev_free_map(map);
> >>            return err;
> >>    }
> >>  
> >> +  if (unlikely(atomic_add_return(op.count, &pages_mapped) > limit))
> >> +  {
> >> +          if (debug)
> >> +                  printk("%s: can't map: over limit\n", __FUNCTION__);
> >> +          gntdev_free_map(map);
> >> +          return err;
> >> +  }
> >> +
> >>    spin_lock(&priv->lock);
> >>    gntdev_add_map(priv, map);
> >>    op.index = map->index << PAGE_SHIFT;
> >> @@ -495,24 +496,6 @@ static long gntdev_ioctl_get_offset_for_vaddr(struct 
> >> gntdev_priv *priv,
> >>    return 0;
> >>  }
> >>  
> >> -static long gntdev_ioctl_set_max_grants(struct gntdev_priv *priv,
> >> -                                  struct ioctl_gntdev_set_max_grants 
> >> __user *u)
> >> -{
> >> -  struct ioctl_gntdev_set_max_grants op;
> >> -
> >> -  if (copy_from_user(&op, u, sizeof(op)) != 0)
> >> -          return -EFAULT;
> >> -  if (debug)
> >> -          printk("%s: priv %p, limit %d\n", __FUNCTION__, priv, op.count);
> >> -  if (op.count > limit)
> >> -          return -EINVAL;
> >> -
> >> -  spin_lock(&priv->lock);
> >> -  priv->limit = op.count;
> >> -  spin_unlock(&priv->lock);
> >> -  return 0;
> >> -}
> >> -
> >>  static long gntdev_ioctl(struct file *flip,
> >>                     unsigned int cmd, unsigned long arg)
> >>  {
> >> @@ -529,9 +512,6 @@ static long gntdev_ioctl(struct file *flip,
> >>    case IOCTL_GNTDEV_GET_OFFSET_FOR_VADDR:
> >>            return gntdev_ioctl_get_offset_for_vaddr(priv, ptr);
> >>  
> >> -  case IOCTL_GNTDEV_SET_MAX_GRANTS:
> >> -          return gntdev_ioctl_set_max_grants(priv, ptr);
> >> -
> >>    default:
> >>            if (debug)
> >>                    printk("%s: priv %p, unknown cmd %x\n",
> > 



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

<Prev in Thread] Current Thread [Next in Thread>