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

[Xen-devel] [PATCH] xen/gntdev: Fix sleep-inside-spinlock

To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Subject: [Xen-devel] [PATCH] xen/gntdev: Fix sleep-inside-spinlock
From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Date: Tue, 11 Oct 2011 15:16:06 -0400
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Dario Faggioli <raistlin@xxxxxxxx>
Delivery-date: Tue, 11 Oct 2011 12:17:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20111011173228.GB32406@xxxxxxxxxxxxxxxxx>
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: <1318353219.21925.80.camel@Palantir> <20111011173228.GB32406@xxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0.2) Gecko/20110906 Thunderbird/6.0.2
On 10/11/2011 01:32 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 11, 2011 at 07:13:38PM +0200, Dario Faggioli wrote:
>> Hello everyone,
>>
>> Since I really plan to spend some time here, let me introduce myself
>> first. My name is Dario Faggioli and I just joined the Citrix Platform
>> Team in Cambridge (although I'll be working from Italy). I've some
>> experience in Linux kernel (mainly scheduling) and not that much
>> experience in Xen or virtualization in general, but I really want to
>> learn and be able to contribute ASAP!
>>
>> In fact, while "doing my homework", I stumbled against the following
>> BUG(). I'm able to reproduce it with xen-unstable and by just by
>> enabling spinlock and mutex debug checks in linus' Linux
>> (65112dccf8a113737684366349d7f9ec373ddc47) _iff_ using 'tap:qcow2' disk
>> image, while no such thing happens with 'phy:'.
> 
> Aha.. I saw that at some point but never narrowed it down to the
> right combination. Thank you for tracking this down.
> 
> I am CC Daniel who might have some patches for this or ideas.
>>
>> [  996.282544] BUG: sleeping function called from invalid context at 
>> /local/scratch/dariof/linux/kernel/mutex.c:271
>> [  996.282570] in_atomic(): 1, irqs_disabled(): 0, pid: 3256, name: qemu-dm
>> [  996.282581] 1 lock held by qemu-dm/3256:
>> [  996.282589]  #0:  (&(&priv->lock)->rlock){......}, at: 
>> [<ffffffff813223da>] gntdev_ioctl+0x2bd/0x4d5
>> [  996.282628] Pid: 3256, comm: qemu-dm Tainted: G        W   3.1.0-rc8+ #5
>> [  996.282638] Call Trace:
>> [  996.282687]  [<ffffffff81054594>] __might_sleep+0x131/0x135
>> [  996.282704]  [<ffffffff816bd64f>] mutex_lock_nested+0x25/0x45
>> [  996.282721]  [<ffffffff8131c7c8>] free_xenballooned_pages+0x20/0xb1
>> [  996.282735]  [<ffffffff8132194d>] gntdev_put_map+0xa8/0xdb
>> [  996.282749]  [<ffffffff816be546>] ? _raw_spin_lock+0x71/0x7a
>> [  996.282763]  [<ffffffff813223da>] ? gntdev_ioctl+0x2bd/0x4d5
>> [  996.282776]  [<ffffffff8132243c>] gntdev_ioctl+0x31f/0x4d5
>> [  996.282790]  [<ffffffff81007d62>] ? check_events+0x12/0x20
>> [  996.282804]  [<ffffffff811433bc>] do_vfs_ioctl+0x488/0x4d7
>> [  996.282818]  [<ffffffff81007d4f>] ? xen_restore_fl_direct_reloc+0x4/0x4
>> [  996.282832]  [<ffffffff8109168b>] ? lock_release+0x21c/0x229
>> [  996.282847]  [<ffffffff81135cdd>] ? rcu_read_unlock+0x21/0x32
>> [  996.282860]  [<ffffffff81143452>] sys_ioctl+0x47/0x6a
>> [  996.282873]  [<ffffffff816bfd82>] system_call_fastpath+0x16/0x1b
>>
>> This seems to be due to free_xenballooned_pages(), called by
>> gntdev_put_map(), taking balloon_mutex, with the latter that can be
>> called within a spin_lock() (e.g., in gntdev_release()).
>>
>> I'm not enough confident with the code do attempt fixing it, but I
>> thought it was worth to at least point it out!
> 

It looks like gntdev_release doesn't need to take the spinlock at all, since
it's a per-file lock on the file's release path. The unmap ioctl looks like
it'll also trigger this message; that's also fixed here. Compile tested only.

-------------------------------------------------------->8

gntdev_put_map tries to acquire a mutex when freeing pages back to the
xenballoon pool, so it cannot be called with a spinlock held. In
gntdev_release, the spinlock is not needed as we are freeing the
structure later; in the ioctl, only the list manipulation needs to be
under the lock.

Reported-By: Dario Faggioli <raistlin@xxxxxxxx>
Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
---
 drivers/xen/gntdev.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index f914b26..23b1c83 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -462,13 +462,11 @@ static int gntdev_release(struct inode *inode, struct 
file *flip)
 
        pr_debug("priv %p\n", priv);
 
-       spin_lock(&priv->lock);
        while (!list_empty(&priv->maps)) {
                map = list_entry(priv->maps.next, struct grant_map, next);
                list_del(&map->next);
                gntdev_put_map(map);
        }
-       spin_unlock(&priv->lock);
 
        if (use_ptemod)
                mmu_notifier_unregister(&priv->mn, priv->mm);
@@ -532,10 +530,11 @@ static long gntdev_ioctl_unmap_grant_ref(struct 
gntdev_priv *priv,
        map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT, op.count);
        if (map) {
                list_del(&map->next);
-               gntdev_put_map(map);
                err = 0;
        }
        spin_unlock(&priv->lock);
+       if (map)
+               gntdev_put_map(map);
        return err;
 }
 
-- 
1.7.6.4


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