tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Modules loading modules?

On Mon, 2 Aug 2010, Antti Kantee wrote:

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

Yes, the cv mechanism, coupled with changing the semantics of module_autoload() (to not require the caller to obtain the lock) makes good sense here. I'm working on it and I should be able to post new diffs tomorrow.

| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at    |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at |
| Kernel Developer |                          | pgoyette at  |

Home | Main Index | Thread Index | Old Index