Re: [RFC/PATCH 0/3] locking fixes for cx88

[Andreas Huber]

Jonathan Nieder <jrnieder <at> gmail.com> writes:

> 
> Hi,
> 
> Huber Andreas wrote[1]:
> 
> > Processes that try to open a cx88-blackbird driven MPEG device will hang up.
> 
> Here's a possible fix based on a patch by Ben Hutchings and
> corrections from Andi Huber.  Warning: probably full of mistakes (my
> fault) since I'm not familiar with any of this stuff.  Untested.
> Review and testing would be welcome.
> 
> Ben Hutchings (2):
>   [media] cx88: fix locking of sub-driver operations
>   [media] cx88: use a mutex to protect cx8802_devlist
> 
> Jonathan Nieder (1):
>   [media] cx88: protect per-device driver list with device lock
> 
>  drivers/media/video/cx88/cx88-blackbird.c |    3 +-
>  drivers/media/video/cx88/cx88-dvb.c       |    2 +
>  drivers/media/video/cx88/cx88-mpeg.c      |   35 +++++++++++++++++++---------
>  drivers/media/video/cx88/cx88.h           |   10 +++++++-
>  4 files changed, 37 insertions(+), 13 deletions(-)
> 

There is an unrelated issue!!!

The driver's active_ref count may become negative which leads to unpredictable
behavior. (MPEG video device inaccessible, etc ...)

Here's a possible fix ...


diff -Nur a/drivers/media/video/cx88/cx88-mpeg.c
b/drivers/media/video/cx88/cx88-mpeg.c
--- a/drivers/media/video/cx88/cx88-mpeg.c	2011-04-02 14:34:21.456569849 +0200
+++ b/drivers/media/video/cx88/cx88-mpeg.c	2011-04-02 14:32:55.467038000 +0200
@@ -642,12 +642,16 @@
 {
 	struct cx88_core *core = drv->core;
 
+	mpeg_dbg(1,"%s active driver references before release attempt:
%d\n",core->name,core->active_ref);
+	
 	if (drv->advise_release && --core->active_ref == 0)
 	{
 		drv->advise_release(drv);
 		core->active_type_id = CX88_BOARD_NONE;
 		mpeg_dbg(1,"%s() Post release GPIO=%x\n", __func__, cx_read(MO_GP0_IO));
 	}
+	
+	if(core->active_ref<0) core->active_ref=0; // prevent us from getting negative
ref counts!
 
 	return 0;
 }


I can confirm that Johnathan's patchset plus applying this fix works!
Tests done on amd64 hardware with 2 Hauppauge HVR1300 TV cards: 
Both were independently able to tune channels and stream encoded video through
their MPEG encoder devices.

Andi.

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