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: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 1/6] xen-gntdev: Fix circular locking dependency
From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Date: Tue, 14 Dec 2010 16:40:20 -0500
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Delivery-date: Tue, 14 Dec 2010 13:41:47 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D07DD76.2010900@xxxxxxxx>
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: National Security Agency
References: <1292338553-20575-1-git-send-email-dgdegra@xxxxxxxxxxxxx> <1292338553-20575-2-git-send-email-dgdegra@xxxxxxxxxxxxx> <4D07DD76.2010900@xxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.12) Gecko/20101027 Fedora/3.1.6-1.fc13 Thunderbird/3.1.6
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.

> 
>> 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>