2008-01-05 22:34:00

by Gregor Jasny

[permalink] [raw]
Subject: Regression: VIDIOCGMBUF ioctl hangs on bttv driver (2.6.24-rc6)

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


Attachments:
(No filename) (1.49 kB)
bttvregression.c (505.00 B)
Download all attachments

2008-01-05 23:30:58

by Rafael J. Wysocki

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

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

2008-01-06 14:16:29

by Gregor Jasny

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

From: Gregor Jasny <[email protected]>

Fix bttv VIDIOCGMBUF locking like done in commit
820eacd84cff23b76693f4be1e28feb672f4488f.

Signed-off-by: Gregor Jasny <[email protected]>
---
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:

2008-01-06 15:18:13

by Ingo Molnar

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


* Gregor Jasny <[email protected]> wrote:

> From: Gregor Jasny <[email protected]>
>
> 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

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

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 <[email protected]> wrote:
> From: Gregor Jasny <[email protected]>
>
> Fix bttv VIDIOCGMBUF locking like done in commit
> 820eacd84cff23b76693f4be1e28feb672f4488f.
>
> Signed-off-by: Gregor Jasny <[email protected]>
> ---
> 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
> [email protected]
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
>

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

I mean video_ioctl2 not vidioc_ioctl2, sorry.

Cheers,
Douglas

On Jan 6, 2008 8:53 PM, Douglas Landgraf <[email protected]> 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 <[email protected]> wrote:
> > From: Gregor Jasny <[email protected]>
> >
> > Fix bttv VIDIOCGMBUF locking like done in commit
> > 820eacd84cff23b76693f4be1e28feb672f4488f.
> >
> > Signed-off-by: Gregor Jasny <[email protected]>
> > ---
> > 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
> > [email protected]
> > http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer
> >
>

2008-01-06 23:23:21

by Ingo Molnar

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


* Douglas Landgraf <[email protected]> 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

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

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

2008-01-07 09:44:08

by Mauro Carvalho Chehab

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

On Mon, 7 Jan 2008 00:22:34 +0100
Ingo Molnar <[email protected]> wrote:

>
> * Douglas Landgraf <[email protected]> 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