Re: [PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e

From: Chris Rankin <rankincj@yahoo.com>
To: Devin Heitmueller <dheitmueller@kernellabs.com>
Cc: linux-media@vger.kernel.org, mchehab@redhat.com,
	Antti Palosaari <crope@iki.fi>
Subject: Re: [PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
Date: Fri, 19 Aug 2011 01:12:20 +0100	[thread overview]
Message-ID: <4E4DAA64.4050302@yahoo.com> (raw)
In-Reply-To: <CAGoCfiw4v-ZsUPmVgOhARwNqjCVK458EV79djD625Sf+8Oghag@mail.gmail.com>

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

The final patch removes the unplug/replug deadlock by not holding the device 
mutex during dvb_init(). However, this mutex has already been locked during 
device initialisation by em28xx_usb_probe() and is not released again until all 
extensions have been initialised successfully.

The device mutex is not held during either em28xx_register_extension() or 
em28xx_unregister_extension() any more. More importantly, I don't believe it can 
safely be held by these functions because they must both - by their nature -  
acquire the device list mutex before they can iterate through the device list. 
In other words, while usb_probe() and usb_disconnect() acquire the device mutex 
followed by the device list mutex, the register/unregister_extension() functions 
would need to acquire these mutexes in the opposite order. And that sounds like 
a potential deadlock.

On the other hand, the new situation is a definite improvement :-).

Signed-off-by: Chris Rankin <rankincj@yahoo.com>


[-- Attachment #2: EM28xx-replug-deadlock.diff --]
[-- Type: text/x-patch, Size: 551 bytes --]

--- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig	2011-08-19 00:50:41.000000000 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c	2011-08-19 00:51:03.000000000 +0100
@@ -542,7 +542,6 @@
 	dev->dvb = dvb;
 	dvb->fe[0] = dvb->fe[1] = NULL;
 
-	mutex_lock(&dev->lock);
 	em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
 	/* init frontend */
 	switch (dev->model) {
@@ -711,7 +710,6 @@
 	em28xx_info("Successfully loaded em28xx-dvb\n");
 ret:
 	em28xx_set_mode(dev, EM28XX_SUSPEND);
-	mutex_unlock(&dev->lock);
 	return result;
 
 out_free:
