Greetings... a patch I would like your comments on...

[Daniel Eischen]

On Fri, 22 Jan 2010, Jilles Tjoelker wrote:

> On Fri, Jan 22, 2010 at 07:55:47AM -0800, Randall Stewart wrote:
>> On Jan 22, 2010, at 7:33 AM, Ivan Voras wrote:
>
>>> On 01/22/10 16:10, Ed Schouten wrote:
>>>> * Ivan Voras<ivoras at freebsd.org>  wrote:
>>>>> This is a good and useful addition! I think Windows has
>>>>> implemented a
>>>>> generalization of this (called "wait objects" or something like
>>>>> that),
>>>>> which effectively allows a select()- (or in this case kqueue())-like
>>>>> syscall to wait on both file descriptors and condvars (as well as
>>>>> probably other MS-style objects). It's useful for multiplexing
>>>>> events
>>>>> for dissimilar sources.
>
>>>> NtWaitForSingleObject(), NtWaitForMultipleObjects(), etc. :-)
>
>>> Yes, I was thinking about WaitForMultipleObjects() - I sometimes
>>> wished I had it in FreeBSD :)
>
>>> I think the hackers@ side of the thread is missing the original link
>>> to the patch file offered for review, so here it is:
>
>>> http://people.freebsd.org/~rrs/kque_umtx.patch
>
> Cool.
>
>>> My kqueue-fu level is too low to be really useful here but from what
>>> I've read it looks like a logical and even reasonably clean way of
>>> doing it.
>
>> thanks it made sense to me ;-)
>
>>> If I read the comment at filt_umtxattach() correctly, in the best
>>> case you would need an extension to the kevent structure to add more
>>> fields like data & udata (for passing values back and forth between
>>> userland and kernel). I agree with this - it would be very
>>> convenient for some future purposes (like file modification
>>> notification) if the kernel filter could both accept and return a
>>> struct of data from/to the userland.
>
>> Yeah, more arguments inside the kevent would allow me to add the
>> COND_CV_WAIT* where a lock and condition are passed
>> in as well... But I was hesitant to add more than was already there
>> since doing
>> so would cause ABI ripples that I did not want to face.
>
> I don't think passing the lock is needed.
>
> Traditional way (error handling omitted):
>
> pthread_mutex_lock(&M);
> while (!P)
> 	pthread_cond_wait(&C, &M);
> do_work();
> pthread_mutex_unlock(&M);
>
> The thread must be registered as a waiter before unlocking the mutex,
> otherwise a wakeup could be lost.
>
> Possible kqueue way (error handling omitted):
>
> kq = kqueue();
> pthread_mutex_lock(&M);
> while (!P)
> {
> 	pthread_cond_kqueue_register_wait_np(&C, kq);
> 	pthread_mutex_unlock(&M);
> 	nevents = kevent(kq, NULL, 0, events, 1, NULL);
> 	... handle other events ...
> 	pthread_mutex_lock(&M);
> }
> do_work();
> pthread_mutex_unlock(&M);
> close(kq);

This is ugly from the userland point of view -- if you need
this, I would encompass everything in a kq event object
thingie.  As in:

/*
  * Create an object containing everything you need to wait
  * or signal kqueue events.
  */
kq = kqueue();
kq_obj = kq_create(kq, ...);

kq_lock(&kq_obj);
while (!P)
{
 	/* Atomically unlocks kq_obj and blocks. */
 	nevents = kq_wait(&kq_obj, ...);

 	/* When you wakeup, kq_obj is locked again. */
}

do_work();  /* Wouldn't you want to unlock before this? */
kq_unlock(&kq_obj);


/* From another thread, you could do: */
kq_lock(&kq_obj);
if (kq_waiters(&kq_obj) > 0)
 	kq_signal(&kq_obj);  /* or kq_broadcast() */
kq_unlock(&kq_obj);


-- 
DE