* [PATCH] tm6000: Prevent Kernel Oops changing channel when stream is still on. @ 2010-05-01 8:51 Bee Hock Goh 2010-05-02 14:42 ` Mauro Carvalho Chehab 2010-05-03 0:08 ` Mauro Carvalho Chehab 0 siblings, 2 replies; 4+ messages in thread From: Bee Hock Goh @ 2010-05-01 8:51 UTC (permalink / raw) To: LMML do a streamoff before setting standard to prevent kernel oops by irq_callback if changing of channel is done while streaming is still on-going. Signed-off-by: Bee Hock Goh <beehock@gmail.com> diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c index c53de47..32f625d 100644 --- a/drivers/staging/tm6000/tm6000-video.c +++ b/drivers/staging/tm6000/tm6000-video.c @@ -1081,8 +1086,8 @@ static int vidioc_s_std (struct file *file, void *priv, v4l2_std_id *norm) struct tm6000_fh *fh=priv; struct tm6000_core *dev = fh->dev; + vidioc_streamoff(file, priv, V4L2_BUF_TYPE_VIDEO_CAPTURE); rc=tm6000_set_standard (dev, norm); - fh->width = dev->width; fh->height = dev->height; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tm6000: Prevent Kernel Oops changing channel when stream is still on. 2010-05-01 8:51 [PATCH] tm6000: Prevent Kernel Oops changing channel when stream is still on Bee Hock Goh @ 2010-05-02 14:42 ` Mauro Carvalho Chehab 2010-05-02 17:26 ` Mauro Carvalho Chehab 2010-05-03 0:08 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 4+ messages in thread From: Mauro Carvalho Chehab @ 2010-05-02 14:42 UTC (permalink / raw) To: Bee Hock Goh; +Cc: LMML Hi Bee, Bee Hock Goh wrote: > do a streamoff before setting standard to prevent kernel oops by > irq_callback if changing of channel is done while streaming is still > on-going. > > > Signed-off-by: Bee Hock Goh <beehock@gmail.com> > > diff --git a/drivers/staging/tm6000/tm6000-video.c > b/drivers/staging/tm6000/tm6000-video.c > index c53de47..32f625d 100644 > --- a/drivers/staging/tm6000/tm6000-video.c > +++ b/drivers/staging/tm6000/tm6000-video.c > > @@ -1081,8 +1086,8 @@ static int vidioc_s_std (struct file *file, void > *priv, v4l2_std_id *norm) > struct tm6000_fh *fh=priv; > struct tm6000_core *dev = fh->dev; > > + vidioc_streamoff(file, priv, V4L2_BUF_TYPE_VIDEO_CAPTURE); > rc=tm6000_set_standard (dev, norm); > - > fh->width = dev->width; > fh->height = dev->height; This doesn't seem to be the right thing to do. The problem here is that changing a video standard takes a long time to happen. As calling an ioctl is protected by KBL, QBUF/DQBUF won't be called, so, the driver will run out of the buffers, and *buf will become null. This can eventually happen during copy_streams(). --- tm6000: Fix a panic if buffer become NULL Changing a video standard takes a long time to happen on tm6000, since it needs to load another firmware, and the i2c implementation on this device is really slow. As calling an ioctl is protected by KBL, QBUF/DQBUF won't be called, so, the driver will run out of the buffers, and *buf will become NULL. This can eventually happen during copy_streams(). The fix is to leave the URB copy loop, if there's no more buffers available. Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> diff --git a/linux/drivers/staging/tm6000/tm6000-video.c b/linux/drivers/staging/tm6000/tm6000-video.c --- a/linux/drivers/staging/tm6000/tm6000-video.c +++ b/linux/drivers/staging/tm6000/tm6000-video.c @@ -539,7 +539,7 @@ static inline int tm6000_isoc_copy(struc } } copied += len; - if (copied>=size) + if (copied >= size || !buf) break; // } } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tm6000: Prevent Kernel Oops changing channel when stream is still on. 2010-05-02 14:42 ` Mauro Carvalho Chehab @ 2010-05-02 17:26 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 4+ messages in thread From: Mauro Carvalho Chehab @ 2010-05-02 17:26 UTC (permalink / raw) To: Bee Hock Goh; +Cc: LMML Mauro Carvalho Chehab wrote: > Hi Bee, > > Bee Hock Goh wrote: >> do a streamoff before setting standard to prevent kernel oops by >> irq_callback if changing of channel is done while streaming is still >> on-going. > > This doesn't seem to be the right thing to do. The problem here is that > changing a video standard takes a long time to happen. As calling an > ioctl is protected by KBL, QBUF/DQBUF won't be called, so, the driver > will run out of the buffers, and *buf will become null. This can eventually > happen during copy_streams(). > > --- > > tm6000: Fix a panic if buffer become NULL > > Changing a video standard takes a long time to happen on tm6000, since it > needs to load another firmware, and the i2c implementation on this device > is really slow. As calling an ioctl is protected by KBL, QBUF/DQBUF won't > be called, so, the driver will run out of the buffers, and *buf will become > NULL. This can eventually happen during copy_streams(). The fix is to leave > the URB copy loop, if there's no more buffers available. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> > > diff --git a/linux/drivers/staging/tm6000/tm6000-video.c b/linux/drivers/staging/tm6000/tm6000-video.c Sorry, I sent the wrong one. That's the proper fix. I've also improved the comments to better express what's happening. Tested here with HVR-900H and the Panic disappeared. --- tm6000: Fix a panic if buffer become NULL Changing a video standard takes a long time to happen on tm6000, since it needs to load another firmware, and the i2c implementation on this device is really slow. When the driver tries to change the video standard, a kernel panic is produced: BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 IP: [<ffffffffa0c7b48a>] tm6000_irq_callback+0x57f/0xac2 [tm6000] ... Kernel panic - not syncing: Fatal exception in interrupt By inspecting it with gdb: (gdb) list *tm6000_irq_callback+0x57f 0x348a is in tm6000_irq_callback (drivers/staging/tm6000/tm6000-video.c:202). 197 /* FIXME: move to tm6000-isoc */ 198 static int last_line = -2, start_line = -2, last_field = -2; 199 200 /* FIXME: this is the hardcoded window size 201 */ 202 unsigned int linewidth = (*buf)->vb.width << 1; 203 204 if (!dev->isoc_ctl.cmd) { 205 c = (header >> 24) & 0xff; 206 Clearly, it was the trial to access *buf, at line 202 that caused the Panic. As ioctl is serialized, While S_STD is handled,QBUF/DQBUF won't be called. So, the driver will run out of the buffers, and *buf will become NULL. As, on tm6000, the same URB can contain more than one video buffer, it is likely to hit a condition where no new buffer is available whily copying the streams. The fix is to leave the URB copy loop, if there's no more buffers are available. The same bug could also be produced by an application that is not fast enough to request new video buffers. The same bug were reported by Bee Hock Goh <beehock@gmail.com>. Thanks-to: Bee Hock Goh <beehock@gmail.com> for reporting the bug Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> Index: work.x86-64/drivers/staging/tm6000/tm6000-video.c =================================================================== --- work.x86-64.orig/drivers/staging/tm6000/tm6000-video.c +++ work.x86-64/drivers/staging/tm6000/tm6000-video.c @@ -395,6 +395,8 @@ HEADER: jiffies); return rc; } + if (!*buf) + return 0; } return 0; @@ -528,7 +530,7 @@ static inline int tm6000_isoc_copy(struc } } copied += len; - if (copied>=size) + if (copied >= size || !buf) break; // } } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tm6000: Prevent Kernel Oops changing channel when stream is still on. 2010-05-01 8:51 [PATCH] tm6000: Prevent Kernel Oops changing channel when stream is still on Bee Hock Goh 2010-05-02 14:42 ` Mauro Carvalho Chehab @ 2010-05-03 0:08 ` Mauro Carvalho Chehab 1 sibling, 0 replies; 4+ messages in thread From: Mauro Carvalho Chehab @ 2010-05-03 0:08 UTC (permalink / raw) To: Bee Hock Goh; +Cc: LMML, stefan Ringel The two patches fixed the OOPS I was having. The big problem I'm still suffering with HVR-900H is that tm6000 insists on dying: hub 1-0:1.0: port 8 disabled by hub (EMI?), re-enabling... usb 1-8: USB disconnect, address 5 tm6000 tm6000_irq_callback :urb resubmit failed (error=-19) tm6000 tm6000_irq_callback :urb resubmit failed (error=-19) tm6000 tm6000_irq_callback :urb resubmit failed (error=-19) tm6000 tm6000_irq_callback :urb resubmit failed (error=-19) tm6000 tm6000_irq_callback :urb resubmit failed (error=-19) tm6000: disconnecting tm6000 #2 xc2028 2-0061: destroying instance As the chipset stops answering USB, a new, non-fatal bug hits: ------------[ cut here ]------------ WARNING: at lib/list_debug.c:48 list_del+0x30/0x87() Hardware name: list_del corruption. prev->next should be ffff88003df65ec0, but was (null) Modules linked in: ir_sony_decoder ir_jvc_decoder ir_rc6_decoder ir_rc5_decoder ir_nec_decoder ir_core tm6000(C) v4l2_common videodev v4l1_compat v4l2_compat_ioctl32 videobuf_vmalloc videobuf_core autofs4 hidp rfcomm l2cap crc16 bluetooth iptable_filter ip_tables ip6t_REJECT xt_tcpudp ip6table_filter ip6_tables x_tables ipv6 powernow_k8 dm_multipath scsi_dh sbs sbshc battery acpi_memhotplug ac lp snd_intel8x0 snd_ac97_codec ac97_bus snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device i2c_algo_bit snd_pcm_oss snd_mixer_oss sg snd_pcm nvidia(P) ide_cd_mod snd_timer serio_raw parport_pc cdrom snd button parport floppy i2c_nforce2 k8temp soundcore snd_page_alloc pcspkr shpchp hwmon forcedeth i2c_core dm_snapshot dm_zero dm_mirror dm_region_hash dm_log dm_mod sata_nv libata sd_mod scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: tuner_xc2028] Pid: 12402, comm: mplayer Tainted: P WC 2.6.33 #4 Call Trace: [<ffffffff81179c24>] ? list_del+0x30/0x87 [<ffffffff8103a43f>] ? warn_slowpath_common+0x77/0x8e [<ffffffff8103a4b2>] ? warn_slowpath_fmt+0x51/0x59 [<ffffffff810c54a5>] ? free_block+0xdf/0xfe [<ffffffff8102c9bc>] ? __wake_up_sync_key+0x3a/0x56 [<ffffffff81179c24>] ? list_del+0x30/0x87 [<ffffffffa015e9c8>] ? videobuf_queue_cancel+0x48/0xba [videobuf_core] [<ffffffffa013e2b3>] ? videobuf_vm_close+0x80/0x14d [videobuf_vmalloc] [<ffffffff810b23d1>] ? remove_vma+0x2c/0x72 [<ffffffff810b2522>] ? exit_mmap+0x10b/0x129 [<ffffffff8103813c>] ? mmput+0x34/0xa2 [<ffffffff8103c077>] ? exit_mm+0x109/0x114 [<ffffffff8103d2b0>] ? do_exit+0x1de/0x66e [<ffffffff8103d7ad>] ? do_group_exit+0x6d/0x97 [<ffffffff8103d7e9>] ? sys_exit_group+0x12/0x16 [<ffffffff810028ab>] ? system_call_fastpath+0x16/0x1b ---[ end trace fed27d3fe75cb89b ]--- ------------[ cut here ]------------ WARNING: at lib/list_debug.c:51 list_del+0x5c/0x87() Hardware name: list_del corruption. next->prev should be ffff88003df65bc0, but was (null) Modules linked in: ir_sony_decoder ir_jvc_decoder ir_rc6_decoder ir_rc5_decoder ir_nec_decoder ir_core tm6000(C) v4l2_common videodev v4l1_compat v4l2_compat_ioctl32 videobuf_vmalloc videobuf_core autofs4 hidp rfcomm l2cap crc16 bluetooth iptable_filter ip_tables ip6t_REJECT xt_tcpudp ip6table_filter ip6_tables x_tables ipv6 powernow_k8 dm_multipath scsi_dh sbs sbshc battery acpi_memhotplug ac lp snd_intel8x0 snd_ac97_codec ac97_bus snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device i2c_algo_bit snd_pcm_oss snd_mixer_oss sg snd_pcm nvidia(P) ide_cd_mod snd_timer serio_raw parport_pc cdrom snd button parport floppy i2c_nforce2 k8temp soundcore snd_page_alloc pcspkr shpchp hwmon forcedeth i2c_core dm_snapshot dm_zero dm_mirror dm_region_hash dm_log dm_mod sata_nv libata sd_mod scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: tuner_xc2028] Pid: 12402, comm: mplayer Tainted: P WC 2.6.33 #4 Call Trace: [<ffffffff81179c50>] ? list_del+0x5c/0x87 [<ffffffff8103a43f>] ? warn_slowpath_common+0x77/0x8e [<ffffffff8103a4b2>] ? warn_slowpath_fmt+0x51/0x59 [<ffffffff810c54a5>] ? free_block+0xdf/0xfe [<ffffffff81035d58>] ? __wake_up+0x30/0x44 [<ffffffff81179c50>] ? list_del+0x5c/0x87 [<ffffffffa015e9c8>] ? videobuf_queue_cancel+0x48/0xba [videobuf_core] [<ffffffffa013e2b3>] ? videobuf_vm_close+0x80/0x14d [videobuf_vmalloc] [<ffffffff810b23d1>] ? remove_vma+0x2c/0x72 [<ffffffff810b2522>] ? exit_mmap+0x10b/0x129 [<ffffffff8103813c>] ? mmput+0x34/0xa2 [<ffffffff8103c077>] ? exit_mm+0x109/0x114 [<ffffffff8103d2b0>] ? do_exit+0x1de/0x66e [<ffffffff8103d7ad>] ? do_group_exit+0x6d/0x97 [<ffffffff8103d7e9>] ? sys_exit_group+0x12/0x16 [<ffffffff810028ab>] ? system_call_fastpath+0x16/0x1b ---[ end trace fed27d3fe75cb89c ]--- usbcore: deregistering interface driver tm6000 Cheers, Mauro -- Cheers, Mauro ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-05-03 0:08 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-05-01 8:51 [PATCH] tm6000: Prevent Kernel Oops changing channel when stream is still on Bee Hock Goh 2010-05-02 14:42 ` Mauro Carvalho Chehab 2010-05-02 17:26 ` Mauro Carvalho Chehab 2010-05-03 0:08 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox