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 <simon@xxxxxxxxxxx>
---
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 linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
