2007/3/12, John Baldwin <[EMAIL PROTECTED]>:
On Saturday 10 March 2007 19:11, Attilio Rao wrote:
> 2007/3/10, Pawel Jakub Dawidek <[EMAIL PROTECTED]>:
> > On Sat, Mar 10, 2007 at 12:44:26PM +0100, Attilio Rao wrote:
> > > 2007/3/9, John Baldwin <[EMAIL PROTECTED]>:
> > > >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.
No, that's more typing than _rw and _sx. Here is what I want to happen if
possible:
Ok, but it doesn't lead to the API explosion and doesn't have the two
problems you mentioned before with struct lock_object:
1) having a "name costrained" member for the lock object
2) having the lock object as first member
cv_wait(cv, mtx);
cv_wait(cv, rw);
cv_wait(cv, sx);
and have the the compiler figure it out. Basically, trying to shoehorn some
C++ into C since mtx, rw, and sx are sub-classes of 'lock_object'. :) That
is, I'd like it to do something like this:
#define cv_wait(cv, lock) do { \
if (typeof(lock) == (struct mtx *)) \
_cv_wait(cv, &lock->mtx_object); \
else if (typeof(lock) == (struct rwlock *)) \
_cv_wait(cv, &lock->rw_object); \
else if (typeof(lock) == (struct sx *)) \
_cv_wait(cv, &lock->sx_object); \
else \
compile_error; \
} while (0)
So you still get type checking, etc. I'm thinking maybe the simplest thing to
do is to rename 'mtx_object', 'rw_object', and 'sx_object' fields to all
be 'lock_object' and then do this:
Even if this idea is not so bad (beacause we primitives are in a small
and controlled number of them) we have a lot of places to update for
that. A macro could be a temporary help, btw.
Attilio
--
Peace can only be achieved by understanding - A. Einstein
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
