Date: Sun, 11 Mar 2007 01:11:31 +0100 From: "Attilio Rao" <attilio@freebsd.org> To: "Pawel Jakub Dawidek" <pjd@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, John Baldwin <jhb@freebsd.org>, cvs-all@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: <3bbf2fe10703101611r60d6e1c5o8ef5fcf309fbf0e3@mail.gmail.com> In-Reply-To: <20070310205236.GA9185@garage.freebsd.pl> References: <200703092241.l29Mf2Ds062856@repoman.freebsd.org> <200703091747.07617.jhb@freebsd.org> <3bbf2fe10703100344w1f2464f0q68086a5af7c4f63c@mail.gmail.com> <20070310205236.GA9185@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
2007/3/10, Pawel Jakub Dawidek <pjd@freebsd.org>:
> 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 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)
This is exactly what John is trying to avoid.
You have however to export cv_wait_*() & friends in the public
namespace and at this point you don't need such wrapper.
I know it is not so elegant, but the other solutions are uglier.
Having a function returning the lock object per-primitive is the most
suitable, IMHO.
Attilio
--
Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe10703101611r60d6e1c5o8ef5fcf309fbf0e3>