Re: [linux-3.14 bisection] complete test-amd64-i386-xl-qcow2

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 2470 bytes --]

On Wed, 2015-09-02 at 10:18 +0100, Ian Campbell wrote:
> [resending to correct stable address, sorry folks]
> 
> TL;DR: Any backport of 30b03d05e074 to earlier than commit 1401c00e59e
> ("xen/gntdev: convert priv->lock to a mutex", which was added in v4.0)
> needs $something doing to it, either s/mutex/spinlock/ or (more likely)
> backporting of 1401c00e59e too.
> 
> Looking at LTS:
> 
> 3.18.y:> 	>   Backported both.
> 3.16.y:> 	>   Has backported neither
> 3.14.y:> 	> * Only backported 30b03d05e074
> 3.12.y:> 	>   Has backported neither
> 3.10.y:> 	> * Only backported 30b03d05e074
> 3.4.y:> 	>   Has backported neither
> 3.2.y:> 	>   Has backported neither
> 
> So AFAICT 3.14.y and 3.10.y need fixes, probably following 3.18 and
> backporting 1401c00e59e.
> 
> 3.16/12/4/2 might need to be careful if they subsequently pick up 30b03d05.
[...]

I came up with the patch below for 3.2.  Let me know if it's not
correct.

Ben.

---
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Date: Fri, 26 Jun 2015 03:28:24 +0200
Subject: xen/gntdevt: Fix race condition in gntdev_release()

commit 30b03d05e07467b8c6ec683ea96b5bffcbcd3931 upstream.

While gntdev_release() is called the MMU notifier is still registered
and can traverse priv->maps list even if no pages are mapped (which is
the case -- gntdev_release() is called after all). But
gntdev_release() will clear that list, so make sure that only one of
those things happens at the same time.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
[bwh: Backported to 3.2:
 - Adjust context
 - gntdev_priv::lock is a spinlock rather than a mutex.  As gntdev_put_map()
   may sleep, we need to unlock inside the loop.]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -493,11 +493,15 @@ static int gntdev_release(struct inode *
 
 	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);
+		spin_unlock(&priv->lock);
 		gntdev_put_map(map);
+		spin_lock(&priv->lock);
 	}
+	spin_unlock(&priv->lock);
 
 	if (use_ptemod)
 		mmu_notifier_unregister(&priv->mn, priv->mm);

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]