Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Gregor Jasny]

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]

Hi,

During some tests I've noticed that the VIDIOCGMBUF ioctl hangs on my
bttv video device. It simply does not return and the process is stuck in
the D+ state. With Kernel 2.6.22.9 the (attached) testcase works like a
charm. The V4L2 interface is working with 2.6.24, too (at least displays
xawtv the usual pixel snow).

Hard and Software:
64 bit Linux 2.6.24-rc6 (todays pull) on a Intel Core 2 Duo

WinTV PCI:
05:02.0 0400: 109e:036e (rev 02)
05:02.1 0480: 109e:0878 (rev 02)

dmesg:
Rincewind:~# dmesg |grep bttv
bttv: driver version 0.9.17 loaded
bttv: using 8 buffers with 2080k (520 pages) each for capture
bttv: Bt8xx card found (0).
bttv0: Bt878 (rev 2) at 0000:05:02.0, irq: 23, latency: 64, mmio: 0xdfefe000
bttv0: detected: Hauppauge WinTV [card=10], PCI subsystem ID is 0070:13eb
bttv0: using: Hauppauge (bt878) [card=10,autodetected]
bttv0: gpio: en=00000000, out=00000000 in=00ffffdb [init]
bttv0: Hauppauge/Voodoo msp34xx: reset line init [5]
bttv0: Hauppauge eeprom indicates model#61324
bttv0: tuner type=14
bttv0: i2c: checking for MSP34xx @ 0x80... found
bttv0: i2c: checking for TDA9875 @ 0xb0... not found
bttv0: i2c: checking for TDA7432 @ 0x8a... not found
bttv0: registered device video0
bttv0: registered device vbi0
bttv0: registered device radio0
bttv0: PLL: 28636363 => 35468950 .. ok

from /proc/interrupts:
 23:      55900          0   IO-APIC-fasteoi   ehci_hcd:usb2, uhci_hcd:usb5, bttv0

If I could help with more logs, a bisection, some debug printks, please contact me.

Thanks!

Gregor

[-- Attachment #2: bttvregression.c --]
[-- Type: text/x-csrc, Size: 505 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <errno.h>

#include <sys/ioctl.h>
#include <sys/mman.h>
#include <fcntl.h>
#include <unistd.h>

#include <linux/ioctl.h>
#include <linux/videodev.h>

int main( int argc, char *argv[])
{
	const char device[] = "/dev/video0";
	int r;

	int fd = open (device, O_RDWR);
	assert(fd);

	struct video_mbuf mbuf = { 0 };
	r = ioctl (fd, VIDIOCGMBUF, &mbuf);
	assert (r == 0);

	printf ("Frames: %i\n", mbuf.frames);	

	close (fd);

	return 0;
}

Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Rafael J. Wysocki]

On Saturday, 5 of January 2008, Gregor Jasny wrote:
> Hi,
> 
> During some tests I've noticed that the VIDIOCGMBUF ioctl hangs on my
> bttv video device. It simply does not return and the process is stuck in
> the D+ state. With Kernel 2.6.22.9 the (attached) testcase works like a
> charm. The V4L2 interface is working with 2.6.24, too (at least displays
> xawtv the usual pixel snow).

Do the 2.6.23.x kernels work?

Rafael

[PATCH] Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Gregor Jasny]

From: Gregor Jasny <gjasny@web.de>

Fix bttv VIDIOCGMBUF locking like done in commit
820eacd84cff23b76693f4be1e28feb672f4488f. 

Signed-off-by: Gregor Jasny <gjasny@web.de>
---
diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c
index c02d92d..581a3c9 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -3063,11 +3063,10 @@ static int bttv_do_ioctl(struct inode *inode, struct file *file,
 		struct video_mbuf *mbuf = arg;
 		unsigned int i;
 
-		mutex_lock(&fh->cap.lock);
 		retval = videobuf_mmap_setup(&fh->cap,gbuffers,gbufsize,
 					     V4L2_MEMORY_MMAP);
 		if (retval < 0)
-			goto fh_unlock_and_return;
+			return retval;
 
 		gbuffers = retval;
 		memset(mbuf,0,sizeof(*mbuf));
@@ -3075,7 +3074,6 @@ static int bttv_do_ioctl(struct inode *inode, struct file *file,
 		mbuf->size   = gbuffers * gbufsize;
 		for (i = 0; i < gbuffers; i++)
 			mbuf->offsets[i] = i * gbufsize;
-		mutex_unlock(&fh->cap.lock);
 		return 0;
 	}
 	case VIDIOCMCAPTURE:

Re: [PATCH] Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Ingo Molnar]

* Gregor Jasny <jasny@vidsoft.de> wrote:

> From: Gregor Jasny <gjasny@web.de>
> 
> Fix bttv VIDIOCGMBUF locking like done in commit
> 820eacd84cff23b76693f4be1e28feb672f4488f. 

i've double-checked that there are no other video drivers affected by 
this problem:

 $ cd drivers/media/
 $ grep -C 10 videobuf_mmap_setup `grep -l mutex_lock */*.c */*/*.c */*/*/*.c`

 video/videobuf-core.c-  mutex_lock(&q->lock);
 video/videobuf-core.c-  mutex_lock(&q->lock);
 video/bt8xx/bttv-driver.c-              mutex_lock(&fh->cap.lock);

so only bttv-driver.c was affected, at first sight.

	Ingo

Re: [v4l-dvb-maintainer] [PATCH] Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Douglas Landgraf]

Hi guys,

    Gregor, we have converted bttv driver to use vidioc_ioctl2 some days ago.
Could you check and create your patch against v4l development tree?
Bttv driver does not have anymore bttv_do_ioctl().

Cheers,
Douglas

On Jan 6, 2008 12:15 PM, Gregor Jasny <jasny@vidsoft.de> wrote:
> From: Gregor Jasny <gjasny@web.de>
>
> Fix bttv VIDIOCGMBUF locking like done in commit
> 820eacd84cff23b76693f4be1e28feb672f4488f.
>
> Signed-off-by: Gregor Jasny <gjasny@web.de>
> ---
> diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c
> index c02d92d..581a3c9 100644
> --- a/drivers/media/video/bt8xx/bttv-driver.c
> +++ b/drivers/media/video/bt8xx/bttv-driver.c
> @@ -3063,11 +3063,10 @@ static int bttv_do_ioctl(struct inode *inode, struct file *file,
>                 struct video_mbuf *mbuf = arg;
>                 unsigned int i;
>
> -               mutex_lock(&fh->cap.lock);
>                 retval = videobuf_mmap_setup(&fh->cap,gbuffers,gbufsize,
>                                              V4L2_MEMORY_MMAP);
>                 if (retval < 0)
> -                       goto fh_unlock_and_return;
> +                       return retval;
>
>                 gbuffers = retval;
>                 memset(mbuf,0,sizeof(*mbuf));
> @@ -3075,7 +3074,6 @@ static int bttv_do_ioctl(struct inode *inode, struct file *file,
>                 mbuf->size   = gbuffers * gbufsize;
>                 for (i = 0; i < gbuffers; i++)
>                         mbuf->offsets[i] = i * gbufsize;
> -               mutex_unlock(&fh->cap.lock);
>                 return 0;
>         }
>         case VIDIOCMCAPTURE:
>
>
> _______________________________________________
> v4l-dvb-maintainer mailing list
> v4l-dvb-maintainer@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
>

Re: [v4l-dvb-maintainer] [PATCH] Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Douglas Landgraf]

I mean video_ioctl2 not vidioc_ioctl2, sorry.

Cheers,
Douglas

On Jan 6, 2008 8:53 PM, Douglas Landgraf <dougsland@gmail.com> wrote:
> Hi guys,
>
>     Gregor, we have converted bttv driver to use vidioc_ioctl2 some days ago.
> Could you check and create your patch against v4l development tree?
> Bttv driver does not have anymore bttv_do_ioctl().
>
> Cheers,
> Douglas
>
>
> On Jan 6, 2008 12:15 PM, Gregor Jasny <jasny@vidsoft.de> wrote:
> > From: Gregor Jasny <gjasny@web.de>
> >
> > Fix bttv VIDIOCGMBUF locking like done in commit
> > 820eacd84cff23b76693f4be1e28feb672f4488f.
> >
> > Signed-off-by: Gregor Jasny <gjasny@web.de>
> > ---
> > diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c
> > index c02d92d..581a3c9 100644
> > --- a/drivers/media/video/bt8xx/bttv-driver.c
> > +++ b/drivers/media/video/bt8xx/bttv-driver.c
> > @@ -3063,11 +3063,10 @@ static int bttv_do_ioctl(struct inode *inode, struct file *file,
> >                 struct video_mbuf *mbuf = arg;
> >                 unsigned int i;
> >
> > -               mutex_lock(&fh->cap.lock);
> >                 retval = videobuf_mmap_setup(&fh->cap,gbuffers,gbufsize,
> >                                              V4L2_MEMORY_MMAP);
> >                 if (retval < 0)
> > -                       goto fh_unlock_and_return;
> > +                       return retval;
> >
> >                 gbuffers = retval;
> >                 memset(mbuf,0,sizeof(*mbuf));
> > @@ -3075,7 +3074,6 @@ static int bttv_do_ioctl(struct inode *inode, struct file *file,
> >                 mbuf->size   = gbuffers * gbufsize;
> >                 for (i = 0; i < gbuffers; i++)
> >                         mbuf->offsets[i] = i * gbufsize;
> > -               mutex_unlock(&fh->cap.lock);
> >                 return 0;
> >         }
> >         case VIDIOCMCAPTURE:
> >
> >
> > _______________________________________________
> > v4l-dvb-maintainer mailing list
> > v4l-dvb-maintainer@linuxtv.org
> > http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
> >
>

Re: [v4l-dvb-maintainer] [PATCH] Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Ingo Molnar]

* Douglas Landgraf <dougsland@gmail.com> wrote:

> Hi guys,
> 
>     Gregor, we have converted bttv driver to use vidioc_ioctl2 some 
> days ago. Could you check and create your patch against v4l 
> development tree? Bttv driver does not have anymore bttv_do_ioctl().

this is about the bttv driver in 2.6.24-rc6/(rc7?) hanging. So whatever 
the devel tree does, this fix (if it's a correct fix) needs to be 
commited upstream ASAP.

	Ingo

ps. http://www.zip.com.au/~akpm/linux/patches/stuff/top-posting.txt

Re: [v4l-dvb-maintainer] [PATCH] Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Douglas Landgraf]

Hi Ingo,

> this is about the bttv driver in 2.6.24-rc6/(rc7?) hanging. So whatever
> the devel tree does, this fix (if it's a correct fix) needs to be
> commited upstream ASAP.

You are right, my misunderstood. Sorry guys!

Cheers,
Douglas

Re: [v4l-dvb-maintainer] [PATCH] Re: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

[Mauro Carvalho Chehab]

On Mon, 7 Jan 2008 00:22:34 +0100
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Douglas Landgraf <dougsland@gmail.com> wrote:
> 
> > Hi guys,
> > 
> >     Gregor, we have converted bttv driver to use vidioc_ioctl2 some 
> > days ago. Could you check and create your patch against v4l 
> > development tree? Bttv driver does not have anymore bttv_do_ioctl().
> 
> this is about the bttv driver in 2.6.24-rc6/(rc7?) hanging. So whatever 
> the devel tree does, this fix (if it's a correct fix) needs to be 
> commited upstream ASAP.

For sure. I've just sent Linus a pull request for the fix.

On the devel tree, all this code got removed, replaced by a simple function
call at videobuf. So, I don't expect similar issues for 2.6.25.

I've already fixed the patch conflicts at devel branch for v4l/dvb -git tree.

Cheers,
Mauro