From patchwork Mon Jan 13 06:12:04 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 3474101 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E676D9F2E9 for ; Mon, 13 Jan 2014 09:52:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 773A6200F2 for ; Mon, 13 Jan 2014 09:52:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D0B9020035 for ; Mon, 13 Jan 2014 09:52:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751406AbaAMJwd (ORCPT ); Mon, 13 Jan 2014 04:52:33 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:50813 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750991AbaAMJwb (ORCPT ); Mon, 13 Jan 2014 04:52:31 -0500 Received: from [177.143.150.208] (helo=smtp.w2.samsung.com) by bombadil.infradead.org with esmtpsa (Exim 4.80.1 #2 (Red Hat Linux)) id 1W2eC5-0005fu-2I; Mon, 13 Jan 2014 09:52:29 +0000 Received: from mchehab by smtp.w2.samsung.com with local (Exim 4.80.1) (envelope-from ) id 1W2akn-0000Re-47; Mon, 13 Jan 2014 04:12:05 -0200 From: Mauro Carvalho Chehab Cc: Mauro Carvalho Chehab , Linux Media Mailing List , Mauro Carvalho Chehab Subject: [PATCH] em28xx: push mutex down to extensions on .fini callback Date: Mon, 13 Jan 2014 04:12:04 -0200 Message-Id: <1389593524-1676-1-git-send-email-m.chehab@samsung.com> X-Mailer: git-send-email 1.8.3.1 To: unlisted-recipients:; (no To-header on input) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Avoid circular mutex lock by pushing the dev->lock to the .fini callback on each extension. As em28xx-dvb, em28xx-alsa and em28xx-rc have their own data structures, and don't touch at the common structure during .fini, only em28xx-v4l needs to be locked. [ 90.994317] ====================================================== [ 90.994356] [ INFO: possible circular locking dependency detected ] [ 90.994395] 3.13.0-rc1+ #24 Not tainted [ 90.994427] ------------------------------------------------------- [ 90.994458] khubd/54 is trying to acquire lock: [ 90.994490] (&card->controls_rwsem){++++.+}, at: [] snd_ctl_dev_free+0x28/0x60 [snd] [ 90.994656] [ 90.994656] but task is already holding lock: [ 90.994688] (&dev->lock){+.+.+.}, at: [] em28xx_close_extension+0x31/0x90 [em28xx] [ 90.994843] [ 90.994843] which lock already depends on the new lock. [ 90.994843] [ 90.994874] [ 90.994874] the existing dependency chain (in reverse order) is: [ 90.994905] -> #1 (&dev->lock){+.+.+.}: [ 90.995057] [] __lock_acquire+0xb43/0x1330 [ 90.995121] [] lock_acquire+0xa2/0x120 [ 90.995182] [] mutex_lock_nested+0x5c/0x3c0 [ 90.995245] [] em28xx_vol_put_mute+0x1ba/0x1d0 [em28xx_alsa] [ 90.995309] [] snd_ctl_elem_write+0xfd/0x140 [snd] [ 90.995376] [] snd_ctl_ioctl+0xe2/0x810 [snd] [ 90.995442] [] do_vfs_ioctl+0x300/0x520 [ 90.995504] [] SyS_ioctl+0x81/0xa0 [ 90.995568] [] system_call_fastpath+0x16/0x1b [ 90.995630] -> #0 (&card->controls_rwsem){++++.+}: [ 90.995780] [] check_prevs_add+0x947/0x950 [ 90.995841] [] __lock_acquire+0xb43/0x1330 [ 90.995901] [] lock_acquire+0xa2/0x120 [ 90.995962] [] down_write+0x3b/0xa0 [ 90.996022] [] snd_ctl_dev_free+0x28/0x60 [snd] [ 90.996088] [] snd_device_free+0x65/0x140 [snd] [ 90.996154] [] snd_device_free_all+0x61/0xa0 [snd] [ 90.996219] [] snd_card_do_free+0x14/0x130 [snd] [ 90.996283] [] snd_card_free+0x84/0x90 [snd] [ 90.996349] [] em28xx_audio_fini+0x97/0xb0 [em28xx_alsa] [ 90.996411] [] em28xx_close_extension+0x56/0x90 [em28xx] [ 90.996475] [] em28xx_usb_disconnect+0x79/0x90 [em28xx] [ 90.996539] [] usb_unbind_interface+0x67/0x1d0 [ 90.996620] [] __device_release_driver+0x7f/0xf0 [ 90.996682] [] device_release_driver+0x25/0x40 [ 90.996742] [] bus_remove_device+0x11c/0x1a0 [ 90.996801] [] device_del+0x136/0x1d0 [ 90.996863] [] usb_disable_device+0xb0/0x290 [ 90.996923] [] usb_disconnect+0xb5/0x1d0 [ 90.996984] [] hub_port_connect_change+0xd6/0xad0 [ 90.997044] [] hub_events+0x313/0x9b0 [ 90.997105] [] hub_thread+0x35/0x170 [ 90.997165] [] kthread+0xff/0x120 [ 90.997226] [] ret_from_fork+0x7c/0xb0 [ 90.997287] [ 90.997287] other info that might help us debug this: [ 90.997287] [ 90.997318] Possible unsafe locking scenario: [ 90.997318] [ 90.997348] CPU0 CPU1 [ 90.997378] ---- ---- [ 90.997408] lock(&dev->lock); [ 90.997497] lock(&card->controls_rwsem); [ 90.997607] lock(&dev->lock); [ 90.997697] lock(&card->controls_rwsem); [ 90.997786] [ 90.997786] *** DEADLOCK *** [ 90.997786] [ 90.997817] 5 locks held by khubd/54: [ 90.997847] #0: (&__lockdep_no_validate__){......}, at: [] hub_events+0xb4/0x9b0 [ 90.998025] #1: (&__lockdep_no_validate__){......}, at: [] usb_disconnect+0x66/0x1d0 [ 90.998204] #2: (&__lockdep_no_validate__){......}, at: [] device_release_driver+0x1d/0x40 [ 90.998383] #3: (em28xx_devlist_mutex){+.+.+.}, at: [] em28xx_close_extension+0x27/0x90 [em28xx] [ 90.998567] #4: (&dev->lock){+.+.+.}, at: [] em28xx_close_extension+0x31/0x90 [em28xx] Signed-off-by: Mauro Carvalho Chehab Tested-by: Antti Palosaari Reviewed-by: Frank Schäfer --- drivers/media/usb/em28xx/em28xx-core.c | 2 -- drivers/media/usb/em28xx/em28xx-video.c | 4 ++++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c index b6dc3327c51c..898fb9bd88a2 100644 --- a/drivers/media/usb/em28xx/em28xx-core.c +++ b/drivers/media/usb/em28xx/em28xx-core.c @@ -1099,12 +1099,10 @@ void em28xx_close_extension(struct em28xx *dev) const struct em28xx_ops *ops = NULL; mutex_lock(&em28xx_devlist_mutex); - mutex_lock(&dev->lock); list_for_each_entry(ops, &em28xx_extension_devlist, next) { if (ops->fini) ops->fini(dev); } - mutex_unlock(&dev->lock); list_del(&dev->devlist); mutex_unlock(&em28xx_devlist_mutex); } diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c index 004fe12ceec7..258628877951 100644 --- a/drivers/media/usb/em28xx/em28xx-video.c +++ b/drivers/media/usb/em28xx/em28xx-video.c @@ -1896,6 +1896,8 @@ static int em28xx_v4l2_fini(struct em28xx *dev) em28xx_info("Disconnecting video extension"); + mutex_lock(&dev->lock); + v4l2_device_disconnect(&dev->v4l2_dev); em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); @@ -1924,6 +1926,8 @@ static int em28xx_v4l2_fini(struct em28xx *dev) v4l2_ctrl_handler_free(&dev->ctrl_handler); v4l2_device_unregister(&dev->v4l2_dev); + mutex_unlock(&dev->lock); + kref_put(&dev->ref, em28xx_free_device); return 0;