On Tuesday 04 July 2006 03:22, Trent Piepho wrote: > On Tue, 4 Jul 2006, Andrew de Quincey wrote: > > Hi - linuxtv.org seems to have sorted itself, so the tree is at: > > > > http://linuxtv.org/hg/~quincy/v4l-dvb-attach2 > > > > I've added the first attempt at Kconfig modifications.... > > > > The 'dynamic' parameter to dvb_unregister_frontend should be renamed to > > something better... e.g. 'was_dvb_attached' or something. > > My memory of how everything worked is hazy now, but I think there is a bug > in dvb_unregister_frontend(). > > if (fe->ops.release) { > fe->ops.release(fe); > if (dynamic) > symbol_put(fe->ops.release); > } > /* fe is invalid now */ [this comment should be right after release()] > > I seem to recall that after ops.release() is called, the frontend struct is > kfree'd. It's also the same if the symbol_put() call were before the call > to release(). Once the symbol is put, all pointers into the frontend > module are dead since the module could be unloaded. I think that has changed since we first looked at this; pb changed the frontend ops structure so that the ops functions are now always copied into the struct dvb_frontend structure now, so they'll persist even if the module unloads under it. _however_ this doesn't prevent the module from NULLing out the release() field during its unload, so I reckon your idea below is still good as it copes with this. > Also, for symbol_put() > you must use the actual symbol name, symbol_put_addr() is for pointers to > functions. I think that you must do this: > > if (fe->ops.release) { > void *rel = (void*)fe->ops.release; > fe->ops.release(fe); > /* fe is invalid now */ > if (dynamic) > symbol_put_addr(rel); > } > > I think this same thing holds for the tuner_ops.release() call and the > symbol put. I think the release_sec() bit ok, as long as release_sec() > doesn't NULL out ops.release_sec. Ack. I think I would just do it all the same way for consistency. > Note that the if(fe->ops.release) shouldn't be necessary, there was already > a BUG_ON(!fe->ops.release). Ah yeah you're right. Will update. _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
