From patchwork Sun Dec 22 14:53:06 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 3394251 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 EA0349F314 for ; Sun, 22 Dec 2013 14:53:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C5206205B3 for ; Sun, 22 Dec 2013 14:53:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 367D02056F for ; Sun, 22 Dec 2013 14:53:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754134Ab3LVOxN (ORCPT ); Sun, 22 Dec 2013 09:53:13 -0500 Received: from mailout2.w2.samsung.com ([211.189.100.12]:34690 "EHLO usmailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753580Ab3LVOxM convert rfc822-to-8bit (ORCPT ); Sun, 22 Dec 2013 09:53:12 -0500 Received: from uscpsbgm1.samsung.com (u114.gpu85.samsung.co.kr [203.254.195.114]) by mailout2.w2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MY7003AOQONCS50@mailout2.w2.samsung.com> for linux-media@vger.kernel.org; Sun, 22 Dec 2013 09:53:11 -0500 (EST) X-AuditID: cbfec372-b7fa96d000006a7b-e2-52b6fcd7e730 Received: from ussync3.samsung.com ( [203.254.195.83]) by uscpsbgm1.samsung.com (USCPMTA) with SMTP id 12.D0.27259.7DCF6B25; Sun, 22 Dec 2013 09:53:11 -0500 (EST) Received: from localhost.localdomain ([105.144.34.14]) by ussync3.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MY700CKPQOJJL50@ussync3.samsung.com>; Sun, 22 Dec 2013 09:53:11 -0500 (EST) Date: Sun, 22 Dec 2013 12:53:06 -0200 From: Mauro Carvalho Chehab To: Frank =?UTF-8?B?U2Now6RmZXI=?= Cc: Antti Palosaari , Linux Media Mailing List Subject: Re: em28xx DEADLOCK reported by lock debug Message-id: <20131222125306.671b9960@samsung.com> In-reply-to: <52B6EE79.9070105@googlemail.com> References: <52B1C79C.1070408@iki.fi> <52B5C718.7030605@googlemail.com> <52B5F229.6020301@iki.fi> <52B6EE79.9070105@googlemail.com> X-Mailer: Claws Mail 3.9.2 (GTK+ 2.24.22; x86_64-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 8BIT X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprHLMWRmVeSWpSXmKPExsVy+t/hYN3rf7YFGTSvkbH43vyezWLr7M/M Fj0btrI6MHs8nTCZ3ePw14UsHp83yQUwR3HZpKTmZJalFunbJXBlrLtsWtAjUfHgyVT2BsbN wl2MHBwSAiYSB4+JdDFyApliEhfurWfrYuTiEBJYwihx9NtdKKeHSWL212PsIFUsAqoS7+dd YQGx2QSMJF41trCC2CICjhLf5v5mBrGZBWIkPvzazAZiCwsYS8xaeR+shlfAUGJux24mEJtT QE9i/4EHzBALWhklDrXcZIa4yEli9wEpiHpBiR+T77FAzFSXmDRvEdR8bYkn7y6wTmAUmIWk bBaSsllIyhYwMq9iFC0tTi4oTkrPNdQrTswtLs1L10vOz93ECAnToh2MzzZYHWIU4GBU4uHN ENsWJMSaWFZcmXuIUYKDWUmEN/oHUIg3JbGyKrUoP76oNCe1+BAjEwenVAPjuTmbfzyqd74f f/VcsQ6v3ezZ/g8yDmuFz+X6fLDilCD/JPmTzvIK/tLpBtenctnX3P6fx17RFHKqLXFd4MQ3 0f0MejX3Wq/VF+76IBkeW3VOufa4sl7SRtk6Mw+FbzzFs/cGxRZXahacfnzkenlYS0Ffne4R 5T/vjapElb7Zamb+Z9qTuViJpTgj0VCLuag4EQDMAlBPMQIAAA== 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.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 Em Sun, 22 Dec 2013 14:51:53 +0100 Frank Schäfer escreveu: > Am 21.12.2013 20:55, schrieb Antti Palosaari: > > On 21.12.2013 18:51, Frank Schäfer wrote: > >> Hi Antti, > >> > >> thank you for reporting this issue. > >> > >> Am 18.12.2013 17:04, schrieb Antti Palosaari: > >>> That same lock debug deadlock is still there (maybe ~4 times I report > >>> it during 2 years). Is that possible to fix easily at all? > >> > >> Patches are always welcome. ;) > > > > haha, I cannot simply learn every driver I meet some problems... > Hint: > > If you report a bug ~4 times in 2 years but never get a reply, it > usually means > a) nobody cares > b) nobody has the resources (time, knowledge) to fix it. > > So you either have to live with this issue or to fix it yourself. It is the latter case: fixing it require lots of efforts. One way to fix would be to change em28xx_close_extension() to something like: Please note that the above is not 100% correct, as one device may have more than one extension. Then, it should be sure that on every place that em28xx_close_extension() is called, dev->lock is not taken. As an alternative, eventually the extension list could be moved to the struct em28xx, but a device list is still needed, in order to handle extension module removal. Another way that would probably be better is to convert the em28xx code that handles extension (extension here is dvb, rc, alsa) to use krefs, And add a kref free code that would call ops->fini. Note that, in this case, dev itself would also need to be a kref. I suspect that using kref would would be cleaner, but a change like that would require to rewrite the extensions code. Btw, there's a related RFC patchset that splits the V4L2 interface from em28xx, transforming it also into an extension. With such patch, a DVB only device should not call any v4l2 init code, nor require V4L2 to be enabled: https://patchwork.linuxtv.org/patch/17967/ The above RFC requires testing. I may be able to find some time to do work on it this end of the year, starting with the V4L2 split patchset, depending if I finish some other things already on my todo list. Regards, Mauro diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c index f6076a512e8f..d938e2bbd62f 100644 --- a/drivers/media/usb/em28xx/em28xx-core.c +++ b/drivers/media/usb/em28xx/em28xx-core.c @@ -1350,13 +1350,19 @@ void em28xx_init_extension(struct em28xx *dev) void em28xx_close_extension(struct em28xx *dev) { + int (*fini)(struct em28xx *) = NULL; const struct em28xx_ops *ops = NULL; mutex_lock(&em28xx_devlist_mutex); list_for_each_entry(ops, &em28xx_extension_devlist, next) { - if (ops->fini) - ops->fini(dev); + fini = ops->fini; } list_del(&dev->devlist); mutex_unlock(&em28xx_devlist_mutex); + + if (fini) { + mutex_lock(&dev->lock); + fini(dev); + mutex_unlock(&dev->lock); + } }