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);
 }
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to