Re: CVS commit: src/sys/kern

[David Young <dyoung%pobox.com@localhost>]

On Sat, Jan 09, 2010 at 01:38:04PM +0000, Mindaugas Rasiukevicius wrote:
> David Young <dyoung%pobox.com@localhost> wrote:
> > Previously, the code distinguished garbage collection, locking
> > acquisition, and resource-reclamation from each other by using
> > more, and more meaningful, subroutine names.  Also, in the common
> > case, it collected and reclaimed garbage immediately at the end of
> > config_detach(9) instead of at some arbitrary time in the future (for
> > example, in the next call to device_lookup()).
> > 
> > Your change obfuscates the code and needlessly postpones a lot of
> > garbage collection.  Please back it out.
> 
> Your code had local a list of devices to G/C - it is redundant.

Now there is a global list, and there is still a local list of devices
to garbage collect.  That is not more redundant?

> Having
> resource freeing/destroying tied with last unlock operation will make sure
> that it wont be done with locks held - that is a common way.  Perhaps
> names "lock/unlock" confuse you?  It can be "enter/exit" or similar.

The names are no longer descriptive.  "Enter" and "exit" are vague.

> Can you explain "arbitrary time in the future" and "needlessly postpones"?
> It does not change the previous behaviour.

You changed the previous behavior in two ways:

config_detach: used to collect the garbage it just created, in the
    common case where no other thread was in the device tree.  Now it
    does not.

device_lookup: collects garbage where previously it did not.

Back the changes out, and let's discuss how to achieve your purposes
with a smaller change.

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 278-3933