On Tue, May 16, 2006, Andrew de Quincey wrote: > On Sunday 14 May 2006 11:38, Johannes Stezenbach wrote: > > On Sat, May 13, 2006, Oliver Endriss wrote: > > > > > > Is chaining used anywhere else than in the lnbp21 driver? > > > > > > I introduced it because I thought that it would be a good idea to limit > > > the number of private pointers in struct dvb_frontend. ;-) > > > > > > Obviously it makes life harder, not easier. So just remove chaining from > > > lnbp21.[ch], and rename misc_priv to something like sec_priv... > > > > > > Imho it's not worth the trouble. ;-) > > > > IMHO this "chaining" of the release function is a bad > > concept. The card driver does the attach, so it should also > > do the release. > > > > Consider: > > foo = foo_attach(); > > bar = bar_attach(foo); > > ... > > bar->release(); > > // do we leak foo now is it magically cleaned up? > > > > Please keep it simple and stupid, the four drivers which > > use lnbp21 can also keep a lnbp21 pointer around and > > release it explicitly. > > Hmm, well we could make a rule that you never chain the release() function, > even though you are allowed to do so for the other function pointers... would > this be ok? Then we would always have a pointer to each used module, and > hence using symbol_put_addr() would be fine. IMHO it is the card driver which creates an instance of the demod/pll/lnbp etc. driver, so it *owns* the reference. The chaining of the fe->ops function pointers is an implementation detail, it could e.g. also be implemented by keeping one fe->ops per driver and have dvb_frontend call them in order. So there is a conceptual difference between chaining fe->ops and passing ownership to some other module. IOW, chaining ->release() is not the problem, but having ->release() decrement the refcount of some other module is. > As for symbol_put_addr - well we've had absolutely zero response to the > patches - both you and Trent have tried to get someone to look at 'em on > LKML, and there's been complete silence... did we have a compat.h patch so > that we can experiment using symbol_put_addr() in the meantime? If one of you > wants to send it, I'll update the repos to use it and remove the chaining so > we can see what that looks like. Trents patch is already in mainline git. Johannes _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
