New TTY layer: condvar(9) and Giant

[Ed Schouten]

* John Baldwin <jhb at freebsd.org> wrote:
> Also, your patches won't work in the case of Giant being recursed (it will 
> only drop Giant once and the sleeping thread will still own Giant).  If you 
> do want to make this work my suggestion would be to make the lc_unlock and 
> lc_lock not do anything for Giant.  You could either do this by 1) patching 
> kern_convar.c so it does something like this:
> 
> 	if (lock != &Giant.lo_object)
> 		cookie = class->lc_unlock(lock);
> 
> or instead patch the lc_lock/lc_unlock routines to just not do anything for 
> Giant like so:
> 
> Index: kern_mutex.c
> ===================================================================
> RCS file: /host/cvs/usr/cvs/src/sys/kern/kern_mutex.c,v
> retrieving revision 1.205
> diff -u -r1.205 kern_mutex.c
> --- kern_mutex.c        13 Feb 2008 23:39:05 -0000      1.205
> +++ kern_mutex.c        13 Mar 2008 14:49:04 -0000
> @@ -134,6 +134,8 @@
>  lock_mtx(struct lock_object *lock, int how)
>  {
> 
> +       if (lock == &Giant.lo_object)
> +               return;
>         mtx_lock((struct mtx *)lock);
>  }
> 
> @@ -149,6 +151,8 @@
>  {
>         struct mtx *m;
> 
> +       if (lock == &Giant.lo_object)
> +               return (0);
>         m = (struct mtx *)lock;
>         mtx_assert(m, MA_OWNED | MA_NOTRECURSED);
>         mtx_unlock(m);

Indeed, those solutions look a lot better. The reason why I just
disabled DROP/PICKUP_GIANT, was because I only wanted to allow those
interfaces to work when Giant was only picked up once.

> I still don't like the idea of letting Giant work with msleep/cv_*wait*() 
> because I think it will be abused.

I don't like it either, but we'll need a mechanism like this to make the
transition easier. I would rather have syscons mpsafe, but it just
depends on too many other components that aren't mpsafe either (keyboard
and mouse input, etc).

I'm personally not afraid about it being abused, because people who are
writing new drivers shouldn't be using Giant anyway.

-- 
 Ed Schouten <ed at fxq.nl>
 WWW: http://g-rave.nl/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20080313/6a9431b7/attachment.pgp