On Tuesday 04 July 2006 10:29, Andrew de Quincey wrote: > 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. OK, updates in http://linuxtv.org/hg/~quincy/v4l-dvb-attach _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
