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 1/6] xen-gntdev: Fix circular locking dependency

To: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Fix circular locking dependency
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Wed, 15 Dec 2010 09:47:22 +0000
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 15 Dec 2010 01:49:06 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D07E444.3020309@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-2-git-send-email-dgdegra@xxxxxxxxxxxxx> <4D07DD76.2010900@xxxxxxxx> <4D07E444.3020309@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Tue, 2010-12-14 at 21:40 +0000, Daniel De Graaf wrote:
> On 12/14/2010 04:11 PM, Jeremy Fitzhardinge wrote:
> > On 12/14/2010 06:55 AM, Daniel De Graaf wrote:
> >> apply_to_page_range will acquire PTE lock while priv->lock is held, and
> >> mn_invl_range_start tries to acquire priv->lock with PTE already held.
> >> Fix by not holding priv->lock during the entire map operation.
> > 
> > Is priv->lock needed to protect the contents of map?
> > 
> >     J
> 
> No, since the map can only be mapped once (checked by map->vma assignment
> while the lock is held). The unmap ioctl will return -EBUSY until
> an munmap() is called on the area, so it also can't race, and the other
> users are only active once the mmap operation completes.

Sounds reasonable enough to me. There are a few unlocked accesses to
vma->map: 
        gntdev_del_map (when called from gntdev_ioctl_map_grant_ref)
        gntdev_vma_close
are these safe? If so then it would be worth a comment about why.

Anyway this patch appears to resolve the lockdep warning I was seeing
with 2.6.37 with qemu-xen backed block devices.

Ian.

> 
> > 
> >> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> >> ---
> >>  drivers/xen/gntdev.c |   19 +++++++++----------
> >>  1 files changed, 9 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> >> index a33e443..c582804 100644
> >> --- a/drivers/xen/gntdev.c
> >> +++ b/drivers/xen/gntdev.c
> >> @@ -581,23 +581,22 @@ static int gntdev_mmap(struct file *flip, struct 
> >> vm_area_struct *vma)
> >>    if (!(vma->vm_flags & VM_WRITE))
> >>            map->flags |= GNTMAP_readonly;
> >>  
> >> +  spin_unlock(&priv->lock);
> >> +
> >>    err = apply_to_page_range(vma->vm_mm, vma->vm_start,
> >>                              vma->vm_end - vma->vm_start,
> >>                              find_grant_ptes, map);
> >> -  if (err) {
> >> -          goto unlock_out;
> >> -          if (debug)
> >> -                  printk("%s: find_grant_ptes() failure.\n", 
> >> __FUNCTION__);
> >> -  }
> >> +  if (err)
> >> +          return err;
> >>  
> >>    err = map_grant_pages(map);
> >> -  if (err) {
> >> -          goto unlock_out;
> >> -          if (debug)
> >> -                  printk("%s: map_grant_pages() failure.\n", 
> >> __FUNCTION__);
> >> -  }
> >> +  if (err)
> >> +          return err;
> >> +
> >>    map->is_mapped = 1;
> >>  
> >> +  return 0;
> >> +
> >>  unlock_out:
> >>    spin_unlock(&priv->lock);
> >>    return err;
> >



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

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