Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 10 Mar 2007 21:52:36 +0100
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Attilio Rao <attilio@freebsd.org>
Cc:        cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org, John Baldwin <jhb@freebsd.org>
Subject:   Re: cvs commit: src/share/man/man9 Makefile condvar.9 lock.9 mi_switch.9 mtx_pool.9 mutex.9 rwlock.9 sleep.9 sleepqueue.9 sx.9 thread_exit.9 src/sys/kern kern_synch.c src/sys/sys mutex.h rwlock.h sleepqueue.h sx.h systm.h
Message-ID:  <20070310205236.GA9185@garage.freebsd.pl>
In-Reply-To: <3bbf2fe10703100344w1f2464f0q68086a5af7c4f63c@mail.gmail.com>
References:  <200703092241.l29Mf2Ds062856@repoman.freebsd.org> <200703091747.07617.jhb@freebsd.org> <3bbf2fe10703100344w1f2464f0q68086a5af7c4f63c@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--d6Gm4EdcadzBjdND
Content-Type: text/plain; charset=iso-8859-2
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 10, 2007 at 12:44:26PM +0100, Attilio Rao wrote:
> 2007/3/9, John Baldwin <jhb@freebsd.org>:
> >I don't have a date set for removing msleep(), esp. given it's wide use.
> >I would like to remove it and all the spl*() functions in 8.0 if we can
> >swing it.
> >
> >I also have patches to let condition variables work with rwlocks and sx
> >locks, but the current implementation results in an API "explosion"
> >since each of the cv_*wait*() functions grows a cv_*wait*_rw() version f=
or
> >rwlocks and a cv_*waut*_sx() version for use with sx locks.  One possibi=
lity
> >would be to just cast the lock argument to (struct lock_object *) since =
all
> >of our locks have a lock_object as the first member, but then you use ha=
ving
> >the compiler do type checking, and I'm really not willing to give up on
> >that.  Too easy to have evil bugs that way.  I suppose we could use some
> >evil macro that used typeof() but that would be very gcc specific?
> >
> >I guess one other possibility is to standardize on the field name for
> >the lock_object, calling it lo_object instead of mtx_object, rw_object,
> >sx_object, etc.  Anyone else have any ideas?
>=20
> What about adding a new function like:
>=20
> static __inline struct lock_object *
> mtx_export_lc(struct mtx *m)
> {
>=20
>        return (&m->mtx_object);
> }
>=20
> to be per-interface (so having sx_export_lc() and rw_export_lc() too)
> and than using in this way:
>=20
> static struct mtx foo_lock;
> static struct cv foo_cv;
> ...
>=20
> mtx_lock(&foo_lock);
> ...
> cv_wait(&foo_cv, mtx_export_lc(&foo_lock));
>=20
> (obviously using new struct lock_object methods you added for locking/unl=
ocking)
>=20
> It sounds reasonable to you?

This is ugly. If we really need to provide information about which type
of lock we are using, I'd probably prefer cv_wait_<locktype>().

What about something like this:

#define	cv_wait(cv, lock)	do {
	switch (LO_CLASSINDEX((struct lock_object *)(lock))) {
	case 1:
		cv_wait_mtx(cv, lock);
		break;
	case 2:
		cv_wait_sx(cv, lock);
		break;
	case 3:
		cv_wait_rw(cv, lock);
		break;
	default:
		panic("Invalid lock.");
	}
} while (0)

Of course magic values should be replaced, but you get the point.

PS. I'd really like to be able to use condvar(9) with sx(9) locks,
because currently I've to manage mu own condvar(9) version for ZFS
that does exactly this.

--=20
Pawel Jakub Dawidek                       http://www.wheel.pl
pjd@FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!

--d6Gm4EdcadzBjdND
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (FreeBSD)

iD8DBQFF8xqUForvXbEpPzQRAsJwAJ4/rTRjOmna173ToHJv89baYQT8wgCfb1LM
YcfgHtQ29Gw1fJxPTipDGbQ=
=xXhu
-----END PGP SIGNATURE-----

--d6Gm4EdcadzBjdND--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070310205236.GA9185>