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

[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 <awalls at 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.c        Tue Jan 06 09:33:46 2009 -0200
+++ b/linux/drivers/media/video/cx88/cx88-mpeg.c        Tue 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 <awalls at 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 "Oops"es 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 majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>