Message ID | 1333295631-31866-3-git-send-email-sgunderson@bigfoot.com (mailing list archive) |
---|---|
State | Rejected, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1SEN6W-0006mb-Td for patchwork@linuxtv.org; Sun, 01 Apr 2012 17:54:08 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-2) with esmtp for <patchwork@linuxtv.org> id 1SEN6W-0003Ix-HA; Sun, 01 Apr 2012 17:54:08 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752511Ab2DAPyE (ORCPT <rfc822;patchwork@linuxtv.org>); Sun, 1 Apr 2012 11:54:04 -0400 Received: from cassarossa.samfundet.no ([129.241.93.19]:40794 "EHLO cassarossa.samfundet.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751948Ab2DAPyD (ORCPT <rfc822; linux-media@vger.kernel.org>); Sun, 1 Apr 2012 11:54:03 -0400 Received: from pannekake.samfundet.no ([2001:700:300:1800::dddd] ident=unknown) by cassarossa.samfundet.no with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.72) (envelope-from <sesse@pannekake.samfundet.no>) id 1SEN6P-0001UP-Qc; Sun, 01 Apr 2012 17:54:02 +0200 Received: from sesse by pannekake.samfundet.no with local (Exim 4.72) (envelope-from <sesse@pannekake.samfundet.no>) id 1SEN6P-0008M0-Hu; Sun, 01 Apr 2012 17:54:01 +0200 From: "Steinar H. Gunderson" <sgunderson@bigfoot.com> To: linux-media@vger.kernel.org Cc: "Steinar H. Gunderson" <sesse@samfundet.no> Subject: [PATCH 03/11] Hack to fix a mutex issue in the DVB layer. Date: Sun, 1 Apr 2012 17:53:43 +0200 Message-Id: <1333295631-31866-3-git-send-email-sgunderson@bigfoot.com> X-Mailer: git-send-email 1.7.2.5 In-Reply-To: <20120401155330.GA31901@uio.no> References: <20120401155330.GA31901@uio.no> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2012.4.1.154227 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1600_1699 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __SANE_MSGID 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' |
Commit Message
Steinar H. Gunderson
April 1, 2012, 3:53 p.m. UTC
From: "Steinar H. Gunderson" <sesse@samfundet.no> dvb_usercopy(), which is called on all ioctls, not only copies data to and from userspace, but also takes a lock on the file descriptor, which means that only one ioctl can run at a time. This means that if one thread of mumudvb is busy trying to get, say, the SNR from the frontend (which can hang due to the issue above), the CAM thread's ioctl(fd, CA_GET_SLOT_INFO, ...) will hang, even though it doesn't need to communicate with the hardware at all. This obviously requires a better fix, but I don't know the generic DVB layer well enough to say what it is. Maybe it's some sort of remnant of from when all ioctl()s took the BKL. Note that on UMP kernels without preemption, mutex_lock is to the best of my knowledge a no-op, so these delay issues would not show up on non-SMP. Signed-off-by: Steinar H. Gunderson <sesse@samfundet.no> --- drivers/media/dvb/dvb-core/dvbdev.c | 2 -- 1 file changed, 2 deletions(-)
Comments
Em 01-04-2012 12:53, Steinar H. Gunderson escreveu: > From: "Steinar H. Gunderson" <sesse@samfundet.no> > > dvb_usercopy(), which is called on all ioctls, not only copies data to and from > userspace, but also takes a lock on the file descriptor, which means that only > one ioctl can run at a time. This means that if one thread of mumudvb is busy > trying to get, say, the SNR from the frontend (which can hang due to the issue > above), the CAM thread's ioctl(fd, CA_GET_SLOT_INFO, ...) will hang, even > though it doesn't need to communicate with the hardware at all. This obviously > requires a better fix, but I don't know the generic DVB layer well enough to > say what it is. Maybe it's some sort of remnant of from when all ioctl()s took > the BKL. Note that on UMP kernels without preemption, mutex_lock is to the best > of my knowledge a no-op, so these delay issues would not show up on non-SMP. > > Signed-off-by: Steinar H. Gunderson <sesse@samfundet.no> > --- > drivers/media/dvb/dvb-core/dvbdev.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c > index 00a6732..e1217f6 100644 > --- a/drivers/media/dvb/dvb-core/dvbdev.c > +++ b/drivers/media/dvb/dvb-core/dvbdev.c > @@ -417,10 +417,8 @@ int dvb_usercopy(struct file *file, > } > > /* call driver */ > - mutex_lock(&dvbdev_mutex); > if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD) > err = -EINVAL; > - mutex_unlock(&dvbdev_mutex); As-is, this would be too risky, as it may break random drivers. A change like that would require to push down the mutex lock into each caller for dvb_user_copy: drivers/media/dvb/dvb-core/dmxdev.c: return dvb_usercopy(file, cmd, arg, dvb_demux_do_ioctl); drivers/media/dvb/dvb-core/dmxdev.c: return dvb_usercopy(file, cmd, arg, dvb_dvr_do_ioctl); drivers/media/dvb/dvb-core/dvb_ca_en50221.c: return dvb_usercopy(file, cmd, arg, dvb_ca_en50221_io_do_ioctl); drivers/media/dvb/dvb-core/dvb_net.c: return dvb_usercopy(file, cmd, arg, dvb_net_do_ioctl); drivers/media/dvb/dvb-core/dvbdev.c: return dvb_usercopy(file, cmd, arg, dvbdev->kernel_ioctl); drivers/media/dvb/dvb-core/dvbdev.c:int dvb_usercopy(struct file *file, drivers/media/dvb/dvb-core/dvbdev.h:we simply define out own dvb_usercopy(), which will hopefully become drivers/media/dvb/dvb-core/dvbdev.h:extern int dvb_usercopy(struct file *file, unsigned int cmd, unsigned long arg, $ git grep kernel_ioctl drivers/media/dvb/ drivers/media/dvb/dvb-core/dvb_frontend.c: .kernel_ioctl = dvb_frontend_ioctl drivers/media/dvb/dvb-core/dvbdev.c: if (!dvbdev->kernel_ioctl) drivers/media/dvb/dvb-core/dvbdev.c: return dvb_usercopy(file, cmd, arg, dvbdev->kernel_ioctl); drivers/media/dvb/dvb-core/dvbdev.h: int (*kernel_ioctl)(struct file *file, unsigned int cmd, void *arg); drivers/media/dvb/firewire/firedtv-ci.c: .kernel_ioctl = fdtv_ca_ioctl, drivers/media/dvb/ttpci/av7110.c: .kernel_ioctl = dvb_osd_ioctl, drivers/media/dvb/ttpci/av7110_av.c: .kernel_ioctl = dvb_video_ioctl, drivers/media/dvb/ttpci/av7110_av.c: .kernel_ioctl = dvb_audio_ioctl, drivers/media/dvb/ttpci/av7110_ca.c: .kernel_ioctl = dvb_ca_ioctl, And optimize the code there to avoid uneeded locks. > > if (err < 0) > goto out; -- 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
diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c index 00a6732..e1217f6 100644 --- a/drivers/media/dvb/dvb-core/dvbdev.c +++ b/drivers/media/dvb/dvb-core/dvbdev.c @@ -417,10 +417,8 @@ int dvb_usercopy(struct file *file, } /* call driver */ - mutex_lock(&dvbdev_mutex); if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD) err = -EINVAL; - mutex_unlock(&dvbdev_mutex); if (err < 0) goto out;