On Thursday 15 June 2006 20:49, Ian Dowse wrote: > In message <[EMAIL PROTECTED]>, John Baldwin writes: > >On Monday 12 June 2006 20:50, Ian Dowse wrote: > >> That would bring back the original issue where a manually kldloaded > >> firmware image would be removed from the list when a driver calls > >> firmware_put(), even though the kld will remain loaded; there is > >> nothing that a driver can do to get the entry back on the list since > >> calling linker_reference_module() will not result in a call to > >> firmware_register() because the module is already (manually) loaded. > > > >No it wouldn't. :) unloadentry() is only called when we are unloading > >an explicitly loaded module from the taskqueue. That is where I think > >the 'fp->file = NULL' should be changed to 'clearentry()'. Either > >that or don't clear fp->file at all. > > Sorry for the delay in replying. True, but clearing the entry is > still assuming that there are no other linker references, and > firmware(9) doesn't really know this. For example if you manually > load a module that depends on a firmware image module, then the > firmware image module will stay loaded after unloadentry() calls > linker_file_unload(), so clearing the entry then would cause future > firmware_get() calls to fail.
But unloadentry() would never unload such a module because fp->file is NULL. unloadentry() would only call clearentry() and then linker_file_unload() on an explicitly loaded firmware module. > >No. You've cleared fp->file. This means that if the other thread gets > >a reference, the firmware_unregister() will fail, but now the kernel will > >never unload this file on a subsequent firmware_put() since it won't see > >that it was explicitly loaded by the kernel since fp->file == NULL. The > >awk script patch fixes a different race where kldunload would succeed > >even though there were open references and drivers would have pointers > >into unmapped memory (or possibly mapped to something else). > > In theory whatever code was attempting to unload the module will > get back an error return and will keep its original reference on > the linker file, so that same code can succeed if it tries to unload > again later. In practice I'm sure there are bugs here ;-) For > example, firmware(9) might want to attempt to avoid clearing fp->file > if its call to linker_file_unload() fails. The problem is that in unloadentry() you are the code trying to unload the module and you ignore the error return. :) I think actually though, the best fix is to just not clear fp->file at all. If the unload succeeds, then the clearentry() in firmware_unregister() will end up clearing fp->file. If the unload fails because another thread grabbed a reference, then fp->file will remain set so that later on when that other thread does a firmware_put() we will try to unload the module in unloadentry() again. -- John Baldwin _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "[EMAIL PROTECTED]"
