[1/1] EM28xx - fix deadlock when unplugging and replugging a DVB adapter

Message ID 4E50FAC7.6080807@yahoo.com (mailing list archive)
State RFC, archived
Headers

Commit Message

Chris Rankin Aug. 21, 2011, 12:32 p.m. UTC
  It occurred to me this morning that since we're no longer supposed to be holding 
the device lock when taking the device list lock, then the 
em28xx_usb_disconnect() function needs changing too.

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

Comments

Antti Palosaari Sept. 13, 2011, 8:04 p.m. UTC | #1
On 08/21/2011 03:32 PM, Chris Rankin wrote:
> It occurred to me this morning that since we're no longer supposed to be
> holding the device lock when taking the device list lock, then the
> em28xx_usb_disconnect() function needs changing too.
>
> Signed-off-by: Chris Rankin <rankincj@yahoo.com>

I ran that also when re-plugging both PCTV 290e and 460e as today 
LinuxTV 3.2 tree. Seems like this patch is still missing and maybe some 
more.

git log drivers/media/video/em28xx/ |grep -A3 "Author: Chris Rankin"
Author: Chris Rankin <rankincj@yahoo.com>
Date:   Tue Sep 13 12:23:39 2011 +0300

     em28xx: ERROR: "em28xx_add_into_devlist" 
[drivers/media/video/em28xx/em28xx.ko] undefined!
--
Author: Chris Rankin <rankincj@yahoo.com>
Date:   Sat Aug 20 16:01:26 2011 -0300

     [media] em28xx: don't sleep on disconnect
--
Author: Chris Rankin <rankincj@yahoo.com>
Date:   Sat Aug 20 08:31:05 2011 -0300

     [media] em28xx: move printk lines outside mutex lock
--
Author: Chris Rankin <rankincj@yahoo.com>
Date:   Sat Aug 20 08:28:17 2011 -0300

     [media] em28xx: clean up resources should init fail
--
Author: Chris Rankin <rankincj@yahoo.com>
Date:   Sat Aug 20 08:21:03 2011 -0300

     [media] em28xx: use atomic bit operations for devices-in-use mask
--
Author: Chris Rankin <rankincj@yahoo.com>
Date:   Sat Aug 20 08:08:34 2011 -0300

     [media] em28xx: pass correct buffer size to snprintf


regards
Antti
  
Chris Rankin Sept. 13, 2011, 8:47 p.m. UTC | #2
On 13/09/11 21:04, Antti Palosaari wrote:
> On 08/21/2011 03:32 PM, Chris Rankin wrote:
>> It occurred to me this morning that since we're no longer supposed to be
>> holding the device lock when taking the device list lock, then the
>> em28xx_usb_disconnect() function needs changing too.
>>
>> Signed-off-by: Chris Rankin <rankincj@yahoo.com>
>
> I ran that also when re-plugging both PCTV 290e and 460e as today LinuxTV 3.2
> tree. Seems like this patch is still missing and maybe some more.

There was also this patch, which fixed a couple of memory leaks:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg36432.html

IIRC, the main purpose of the other patch was to delete the 
em28xx_remove_from_devlist() function as well by adding the

     list_del(&dev->devlist);

to the em28xx_close_extension() function instead.

http://www.mail-archive.com/linux-media@vger.kernel.org/msg35783.html

Cheers,
Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig	2011-08-19 00:45:48.000000000 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c	2011-08-21 13:16:43.000000000 +0100
@@ -2929,7 +2929,9 @@ 
 		goto fail_reg_analog_devices;
 	}
 
+	mutex_unlock(&dev->lock);
 	em28xx_init_extension(dev);
+	mutex_lock(&dev->lock);
 
 	/* Save some power by putting tuner to sleep */
 	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
@@ -3191,10 +3193,10 @@ 
 		em28xx_release_resources(dev);
 	}
 
-	em28xx_close_extension(dev);
-
 	mutex_unlock(&dev->lock);
 
+	em28xx_close_extension(dev);
+
 	if (!dev->users) {
 		kfree(dev->alt_max_pkt_size);
 		kfree(dev);