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

[Pawel Jakub Dawidek]

On Sat, Mar 10, 2007 at 12:44:26PM +0100, Attilio Rao wrote:
> 2007/3/9, John Baldwin <jhb at 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 for
> >rwlocks and a cv_*waut*_sx() version for use with sx locks.  One possibility
> >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 having
> >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?
> 
> What about adding a new function like:
> 
> static __inline struct lock_object *
> mtx_export_lc(struct mtx *m)
> {
> 
>        return (&m->mtx_object);
> }
> 
> to be per-interface (so having sx_export_lc() and rw_export_lc() too)
> and than using in this way:
> 
> static struct mtx foo_lock;
> static struct cv foo_cv;
> ...
> 
> mtx_lock(&foo_lock);
> ...
> cv_wait(&foo_cv, mtx_export_lc(&foo_lock));
> 
> (obviously using new struct lock_object methods you added for locking/unlocking)
> 
> 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.

-- 
Pawel Jakub Dawidek                       http://www.wheel.pl
pjd at FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20070310/00ce53d7/attachment.pgp