Re: Module and device configuration locking [was Re: Modules loading modules?]

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

On Wed, 4 Aug 2010, Andrew Doran wrote:

> Sorry for not replying sooner.
>
> Please don't add a generic recursive lock facility, it would be abused.

Yeah!  My current patches have no new generic facilities at all.

> I'd like something roughly like the following.  I think this should
> also cover major configuration tasks such as device attach and detach,
> so it wouldn't really be module specific.  The naming is a poor suggestion.
>
> void
> sysconfig_lock(void)
> {
>
>         if (mutex_owned(&sysconfig_mutex)) {
>                 /*
>                  * At this point it's OK for the thread to hold other
>                  * locks, since the system configuration lock was the
>                  * first taken on the way in.
>                  */
>                 sysconfig_recurse++;
>                 return;
>         }
>         /*
>          * I don't remember the correct argument sequence to the
>          * following, but basically we need to assert that NO locks,
>          * sleep or spin, other than kernel_lock are held,
>         * as the system configuration lock would be the outermost
>          * lock in the system.
>          */
>         LOCKDEBUG_BARRIER(...);
>         mutex_enter(&sysconfig_mutex);
>         KASSERT(sysconfig_recurse == 0);
> }
>
> For the boot autoconfig path where you have things being driven by the
> kernel, this would work OK.  Situations where there's a kthread or
> something attaching and detaching devices on the fly are a bit different,
> since they likely have local atomicity requirements (need to take device
> private locks).  There you'd probably need the caller to take the lock.
> USB comes to mind (along with some hardware RAID drivers that do hotplug
> attach/detach).
>
> Thoughts?

Well, at first glance, this proposal makes sense.  But it is certainly a 
much larger task than dealing with the immediate problem - modules which 
want to load other modules.

So, I really have three questions:

1. In keeping with earlier concerns about holding locks for long periods
   of time (eeh and mrg both commented on this), the approach described
   above would seem to have an even longer hold-time than the current
   module_lock.  Would not something similar to haad's condvar approach
   be appropriate for this proposal, as well as for the more-limited-in-
   scope recursive module lock?

2. Would it be reasonable to solve the immediate problem as previously
   proposed, rather than waiting for this "ultimate" solution?  I think
   it would be a long time before we could find and resolve all of the
   "issues" that might be created in the various "threaded" situations
   that may exist.  I know I'm certainly not sufficiently qualified to
   identify all those "situations", and I also know that I don't have
   sufficient available work-hours to do this in any reasonable time-
   frame.

   I'd still like to move forward with the most recent solution to the
   module_lock problem.

3. Since we're still technically abusing the mutex(9) man-page caveat
   about using mutex_owned() for making locking decisions, it would
   be very much appreciated (at least by myself) to have an explanation
   of WHY it is OK in this case to do it here, but not somewhere else.





-------------------------------------------------------------------------
| 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  |
-------------------------------------------------------------------------