On Monday 19 June 2006 14:24, Ian Dowse wrote: > In message <[EMAIL PROTECTED]>, John Baldwin writes: > >On Friday 16 June 2006 20:47, Ian Dowse wrote: > >> - driver calls firmware_get, firmware image loaded and fp->file set to non-NULL > >> - manually kldload some_module_that_depends_on_firmware_image > >> - driver calls firmware_put, unloadentry called and sets fp->file = NULL > > > >In practice no modules depend on firmware modules. :) I think we should > >take the approach of not clearing fp->file in unloadentry() however. > >That would result in correct behavior in every case I can think of (or as > >close to correct as you can get). In the above case the > >linker_file_unload() would have fail leaving the firmware module around. > > How about the following patch? In the above case linker_file_unload() > would actually succeed because the linker file reference count is > just dropping from 2 to 1, so we'd get a negative reference count > later if we kept fp->file non-NULL after it returns. > > Ian > > Index: subr_firmware.c > =================================================================== > RCS file: /home/ncvs/src/sys/kern/subr_firmware.c,v > retrieving revision 1.2 > diff -u -r1.2 subr_firmware.c > --- subr_firmware.c 10 Jun 2006 17:04:07 -0000 1.2 > +++ subr_firmware.c 19 Jun 2006 18:15:21 -0000 > @@ -206,7 +206,7 @@ > { > struct firmware *fp; > linker_file_t file; > - int i; > + int i, err; > > mtx_lock(&firmware_mtx); > for (;;) { > @@ -226,9 +226,18 @@ > fp->file = NULL; > mtx_unlock(&firmware_mtx); > > - linker_file_unload(file, LINKER_UNLOAD_NORMAL); > - > + err = linker_file_unload(file, LINKER_UNLOAD_NORMAL); > mtx_lock(&firmware_mtx); > + if (err) { > + /* > + * If linker_file_unload() failed then we still hold > + * a reference on the module so it should not be > + * possible for it to go away or be re-registered. > + */ > + KASSERT(fp->file == NULL, > + ("firmware entry reused while referenced!")); > + fp->file = file; > + } > } > mtx_unlock(&firmware_mtx); > } >
Ok. -- John Baldwin _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "[EMAIL PROTECTED]"