Re: Modules loading modules?

[Paul Goyette <paul%whooppee.com@localhost>]

At last, a response that doesn't just say "you can't do that" but 
instead provides a crystal-clear alternative.   :)

I like this approach since it avoids the need to implement the new 
"recursive mutex" type (which leads to the tempting-but-forbidden 
mis-use of mutex_owned()).  It also would appear to alleviate eeh's 
and mrg's concerns for holding the mutex across long, blocking 
operations.

I still think it is preferable to move all locking associated with 
module_autoload() into the routine itself as described in the other 
thread (with subject line "Locking for module_autoload()").  Then it 
will be nearly trivial to replace the current locking with your 
suggested replacement.


On Mon, 2 Aug 2010, Adam Hamsik wrote:

> On Aug,Sunday 1 2010, at 3:53 PM, Antti Kantee wrote:
>
> > On Sun Aug 01 2010 at 06:10:07 -0700, Paul Goyette wrote:
> > > One question:  Since an adaptive kmutex_t already includes an owner
> > > field, would we really need to have another copy of it in the rmutex_t
> > > structure?
> >
> > Good point.  I think it's ok to do:
> >
> >  if (mutex_owned(mtx))
> >    cnt++
> >  else
> >    lock
>
> When I did locking for dm driver, it was said by rmind@ that mutex_owned should 
> not be used for any locking decisions and should be used only for diagnostic 
> purposes. What I would in this case is add two new routines to module_framework 
> ->
>
> module_lock()
> module_unlock()
>
> These two will then do something like this.
>
> kmutex_t module_lock;
> uint32_t refcnt;
> lwp_t mod_lwp;
> condvar_t cv;
>
>
> module_lock() {
>
> mutex_enter(module_lock);
> start
> if (refcnt == 0) {
>         refcnt++;
>         mod_lwp = curlwp;
> else {
>         if (mod_lwp == curlwp)
>                 refcnt++;
>         else {
>                 cv_wait(cv);
>                 goto start
>         }
> }
>
> mutex_exit(module_lock);
> }
>
>
> mudule_unlock() {
>  mutex_enter(module_lock);
>         KASSERT(mod_lwp != curlwp); /* module_unlock should be only called from 
> lwp which called module_lock */
>
>         refcnt--;
>
>         if (refcnt == 0) {
>                 mod_lwp = 0;
>                 cv_broadcast(cv);
>         }
>
>  mutex_exit(module_lock);
> }
>
> This way module_lock can be called multiple times by one lwp and will sleep for 
> everyone else.
>
> Regards
>
> Adam.

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