On Tuesday 16 May 2006 14:54, Johannes Stezenbach wrote: > On Tue, May 16, 2006, Andrew de Quincey wrote: > > On Tuesday 16 May 2006 12:25, Johannes Stezenbach wrote: > > > IOW, chaining ->release() is not the problem, but having > > > ->release() decrement the refcount of some other module is. > > > > Hmm, sorry, not sure what you mean here - where is one module > > decrementing the refcount of another module? Ah, do you mean the > > symbol_put in the current dvb_attach() ? > > Sorry, you published your current idea as a patch against an > ill-conceived, discarded idea. This makes it very hard to read. > http://linuxtv.org/hg/~quincy/v4l-dvb-attach > > For discussion it would be *much* better to have a small patch > against current mainline which just does the core changes plus > one or two example demod driver changes. > > Anyway: > > dvb_unregister_frontend(): > ... > > + while(fe->ops->release) { > + struct module *m = fe->ops->release(fe); > + if (m) > + module_put(m); > + } > + > > - this way of chaining release() looks just sick > (is it even legal C code? can the optimizer not optimize > away the fe->ops->release register reload and turn this > into an endless loop?) Umm, why would it do that? > - the module_put() in there is just as wrong as doing > the module_put() inside release() Fair enough. > Really, the more I think about it, this whole chaining idea > is garbage. Better change lnbp21. OK, so how should we go about doing this. The problem is the lnbp21 and suchlike need to override methods within frontend_ops, yet also need to have their own release() method. What about adding a second release_sec() method pointer to frontend_ops. This permits your dvb_detach() idea using symbol_put_addr(), since we will have a pointer to an address within the LNBP21 module. This also has the advantage it is a minimal change as possible, and doesn't touch anything but the LNBP21/ISL6421 modules... plus it doesn't need chaining. WDYT? _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
