There are several instances of dvb-core functions using 
mutex_lock_interruptible and returning -ERESTARTSYS where the calling function 
will either never retry or never check the return value.

dmxdev.c:
        dvb_dmxdev_filter_free (via dvb_demux_release):
        dvb_dvr_release:
                return value ignored

This causes a race condition with dvb_dmxdev_filter_free and dvb_dvr_release, 
both of which are filesystem release functions whose return value is ignored 
and will never be retried. When this happens it becomes impossible to open dvr0 
again (-EBUSY) since it has not been released properly. I can only reproduce 
this by ^C-ing dvbrecord (which causes the bug it every time).

[  260.767599] dvb_dmxdev_filter_free: start
[  260.767612] dvb_dmxdev_filter_free: lock1 (dmxdev)
[  260.767615] dvb_dmxdev_filter_free lock2 (dmxdevfilter)
[  260.767683] dvb_dmxdev_filter_free: unlock (dmxdevfilter, dmxdev)

[  260.767690] dvb_dmxdev_filter_free: start
[  260.767693] dvb_dmxdev_filter_free: lock1 (dmxdev)
[  260.767696] dvb_dmxdev_filter_free lock2 (dmxdevfilter)
[  260.767897] dvb_dvr_release: start
[  260.767900] dvb_dvr_release: failed (dmxdev)
[  260.784656] dvb_dmxdev_filter_free: unlock (dmxdevfilter, dmxdev)

With the changes to dmxdev.c in this patch, the following happens instead:

[ 1194.907503] dvb_dmxdev_filter_free: start
[ 1194.907517] dvb_dmxdev_filter_free: lock1 (dmxdev)
[ 1194.907520] dvb_dmxdev_filter_free lock2 (dmxdevfilter)
[ 1194.907551] dvb_dmxdev_filter_free: unlock (dmxdevfilter, dmxdev)

[ 1194.907557] dvb_dmxdev_filter_free: start
[ 1194.907560] dvb_dmxdev_filter_free: lock1 (dmxdev)
[ 1194.907563] dvb_dmxdev_filter_free lock2 (dmxdevfilter)
[ 1194.907650] dvb_dvr_release: start
[ 1194.922452] dvb_dmxdev_filter_free: unlock (dmxdevfilter, dmxdev)
[ 1194.922716] dvb_dvr_release: lock (dmxdev)
[ 1194.922885] dvb_dvr_release: unlock (dmxdev)


The following functions also do something similar, some of which may be 
responsible for a bug I have which results in dvb breaking requiring a reboot:

[384577.489000] dvb_demux_feed_del: feed not in list (type=0 state=0 pid=ffff)
ioctl(DMX_SET_PES_FILTER) for Video PID failed (Invalid argument)

Since this happens seemingly at random (but usually 1MB into recording a TS) I 
can't tell if this fixes that bug or not until it doesn't happen again for a 
very long time, but I can confirm that this causes no deadlocks because I have 
CONFIG_LOCKDEP enabled.

dvb_demux.c:
        dmx_section_feed_stop_filtering:
        dmx_section_feed_release_filter:
        dmx_ts_feed_stop_filtering:
        dvbdmx_connect_frontend:
        dvbdmx_disconnect_frontend:
        dvbdmx_release_section_feed:
        dvbdmx_release_ts_feed:
                return value ignored
                        Most of these return values get ignored by dvb-core 
code itself.

dvbdev.c:
        dvb_register_device:
        dvb_register_adapter:
        dvb_unregister_adapter:
                return value ignored
                        Is there some important reason for these to never block?

Signed-off-by: Simon Arlott <[EMAIL PROTECTED]>
---
 drivers/media/dvb/dvb-core/dmxdev.c    |   12 +++---------
 drivers/media/dvb/dvb-core/dvb_demux.c |   21 +++++++--------------
 drivers/media/dvb/dvb-core/dvbdev.c    |    9 +++------
 3 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/drivers/media/dvb/dvb-core/dmxdev.c 
b/drivers/media/dvb/dvb-core/dmxdev.c
index fc77de4..66baaa8 100644
--- a/drivers/media/dvb/dvb-core/dmxdev.c
+++ b/drivers/media/dvb/dvb-core/dmxdev.c
@@ -180,8 +180,7 @@ static int dvb_dvr_release(struct inode 
        struct dvb_device *dvbdev = file->private_data;
        struct dmxdev *dmxdev = dvbdev->priv;
 
-       if (mutex_lock_interruptible(&dmxdev->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&dmxdev->mutex);
 
        if ((file->f_flags & O_ACCMODE) == O_WRONLY) {
                dmxdev->demux->disconnect_frontend(dmxdev->demux);
@@ -673,13 +672,8 @@ static int dvb_demux_open(struct inode *
 static int dvb_dmxdev_filter_free(struct dmxdev *dmxdev,
                                  struct dmxdev_filter *dmxdevfilter)
 {
-       if (mutex_lock_interruptible(&dmxdev->mutex))
-               return -ERESTARTSYS;
-
-       if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
-               mutex_unlock(&dmxdev->mutex);
-               return -ERESTARTSYS;
-       }
+       mutex_lock(&dmxdev->mutex);
+       mutex_lock_interruptible(&dmxdevfilter->mutex);
 
        dvb_dmxdev_filter_stop(dmxdevfilter);
        dvb_dmxdev_filter_reset(dmxdevfilter);
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c 
b/drivers/media/dvb/dvb-core/dvb_demux.c
index fcff5ea..6d8d1c3 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -673,8 +673,7 @@ static int dmx_ts_feed_stop_filtering(st
        struct dvb_demux *demux = feed->demux;
        int ret;
 
-       if (mutex_lock_interruptible(&demux->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&demux->mutex);
 
        if (feed->state < DMX_STATE_GO) {
                mutex_unlock(&demux->mutex);
@@ -748,8 +747,7 @@ static int dvbdmx_release_ts_feed(struct
        struct dvb_demux *demux = (struct dvb_demux *)dmx;
        struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
 
-       if (mutex_lock_interruptible(&demux->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&demux->mutex);
 
        if (feed->state == DMX_STATE_FREE) {
                mutex_unlock(&demux->mutex);
@@ -916,8 +914,7 @@ static int dmx_section_feed_stop_filteri
        struct dvb_demux *dvbdmx = dvbdmxfeed->demux;
        int ret;
 
-       if (mutex_lock_interruptible(&dvbdmx->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdmx->mutex);
 
        if (!dvbdmx->stop_feed) {
                mutex_unlock(&dvbdmx->mutex);
@@ -942,8 +939,7 @@ static int dmx_section_feed_release_filt
        struct dvb_demux_feed *dvbdmxfeed = (struct dvb_demux_feed *)feed;
        struct dvb_demux *dvbdmx = dvbdmxfeed->demux;
 
-       if (mutex_lock_interruptible(&dvbdmx->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdmx->mutex);
 
        if (dvbdmxfilter->feed != dvbdmxfeed) {
                mutex_unlock(&dvbdmx->mutex);
@@ -1016,8 +1012,7 @@ static int dvbdmx_release_section_feed(s
        struct dvb_demux_feed *dvbdmxfeed = (struct dvb_demux_feed *)feed;
        struct dvb_demux *dvbdmx = (struct dvb_demux *)demux;
 
-       if (mutex_lock_interruptible(&dvbdmx->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdmx->mutex);
 
        if (dvbdmxfeed->state == DMX_STATE_FREE) {
                mutex_unlock(&dvbdmx->mutex);
@@ -1126,8 +1121,7 @@ static int dvbdmx_connect_frontend(struc
        if (demux->frontend)
                return -EINVAL;
 
-       if (mutex_lock_interruptible(&dvbdemux->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdemux->mutex);
 
        demux->frontend = frontend;
        mutex_unlock(&dvbdemux->mutex);
@@ -1138,8 +1132,7 @@ static int dvbdmx_disconnect_frontend(st
 {
        struct dvb_demux *dvbdemux = (struct dvb_demux *)demux;
 
-       if (mutex_lock_interruptible(&dvbdemux->mutex))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdemux->mutex);
 
        demux->frontend = NULL;
        mutex_unlock(&dvbdemux->mutex);
diff --git a/drivers/media/dvb/dvb-core/dvbdev.c 
b/drivers/media/dvb/dvb-core/dvbdev.c
index ee9f723..7b69149 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -201,8 +201,7 @@ int dvb_register_device(struct dvb_adapt
        struct dvb_device *dvbdev;
        int id;
 
-       if (mutex_lock_interruptible(&dvbdev_register_lock))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdev_register_lock);
 
        if ((id = dvbdev_get_free_id (adap, type)) < 0) {
                mutex_unlock(&dvbdev_register_lock);
@@ -281,8 +280,7 @@ int dvb_register_adapter(struct dvb_adap
 {
        int num;
 
-       if (mutex_lock_interruptible(&dvbdev_register_lock))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdev_register_lock);
 
        if ((num = dvbdev_get_free_adapter_num ()) < 0) {
                mutex_unlock(&dvbdev_register_lock);
@@ -310,8 +308,7 @@ EXPORT_SYMBOL(dvb_register_adapter);
 
 int dvb_unregister_adapter(struct dvb_adapter *adap)
 {
-       if (mutex_lock_interruptible(&dvbdev_register_lock))
-               return -ERESTARTSYS;
+       mutex_lock(&dvbdev_register_lock);
        list_del (&adap->list_head);
        mutex_unlock(&dvbdev_register_lock);
        return 0;
-- 
1.4.3.1


-- 
Simon Arlott







Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Reply via email to