From patchwork Tue Aug 16 08:50:01 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Rankin X-Patchwork-Id: 7612 Return-path: Envelope-to: mchehab@infradead.org Delivery-date: Tue, 16 Aug 2011 08:55:26 +0000 Received: from casper.infradead.org [85.118.1.10] by gaivota with IMAP (fetchmail-6.3.20) for (single-drop); Tue, 16 Aug 2011 07:51:04 -0700 (PDT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QtFQj-00056i-Rd; Tue, 16 Aug 2011 08:55:26 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751625Ab1HPIzY (ORCPT + 1 other); Tue, 16 Aug 2011 04:55:24 -0400 Received: from nm1-vm0.bt.bullet.mail.ird.yahoo.com ([212.82.108.94]:41778 "HELO nm1-vm0.bt.bullet.mail.ird.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751270Ab1HPIzX (ORCPT ); Tue, 16 Aug 2011 04:55:23 -0400 X-Greylist: delayed 317 seconds by postgrey-1.27 at vger.kernel.org; Tue, 16 Aug 2011 04:55:23 EDT Received: from [212.82.108.230] by nm1.bt.bullet.mail.ird.yahoo.com with NNFMP; 16 Aug 2011 08:50:05 -0000 Received: from [212.82.108.224] by tm3.bt.bullet.mail.ird.yahoo.com with NNFMP; 16 Aug 2011 08:50:05 -0000 Received: from [127.0.0.1] by omp1001.bt.mail.ird.yahoo.com with NNFMP; 16 Aug 2011 08:50:05 -0000 X-Yahoo-Newman-Id: 224078.31214.bm@omp1001.bt.mail.ird.yahoo.com Received: (qmail 96006 invoked from network); 16 Aug 2011 08:50:05 -0000 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=yahoo.com; h=DKIM-Signature:X-Yahoo-Newman-Property:X-YMail-OSG:X-Yahoo-SMTP:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:Content-Type; b=37UqBVfNfK2/OwZGze5V7Md4zTaE4V9tZMv25y0ipC2thSnRij8fM6Gp/OlqKcnXLiyk1G0w0X4CyUHQ/W6D9c8Lgg2PsK9q47eDRQedKnjcsn4NHw5A9+DNLWtjg997POYD9w29EpsCmtnxUda2yeZf0TUvnC2dn+E2pWCKopU= ; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s1024; t=1313484605; bh=8gHk7bdwuzIKEkHIaPlrdkmcEbSYEQSNCrFUQRVLYMk=; h=X-Yahoo-Newman-Property:X-YMail-OSG:X-Yahoo-SMTP:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:Content-Type; b=wdNG//C4blALWxCV3OIOQ4bPizS+McMzL+3bdMxyMi1uTsEHrNsvIeL30FNQlqh/0yYmhaGaoUgF8vCPOZaiCZrweGArZSE6uaYKk5qHKNAPlSVl1KJ832bmsjQyS1ehzVTVAPH2ueZqPU6M9Sde3fiqWcwyAiywY9kdGE/KIeM= X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: WtpD_5cVM1kmu.m5pBC1ioMVxOyRjb.UgyE9qNZF9jb7k7t E5oBZkAiizvzEpOvsPRw3pwwBUKT5YqOITw2GBV9pYtGLotDr95wPYoXSDgI BninIbsAd._1AOz6SAm8HPWNWwqc6jKc4ezthE96TsErG6hUL34uY02NqRRP RDuxoqvPLZpKrCqjLQ7frErLuOmUBlwjC.3CFIcjF5oaLQp7TmVdsvqgvwgT G.nL42esU8WxKjEalF2etgh_hTk9RKafoRzDQqhHJoOcnLdug1pB5ZooBWHC m79lUl4NpODYhXV..5w5S1sYC20tH2LSdy3sCtzQxVDLc_O0qKuaYg3Ml6bC 6ayxXsqBFuBgYfP1SlguS_VH07IyxzPBvjRoCSeSYB5x4WbADueiq9lf2TO7 VPjo4 X-Yahoo-SMTP: dMK34oyswBBlfKesWTI5ovDjFOUFE6shtILt.ZXnUEjQHhWq Received: from wellhouse.underworld (rankincj@86.178.152.254 with login) by smtp824.mail.ukl.yahoo.com with SMTP; 16 Aug 2011 08:50:04 +0000 GMT Received: from volcano.underworld (volcano.underworld [192.168.0.3]) by wellhouse.underworld (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id p7G8o1cp031143 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT) for ; Tue, 16 Aug 2011 09:50:03 +0100 Message-ID: <4E4A2F39.6090809@yahoo.com> Date: Tue, 16 Aug 2011 09:50:01 +0100 From: Chris Rankin User-Agent: Mozilla/5.0 (X11; Linux i686; rv:5.0) Gecko/20110707 Thunderbird/5.0 MIME-Version: 1.0 To: linux-media@vger.kernel.org Subject: [PATCH] Fix locking problem between em28xx and em28xx-dvb modules Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi, I've thought about this patch a bit more overnight, and it occurred to me that while em28xx_init_extension() now takes the device mutex followed by the device list mutex, my original fix would have had em28xx_register_extension() taking the device list mutex followed by the device mutex. And that sounds suspiciously like a potential deadlock to me. So I was wondering: does em28xx_register_extension() actually need to lock the device if the device list has already been locked? I've also moved two printk()s outside the region where we hold the device list mutex, because locked bits should be as brief as possible and neither printk() does anything that needs the lock. A new patch is attached, for review. Signed-off-by: Chris Rankin --- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-16 09:15:46.000000000 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-16 09:21:08.000000000 +0100 @@ -1193,8 +1193,8 @@ list_for_each_entry(dev, &em28xx_devlist, devlist) { ops->init(dev); } - printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name); mutex_unlock(&em28xx_devlist_mutex); + printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name); return 0; } EXPORT_SYMBOL(em28xx_register_extension); @@ -1207,9 +1207,9 @@ list_for_each_entry(dev, &em28xx_devlist, devlist) { ops->fini(dev); } - printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name); list_del(&ops->next); mutex_unlock(&em28xx_devlist_mutex); + printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name); } EXPORT_SYMBOL(em28xx_unregister_extension); --- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig 2011-08-16 09:16:03.000000000 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c 2011-08-16 09:17:06.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: