Re: [PATCH] dvb-core: Fix several locking related problems.

2007-03-10 Thread Mauro Carvalho Chehab 
Em Sáb, 2007-03-10 às 02:49 +0100, Johannes Stezenbach escreveu:
> On Sun, Mar 04, 2007 at 05:45:54PM +, Simon Arlott wrote:
> > Fix 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.
> > 
> > These cause 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.
> > 
> > Signed-off-by: Simon Arlott <[EMAIL PROTECTED]>
> 
> Acked-By: Johannes Stezenbach <[EMAIL PROTECTED]>
> 
> I can't test this but to me it looks good.
> Mauro, could you please pick it up and keep it in the
> linuxtv.org repository for a while for testing?

Done.
Thanks, Johannes.

-- 
Cheers,
Mauro

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] dvb-core: Fix several locking related problems.

2007-03-10 Thread Mauro Carvalho Chehab 
Em Sáb, 2007-03-10 às 02:49 +0100, Johannes Stezenbach escreveu:
 On Sun, Mar 04, 2007 at 05:45:54PM +, Simon Arlott wrote:
  Fix 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.
  
  These cause 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.
  
  Signed-off-by: Simon Arlott [EMAIL PROTECTED]
 
 Acked-By: Johannes Stezenbach [EMAIL PROTECTED]
 
 I can't test this but to me it looks good.
 Mauro, could you please pick it up and keep it in the
 linuxtv.org repository for a while for testing?

Done.
Thanks, Johannes.

-- 
Cheers,
Mauro

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] dvb-core: Fix several locking related problems.

2007-03-09 Thread Johannes Stezenbach 
On Sun, Mar 04, 2007 at 05:45:54PM +, Simon Arlott wrote:
> Fix 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.
> 
> These cause 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.
> 
> Signed-off-by: Simon Arlott <[EMAIL PROTECTED]>

Acked-By: Johannes Stezenbach <[EMAIL PROTECTED]>

I can't test this but to me it looks good.
Mauro, could you please pick it up and keep it in the
linuxtv.org repository for a while for testing?


Thanks,
Johannes


> ---
> On 04/03/07 15:41, Andreas Oberritter wrote:
> >please send an updated patch together with
> >Signed-off-by line to Mauro <[EMAIL PROTECTED]> and ask him to apply
> >it for inclusion into the -mm tree for further testing.
> 
> Unless there are other -mm trees I've not heard about, presumably I should 
> just do this myself. Doesn't linux-dvb have it's own development tree this 
> would get better tested in?
> 
> The dvb_dvr_release change has been working for me for 6 months and the 
> dvb_dmxdev_filter_free (dvb_dmxdev_filter_free) change looks equivalent.
> See http://www.linuxtv.org/pipermail/linux-dvb/2007-February/016120.html 
> for an example of the bug before and after fixing.
> 
> All the other changes run ok for me but should have lockdep enabled when 
> testing (if there's a possible deadlock somewhere, using _interruptible 
> will hide it).
> 
> 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..a5c0e1a 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 *inode, struct 
> file *file)
>   struct dvb_device *dvbdev = file->private_data;
>   struct dmxdev *dmxdev = dvbdev->priv;
> 
> - if (mutex_lock_interruptible(>mutex))
> - return -ERESTARTSYS;
> + mutex_lock(>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 *inode, struct 
> file *file)
> static int dvb_dmxdev_filter_free(struct dmxdev *dmxdev,
> struct dmxdev_filter *dmxdevfilter)
> {
> - if (mutex_lock_interruptible(>mutex))
> - return -ERESTARTSYS;
> -
> - if (mutex_lock_interruptible(>mutex)) {
> - mutex_unlock(>mutex);
> - return -ERESTARTSYS;
> - }
> + mutex_lock(>mutex);
> + mutex_lock(>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(struct 
> dmx_ts_feed *ts_feed)
>   struct dvb_demux *demux = feed->demux;
>   int ret;
> 
> - if (mutex_lock_interruptible(>mutex))
> - return -ERESTARTSYS;
> + mutex_lock(>mutex);
> 
>   if (feed->state < DMX_STATE_GO) {
>   mutex_unlock(>mutex);
> @@ -748,8 +747,7 @@ static int dvbdmx_release_ts_feed(struct dmx_demux *dmx,
>   struct dvb_demux *demux = (struct dvb_demux *)dmx;
>   struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
> 
> - if (mutex_lock_interruptible(>mutex))
> - return -ERESTARTSYS;
> + mutex_lock(>mutex);
> 
>   if (feed->state == DMX_STATE_FREE) {
>   mutex_unlock(>mutex);
> @@ -916,8 +914,7 @@ static int dmx_section_feed_stop_filtering(struct 
> dmx_section_feed *feed)
>   struct dvb_demux *dvbdmx = dvbdmxfeed->demux;
>   int ret;
> 
> - if (mutex_lock_interruptible(>mutex))
> - return -ERESTARTSYS;
> + mutex_lock(>mutex);
> 
>   if (!dvbdmx->stop_feed) {
>   mutex_unlock(>mutex);
> @@ -942,8 +939,7 @@ static int dmx_section_feed_release_filter(struct 
> dmx_section_feed *feed,
>   struct dvb_demux_feed *dvbdmxfeed = (struct dvb_demux_feed *)feed;
>   struct dvb_demux *dvbdmx = dvbdmxfeed->demux;
> 
> - if (mutex_lock_interruptible(>mutex))
> - return -ERESTARTSYS;
> + mutex_lock(>mutex);
> 
>   if (dvbdmxfilter->feed != dvbdmxfeed) {
>   mutex_unlock(>mutex);
> @@ -1016,8 +1012,7 @@

Re: [PATCH] dvb-core: Fix several locking related problems.

2007-03-09 Thread Johannes Stezenbach 
On Sun, Mar 04, 2007 at 05:45:54PM +, Simon Arlott wrote:
 Fix 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.
 
 These cause 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.
 
 Signed-off-by: Simon Arlott [EMAIL PROTECTED]

Acked-By: Johannes Stezenbach [EMAIL PROTECTED]

I can't test this but to me it looks good.
Mauro, could you please pick it up and keep it in the
linuxtv.org repository for a while for testing?


Thanks,
Johannes


 ---
 On 04/03/07 15:41, Andreas Oberritter wrote:
 please send an updated patch together with
 Signed-off-by line to Mauro [EMAIL PROTECTED] and ask him to apply
 it for inclusion into the -mm tree for further testing.
 
 Unless there are other -mm trees I've not heard about, presumably I should 
 just do this myself. Doesn't linux-dvb have it's own development tree this 
 would get better tested in?
 
 The dvb_dvr_release change has been working for me for 6 months and the 
 dvb_dmxdev_filter_free (dvb_dmxdev_filter_free) change looks equivalent.
 See http://www.linuxtv.org/pipermail/linux-dvb/2007-February/016120.html 
 for an example of the bug before and after fixing.
 
 All the other changes run ok for me but should have lockdep enabled when 
 testing (if there's a possible deadlock somewhere, using _interruptible 
 will hide it).
 
 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..a5c0e1a 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 *inode, struct 
 file *file)
   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 *inode, struct 
 file *file)
 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(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(struct 
 dmx_ts_feed *ts_feed)
   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 dmx_demux *dmx,
   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_filtering(struct 
 dmx_section_feed *feed)
   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_filter(struct 
 dmx_section_feed *feed,
   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