Re: CVS commit: src/sys

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Hello,

> Module Name:    src
> Committed By:   dyoung
> Date:           Thu Nov 12 19:10:31 UTC 2009
> 
> Modified Files:
>         src/sys/kern: subr_autoconf.c
>         src/sys/sys: device.h
> 
> Log Message:
> Move a device-deactivation pattern that is replicated throughout
> the system into config_deactivate(dev): deactivate dev and all of
> its descendants.  Block all interrupts while calling each device's
> activation hook, ca_activate.  Now it is possible to simplify or
> to delete several device-activation hooks throughout the system.
>
> ...
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.186 -r1.187 src/sys/kern/subr_autoconf.c
> cvs rdiff -u -r1.124 -r1.125 src/sys/sys/device.h

- Can you tell what relevant code requires alldevs_mtx to be at IPL_HIGH?

- Since alldevs_mtx became a spin-lock, it is no longer valid to assert for
  mutex _not_ being held.  In other words, such cases are wrong:

        KASSERT(!mutex_owned(&alldevs_mtx));

- You have added config_collect_garbage(), which is mostly called before
  config_alldevs_lock().  How about changing it to be used as/with last
  unlock?  That is, collect the objects into a list, release the lock and
  then destroy all objects.  Apart from avoiding unecessary unlock/relock
  dances, it would also be simpler.

- May I suggest to avoid "inverted" logic like this:

        if (rv != 0)
                ;
        else if (dev->dv_del_gen != 0)
                ;
        else {
                dev->dv_del_gen = alldevs_gen;
                alldevs_garbage = true;
        }

Thanks.

-- 
Mindaugas