On Tue, Sep 22, 2009 at 5:06 AM, David Ellingsworth <david@xxxxxxxxxxxxxxxxx> wrote: > On Mon, Sep 21, 2009 at 8:03 PM, Alexey Klimov <klimov.linux@xxxxxxxxx> wrote: >> Hello, David >> >> On Mon, Sep 14, 2009 at 11:09 PM, Alexey Klimov <klimov.linux@xxxxxxxxx> wrote: >>> Hello David, >>> >>> On Sun, Sep 13, 2009 at 7:22 AM, David Ellingsworth >>> <david@xxxxxxxxxxxxxxxxx> wrote: >>>> What follow is a series of patches to clean up the radio-mr800 driver. I >>>> do _not_ have access to this device so these patches need to be tested. >>>> These patches should apply to Mauro's git tree and against the 2.6.31 >>>> release kernel. The patches in this series are as follows: >>>> >>>> 01. radio-mr800: implement proper locking >>>> 02. radio-mr800: simplify video_device allocation >>>> 03. radio-mr800: simplify error paths in usb probe callback >>>> 04. radio-mr800: remove an unnecessary local variable >>>> 05. radio-mr800: simplify access to amradio_device >>>> 06. radio-mr800: simplify locking in ioctl callbacks >>>> 07. radio-mr800: remove device-removed indicator >>>> 08. radio-mr800: fix potential use after free >>>> 09. radio-mr800: remove device initialization from open/close >>>> 10. radio-mr800: ensure the radio is initialized to a consistent state >>>> 11. radio-mr800: fix behavior of set_radio function >>>> 12. radio-mr800: preserve radio state during suspend/resume >>>> 13. radio-mr800: simplify device warnings >>>> 14. radio-mr800: set radio frequency only upon success >>>> >>>> The first 7 in this series are the same as those submitted in my last series >>>> and will not be resent. The remaining 7 patches in this series will be sent >>>> separately for review. >> >> I applied your patches and tested radio device. Radio works fine, but >> unfortunately sudden disconnect while playing radio with gnomeradio >> creates troubles. I see such oops in dmesg: >> >> radio_mr800: version 0.11-david AverMedia MR 800 USB FM radio driver >> usb 2-2: new low speed USB device using ohci_hcd and address 16 >> usb 2-2: New USB device found, idVendor=07ca, idProduct=b800 >> usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 >> usb 2-2: Product: AVerMedia USB Radio >> usb 2-2: Manufacturer: AVerMedia Technologies >> usb 2-2: configuration #1 chosen from 1 choice >> radio-mr800 2-2:1.0: Non-NULL drvdata on register >> usb 2-2: USB disconnect, address 16 >> BUG: unable to handle kernel NULL pointer dereference at 000000000000009f >> IP: [<ffffffff8119222b>] dev_set_drvdata+0x25/0x30 >> PGD 3c0e1067 PUD 33b1d067 PMD 0 >> Oops: 0002 [#1] SMP >> last sysfs file: /sys/devices/pci0000:00/0000:00:02.0/usb2/2-2/2-2:1.0/uevent >> CPU 1 >> Modules linked in: radio_mr800 v4l2_common videodev v4l1_compat >> v4l2_compat_ioctl32 nls_iso8859_1 nls_cp437 vfat fat usb_storage >> nls_utf8 cifs ext2 ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 >> nf_conntrack nf_defrag_ipv4 ip_tables x_tables ppp_async crc_ccitt >> ppp_generic slhc cpufreq_powersave powernow_k8 freq_table >> snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device >> snd_pcm_oss snd_mixer_oss reiserfs usbhid hid snd_hda_codec_analog >> snd_hda_intel ohci_hcd ehci_hcd snd_hda_codec snd_hwdep snd_pcm >> snd_timer nvidia(P) snd usbcore soundcore snd_page_alloc rtc_cmos sg >> rtc_core rtc_lib i2c_nforce2 forcedeth e100 nls_base k8temp mii >> i2c_core hwmon thermal button [last unloaded: v4l2_compat_ioctl32] >> Pid: 11790, comm: gnomeradio Tainted: P 2.6.31 #12 System Product Name >> RIP: 0010:[<ffffffff8119222b>] [<ffffffff8119222b>] dev_set_drvdata+0x25/0x30 >> RSP: 0018:ffff880031741e28 EFLAGS: 00010206 >> RAX: 0000000000000017 RBX: ffff88003c3a4030 RCX: ffffffff8109458a >> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88003c3a4030 >> RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 >> R10: ffff880029391e70 R11: 0000000000000246 R12: ffffffff81349ec0 >> R13: ffff880029391e70 R14: ffff8800173d7000 R15: ffff88003fac8300 >> FS: 00007fa0f88e3750(0000) GS:ffff880002900000(0000) knlGS:00000000f7327a10 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 000000000000009f CR3: 000000003165c000 CR4: 00000000000006e0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 >> Process gnomeradio (pid: 11790, threadinfo ffff880031740000, task >> ffff88003bcc5820) >> Stack: >> ffff8800173d7000 ffff88003c3a5a60 ffff88003c3a5a60 ffffffffa0c603cb >> <0> ffff88003c3a5800 ffffffffa0c603e5 ffff88003c3a5800 ffff88003e400890 >> <0> ffffffff81349ec0 ffffffffa000d5c7 ffff8800331593c0 ffffffff8118fb37 >> Call Trace: >> [<ffffffffa0c603cb>] ? v4l2_device_disconnect+0x13/0x1c [videodev] >> [<ffffffffa0c603e5>] ? v4l2_device_unregister+0x11/0x4b [videodev] >> [<ffffffffa000d5c7>] ? usb_amradio_video_device_release+0x11/0x26 [radio_mr800] >> [<ffffffff8118fb37>] ? device_release+0x41/0x6a >> [<ffffffff81118bf3>] ? kobject_release+0x48/0x5e >> [<ffffffff81118bab>] ? kobject_release+0x0/0x5e >> [<ffffffff811198ad>] ? kref_put+0x41/0x4a >> [<ffffffffa0c5c2f3>] ? v4l2_release+0x33/0x37 [videodev] >> [<ffffffff81092dfd>] ? __fput+0x100/0x1c9 >> [<ffffffff81090538>] ? filp_close+0x5f/0x6a >> [<ffffffff810905d4>] ? sys_close+0x91/0xc4 >> [<ffffffff8100ad6b>] ? system_call_fastpath+0x16/0x1b >> Code: 00 00 c3 31 c0 c3 55 48 89 f5 53 48 89 fb 48 83 ec 08 48 85 ff >> 74 1b 48 83 7f 08 00 75 09 e8 45 de ff ff 85 c0 75 0b 48 8b 43 08 <48> >> 89 a8 88 00 00 00 58 5b 5d c3 53 48 8b 87 80 00 00 00 48 89 >> RIP [<ffffffff8119222b>] dev_set_drvdata+0x25/0x30 >> RSP <ffff880031741e28> >> CR2: 000000000000009f >> ---[ end trace 897762e94cb738ad ]--- >> >> and lsmod shows that radio-mr800 module is in use. The problem doesn't >> exist with current driver. >> I'm trying to find patch that provides such behaviour and for this >> moment i can say that first six patches(among 14) don't broke the >> driver. >> >> >> -- >> Best regards, Klimov Alexey >> > > I think I see the problem.. I think it's with #8 of the series.. > instead of removing that line, place it before the call to > video_unregister_device. It must not be placed after since > video_unregister_device may cause the entire structure to be freed. My > reasoning behind removing it was due to the fact that > video_unregister_device can cause usb_amradio_video_device_release to > be called which in turn calls v4l2_device_disconnect. > > Regards, > > David Ellingsworth Hello David, looks like [RFC/RFT 08/14] Version 2 makes things right :) Driver is living okay on sudden disconnect now. Thank you for your work. I have good feelings about this patchset, so if there is no objections.. Mauro, Douglas, you can use my ack for this patchset: Acked-by: Alexey Klimov <klimov.linux@xxxxxxxxx> -- Best regards, Klimov Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
