Date: Thu, 13 Mar 2008 16:17:13 +0100 From: Ed Schouten <ed@80386.nl> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-arch@freebsd.org Subject: Re: New TTY layer: condvar(9) and Giant Message-ID: <20080313151713.GD80576@hoeg.nl> In-Reply-To: <200803131049.58051.jhb@freebsd.org> References: <20080313135035.GB80576@hoeg.nl> <200803131049.58051.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--hNweOTLwwbnii4NA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
* John Baldwin <jhb@freebsd.org> wrote:
> Also, your patches won't work in the case of Giant being recursed (it wil=
l=20
> only drop Giant once and the sleeping thread will still own Giant). If y=
ou=20
> do want to make this work my suggestion would be to make the lc_unlock an=
d=20
> lc_lock not do anything for Giant. You could either do this by 1) patchi=
ng=20
> kern_convar.c so it does something like this:
>=20
> if (lock !=3D &Giant.lo_object)
> cookie =3D class->lc_unlock(lock);
>=20
> or instead patch the lc_lock/lc_unlock routines to just not do anything f=
or=20
> Giant like so:
>=20
> Index: kern_mutex.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> 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)
> {
>=20
> + if (lock =3D=3D &Giant.lo_object)
> + return;
> mtx_lock((struct mtx *)lock);
> }
>=20
> @@ -149,6 +151,8 @@
> {
> struct mtx *m;
>=20
> + if (lock =3D=3D &Giant.lo_object)
> + return (0);
> m =3D (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*()=
=20
> 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.
--=20
Ed Schouten <ed@fxq.nl>
WWW: http://g-rave.nl/
--hNweOTLwwbnii4NA
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (FreeBSD)
iEYEARECAAYFAkfZRXkACgkQ52SDGA2eCwWNmQCfQFxfN0qyrvU2BF1xOXCfmF5V
X3wAni0zPlW+3eO5i1vaCCDQg5YAOCzf
=pJ4B
-----END PGP SIGNATURE-----
--hNweOTLwwbnii4NA--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080313151713.GD80576>