Re: Modules loading modules?

[Antti Kantee <pooka%cs.hut.fi@localhost>]

On Mon Aug 02 2010 at 16:30:03 +1000, matthew green wrote:
> this is an incomplete reading of the manual page, and you can not
> use mutex_owned() the way you are trying to (regardless of what
> pooka posted.) you can't even using it in various forms of assertions
> safely.  from the man page:
> 
>       It should not be used to make locking decisions at run time, or to
>       verify that a lock is not held.

That's the mantra, yes.

> ie, you can not even KASSERT(!mutex_owned()).

Strictly speaking you can in a case where you have two locks always
are taken as l1,l2 and released as l2,l1 provided you're not dealing
with a spin mutex.  Does it make any sense?  no (l2 is useless).
Is it possible?  yes.

Now, on to sensible stuff.  I'm quite certain that warning was written
to make people avoid writing bad code without understanding locking --
if you need to used mutex_owned() to decide if you should lock a normal
mutex, your code is broken.  The other less likely possibility is that
someone plans to change mutex_owned in the future.

Further data point: the same warning is in rw_held, yet it was used to
implement recursive locking in vlockmgr until its very recent demise.

Ignoring man page mantras and focusing on how the code works, I do not
see anything wrong with Paul's use of mutex_owned().

> but i'm still not sure why we're going to such lengths to hold a
> lock across such a heavy weight operation like loading a module.
> that may involve disk speeds, and so you're looking at waiting
> millions of cycles for the lock.  aka, what eeh posted.

It's held for millions of cycles already now and nobody has pointed out
measurable problems.  But if it is deemed necessary, you can certainly
hide a cv underneath.  The efficiency requirements for the module lock
are probably anyway in the "who cares wins ... not" spectrum.  At least
I'm not aware of any fastpath wanting to use it.

Anyway, no real opinion there.  A cv most likely is the safe, no-brainer
choice.