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]"

Reply via email to