Re: [linux-dvb] s2-lipliandvb oops (cx88) - cx88 maintainer ?

2009-01-07 Thread Mauro Carvalho Chehab
On Tue, 06 Jan 2009 18:52:56 -0500
Andy Walls awa...@radix.net wrote:

 Mauro,
 
 Please be aware that I am not happy with my own patch.  The function
 should really make sure everything is OK *before* putting the object on
 the cx8802_devlist.  The failure cases are Oopses waiting to happen:
 the pointer is on the list, but the objects are deallocated in the
 failure cases - not good. :P

Agreed. We should fix it by providing some lock to avoid having udev opening
the device before the end of the complete device initialization.

As I had to patch that file on the same point, instead of adding your
patch, I've added your logic on my patch, and added a comment pointing to your
patch at mail archives.


Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-dvb] s2-lipliandvb oops (cx88) - cx88 maintainer ?

2009-01-07 Thread Mauro Carvalho Chehab
On Tue, 06 Jan 2009 19:33:36 -0500
Andy Walls awa...@radix.net wrote:

 Thanks for the report.  That's actually exactly what I would expect. 
 
 The race I think happens should only happen after the first device is
 added to the cx8802_devlist and while the cx88-dvb module is probing
 devices a second device is being added to the cx8802_devlist with a
 pointer not properly set yet.
 (Of course, I'm not sure why Mauro's recent change didn't work for
 Gregoire.)

Probably because I moved also some code from cx88-mpeg into cx88-dvb. We should
rewrite the locks on the drivers to work better after the KBL unlock patches
that went on 2.6.27 and 2.6.28.


Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-dvb] s2-lipliandvb oops (cx88) - cx88 maintainer ?

2009-01-06 Thread Andy Walls
On Tue, 2009-01-06 at 14:49 -0200, Mauro Carvalho Chehab wrote:
 On Mon, 05 Jan 2009 19:46:40 -0500
 Andy Walls awa...@radix.net wrote:
 
  I you run across the oops often, then the suspected race condition in
  the function I mentioned needs to be fixed.  That may be as simple as
  this lame patch:
 
 Could you please provide you your SOB?

diff -r ce8589c52a7f linux/drivers/media/video/cx88/cx88-mpeg.c
--- a/linux/drivers/media/video/cx88/cx88-mpeg.cTue Jan 06 09:33:46 
2009 -0200
+++ b/linux/drivers/media/video/cx88/cx88-mpeg.cTue Jan 06 17:27:03 
2009 +0100
@@ -830,6 +830,9 @@
err = cx8802_init_common(dev);
if (err != 0)
goto fail_free;
+   /* Maintain a reference so cx88-video can query the 8802 device. */ 
+   core-dvbdev = dev;
+
 
INIT_LIST_HEAD(dev-drvlist);
list_add_tail(dev-devlist,cx8802_devlist);
@@ -851,20 +854,19 @@
__func__);
videobuf_dvb_dealloc_frontends(dev-frontends);
err = -ENOMEM;
+   /* FIXME - need to pull dev off cx8802_devlist*/
goto fail_free;
}
}
}
 #endif
 
-   /* Maintain a reference so cx88-video can query the 8802 device. */
-   core-dvbdev = dev;
-
/* now autoload cx88-dvb or cx88-blackbird */
request_modules(dev);
return 0;
 
  fail_free:
+   /* FIXME - shouldn't we pull dev off the cx8802_devlist - oops */ 
kfree(dev);
  fail_core:
cx88_core_put(core,pci_dev);


Signed-off-by: Andy Walls awa...@radix.net

Mauro,

Please be aware that I am not happy with my own patch.  The function
should really make sure everything is OK *before* putting the object on
the cx8802_devlist.  The failure cases are Oopses waiting to happen:
the pointer is on the list, but the objects are deallocated in the
failure cases - not good. :P


 IMO, the proper fix would be to add some locking at cx88 init. I suspect that
 this breakage (and other similar ones) are tue to the absense of KBL on newer 
 kernels.

Yes, locking somehow would probably be a good idea here.  I haven't
looked at it though.

I have no cx88 based hardware with which to test.

I don't know.  This looks like a race.  New timing caused by a
signifcant amount of kernel changes could have caused it to trip more
frequently.



 Gregoire,
 
 What kernel version are you using?
 
   
fail_free:
  +   /* FIXME - shouldn't we pull dev off the cx8802_devlist - oops */
 
 Better to add here:
   core-dvbdev = NULL;

Sure, but we still have the problem of pointers to deallocated objects
on the cx8802_devlist.

Regards,
Andy

 
 
 
 Cheers,
 Mauro
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-dvb] s2-lipliandvb oops (cx88) - cx88 maintainer ?

2009-01-06 Thread Andy Walls
On Tue, 2009-01-06 at 21:01 +0100, Gregoire Favre wrote:
 On Tue, Jan 06, 2009 at 05:09:26PM -0200, Mauro Carvalho Chehab wrote:
 
 Hello,
 
  I've just commit a patch that should fix this and another reported issue 
  when selecting parts of cx88 code as module and other parts as monolithic. 
  
  Could you please test if the patch also fixed the OOPS and doesn't generate 
  any regression?
 
 OoO not good :
 
 cx88[0]: wm8775' i2c attach [addr=0x1b,client=wm8775']
 cx88[0]/0: registered device video0 [v4l2]
 cx88[0]/0: registered device vbi0
 cx88[0]/0: registered device radio0
 cx8800 :04:05.0: PCI INT A - GSI 20 (level, low) - IRQ 20
 cx88[1]: subsystem: 14f1:0084, board: Geniatech DVB-S [card=52,autodetected], 
 frontend(s): 1
 cx88[1]: TV tuner type 4, Radio tuner type -1
 cx88[1]: i2c register ok
 cx88[1]/0: found at :04:05.0, rev: 3, irq: 20, latency: 64, mmio: 
 0xd900
 cx88[1]/0: registered device video1 [v4l2]
 cx88[1]/0: registered device vbi1
 cx2388x alsa driver version 0.0.6 loaded
 cx88_audio :04:02.1: PCI INT A - GSI 23 (level, low) - IRQ 23
 cx88[0]/1: CX88x/0: ALSA support for cx2388x boards
 cx88/2: cx2388x MPEG-TS Driver Manager version 0.0.6 loaded
 cx88[0]/2: cx2388x 8802 Driver Manager
 cx88-mpeg driver manager :04:02.2: PCI INT A - GSI 23 (level, low) - 
 IRQ 23
 cx88[0]/2: found at :04:02.2, rev: 5, irq: 23, latency: 64, mmio: 
 0xdd00
 cx8802_probe() allocating 2 frontend(s)
 cx88[1]/2: cx2388x 8802 Driver Manager
 cx88-mpeg driver manager :04:05.2: PCI INT A - GSI 20 (level, low) - 
 IRQ 20
 cx88[1]/2: found at :04:05.2, rev: 3, irq: 20, latency: 64, mmio: 
 0xda00
 cx8802_probe() allocating 1 frontend(s)
 cx88/2: cx2388x dvb driver version 0.0.6 loaded
 cx88/2: registering cx8802 driver, type: dvb access: shared
 cx88[0]/2: subsystem: 0070:6902, board: Hauppauge WinTV-HVR4000 
 DVB-S/S2/T/Hybrid [card=68]
 cx88[0]/2-dvb: cx8802_dvb_probe
 cx88[0]/2-dvb:  -being probed by Card=68 Name=cx88[0], PCI 04:02
 BUG: unable to handle kernel NULL pointer dereference at 
 IP: [a090e15a] vp3054_i2c_probe+0x1a/0x160 [cx88_vp3054_i2c]
 PGD bfb9067 PUD bfb8067 PMD 0
 Oops:  [#1] PREEMPT SMP
 last sysfs file: 
 /sys/devices/pci:00/:00:1e.0/:04:05.0/video4linux/vbi1/index
 CPU 1
 Modules linked in: cx88_dvb(+) cx88_vp3054_i2c cx8802 videobuf_dvb cx88_alsa 
 wm8775 tuner_simple tuner_types tda9887 tda8290 tuner cx8800 cx88xx 
 i2c_algo_bit tveeprom v4l2_common videodev v4l1_compat v4l2_compat_ioctl32 
 btcx_risc videobuf_dma_sg videobuf_core stv0299 budget_ci budget_core 
 dvb_core saa7146 ttpci_eeprom ir_common udf ipv6 coretemp w83627ehf w83791d 
 hwmon_vid hwmon nfs lockd sunrpc firewire_ohci firewire_core crc_itu_t 
 nvidia(P) ohci1394 i2c_i801 ieee1394 snd_hda_intel usb_storage [last 
 unloaded: v4l1_compat]
 Pid: 13797, comm: modprobe Tainted: P   2.6.28-gentoo #1
 RIP: 0010:[a090e15a]  [a090e15a] 
 vp3054_i2c_probe+0x1a/0x160 [cx88_vp3054_i2c]
 RSP: 0018:88001f1edd18  EFLAGS: 00010246
 RAX: 0045 RBX:  RCX: 0007
 RDX: 8800a78b1000 RSI: 0046 RDI: 
 RBP: 880037bad000 R08:  R09: 0001
 R10: 88001f1edc68 R11: 807ca320 R12: 
 R13:  R14: 0155d0e0 R15: 0155d0f8
 FS:  7fd0a645f6f0() GS:88017f803780() knlGS:
 CS:  0010 DS:  ES:  CR0: 8005003b
 CR2:  CR3: 1f35c000 CR4: 06e0
 DR0:  DR1:  DR2: 
 DR3:  DR6: 0ff0 DR7: 0400
 Process modprobe (pid: 13797, threadinfo 88001f1ec000, task 
 88017e4cc2c0)
 Stack:
  0070 ffed 880037bad000 
   a0a168d8  
  88002dba7838 88001f1edd78  88007f8e81e0
 Call Trace:
  [a0a168d8] ? cx8802_dvb_probe+0x78/0x1de0 [cx88_dvb]
  [802f3019] ? __sysfs_add_one+0x39/0xb0
  [a0a1038b] ? cx8802_register_driver+0x1cb/0x250 [cx8802]
  [a0a18700] ? dvb_init+0x0/0x30 [cx88_dvb]
  [80209042] ? _stext+0x42/0x1b0
  [802670bc] ? load_module+0x177c/0x19b0
  [80247c90] ? msleep+0x0/0x40
  [802673a5] ? sys_init_module+0xb5/0x1e0
  [8020bbcb] ? system_call_fastpath+0x16/0x1b
 Code: 98 df 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 48 83 ec 28 48 89 5c 
 24 08 48 89 6c 24 10 4c 89 64 24 18 4c 89 6c 24 20 31 db 4c 8b 27 48 89 fd 
 41 83 bc 24 c0 06 00 00 2a 74 1b 89 d8 48 8b
 RIP  [a090e15a] vp3054_i2c_probe+0x1a/0x160 [cx88_vp3054_i2c]
  RSP 88001f1edd18
 CR2: 
 ---[ end trace 00734a6876437fa0 ]---
 
 ksymoops 2.4.11 on x86_64 2.6.28-gentoo.  Options used
  -V (default)
  -k /proc/ksyms (default)
  -l /proc/modules