| Message ID | 1302900017-10437-1-git-send-email-khoroshilov@ispras.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, 2011-04-16 at 00:40 +0400, Alexey Khoroshilov wrote: > There are a couple of places where mutex_unlock() is called even > if mutex_lock_interruptible() failed. The patch fixes the issue. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > --- > drivers/media/dvb/dvb-usb/lmedm04.c | 12 +++++++----- > 1 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c > index f2db012..40907df 100644 > --- a/drivers/media/dvb/dvb-usb/lmedm04.c > +++ b/drivers/media/dvb/dvb-usb/lmedm04.c > @@ -591,9 +591,10 @@ static int lme2510_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff) > else { > deb_info(1, "STM Steam Off"); > /* mutex is here only to avoid collision with I2C */ > - ret = mutex_lock_interruptible(&adap->dev->i2c_mutex); > + if (mutex_lock_interruptible(&adap->dev->i2c_mutex) < 0) > + return -EAGAIN; > > - ret |= lme2510_usb_talk(adap->dev, clear_reg_3, > + ret = lme2510_usb_talk(adap->dev, clear_reg_3, > sizeof(clear_reg_3), rbuf, rlen); > st->stream_on = 0; > st->i2c_talk_onoff = 1; Unfortunately dvb-usb does not provide -EAGAIN on streaming_ctrl stopping and returns 0 instead. This code is deliberate even if the i2c_mutex is unlocked, the code must be executed, and is protected by on going i2c_mutex by the usb_mutex. This patch produces a intermittent stall on channel change. Regards Malcolm -- 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
On Sat, 2011-04-16 at 01:01 +0100, Malcolm Priestley wrote: > On Sat, 2011-04-16 at 00:40 +0400, Alexey Khoroshilov wrote: > > There are a couple of places where mutex_unlock() is called even > > if mutex_lock_interruptible() failed. The patch fixes the issue. > > > > Found by Linux Driver Verification project (linuxtesting.org). > > > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > > --- > > drivers/media/dvb/dvb-usb/lmedm04.c | 12 +++++++----- > > 1 files changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c > > index f2db012..40907df 100644 > > --- a/drivers/media/dvb/dvb-usb/lmedm04.c > > +++ b/drivers/media/dvb/dvb-usb/lmedm04.c > > @@ -591,9 +591,10 @@ static int lme2510_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff) > > else { > > deb_info(1, "STM Steam Off"); > > /* mutex is here only to avoid collision with I2C */ > > - ret = mutex_lock_interruptible(&adap->dev->i2c_mutex); > > + if (mutex_lock_interruptible(&adap->dev->i2c_mutex) < 0) > > + return -EAGAIN; > > > > - ret |= lme2510_usb_talk(adap->dev, clear_reg_3, > > + ret = lme2510_usb_talk(adap->dev, clear_reg_3, > > sizeof(clear_reg_3), rbuf, rlen); > > st->stream_on = 0; > > st->i2c_talk_onoff = 1; > > Unfortunately dvb-usb does not provide -EAGAIN on streaming_ctrl > stopping and returns 0 instead. > I have added a patch to dvb-usb to return device errors to the demuxer This patch may now be applied. Tested by: Malcolm Priestley <tvboxspy@gmail.com> Regards Malcolm -- 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-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c index f2db012..40907df 100644 --- a/drivers/media/dvb/dvb-usb/lmedm04.c +++ b/drivers/media/dvb/dvb-usb/lmedm04.c @@ -591,9 +591,10 @@ static int lme2510_streaming_ctrl(struct dvb_usb_adapter *adap, int onoff) else { deb_info(1, "STM Steam Off"); /* mutex is here only to avoid collision with I2C */ - ret = mutex_lock_interruptible(&adap->dev->i2c_mutex); + if (mutex_lock_interruptible(&adap->dev->i2c_mutex) < 0) + return -EAGAIN; - ret |= lme2510_usb_talk(adap->dev, clear_reg_3, + ret = lme2510_usb_talk(adap->dev, clear_reg_3, sizeof(clear_reg_3), rbuf, rlen); st->stream_on = 0; st->i2c_talk_onoff = 1; @@ -1017,12 +1018,13 @@ static int lme2510_powerup(struct dvb_usb_device *d, int onoff) static u8 rbuf[1]; int ret, len = 3, rlen = 1; - ret = mutex_lock_interruptible(&d->i2c_mutex); + if (mutex_lock_interruptible(&d->i2c_mutex) < 0) + return -EAGAIN; if (onoff) - ret |= lme2510_usb_talk(d, lnb_on, len, rbuf, rlen); + ret = lme2510_usb_talk(d, lnb_on, len, rbuf, rlen); else - ret |= lme2510_usb_talk(d, lnb_off, len, rbuf, rlen); + ret = lme2510_usb_talk(d, lnb_off, len, rbuf, rlen); st->i2c_talk_onoff = 1;
There are a couple of places where mutex_unlock() is called even if mutex_lock_interruptible() failed. The patch fixes the issue. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> --- drivers/media/dvb/dvb-usb/lmedm04.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)