From owner-freebsd-hackers@FreeBSD.ORG Fri Jan 22 17:58:41 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 14E1910656A8 for ; Fri, 22 Jan 2010 17:58:41 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.10]) by mx1.freebsd.org (Postfix) with ESMTP id D858F8FC14 for ; Fri, 22 Jan 2010 17:58:40 +0000 (UTC) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.14.4/8.14.4/NETPLEX) with ESMTP id o0MHwcZM011238; Fri, 22 Jan 2010 12:58:38 -0500 (EST) X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.2.2 (mail.netplex.net [204.213.176.10]); Fri, 22 Jan 2010 12:58:39 -0500 (EST) Date: Fri, 22 Jan 2010 12:58:38 -0500 (EST) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net To: Jilles Tjoelker In-Reply-To: <20100122172525.GA37884@stack.nl> Message-ID: References: <9bbcef731001220527u5bbec479n59143b6631c6e2d8@mail.gmail.com> <20100122151035.GX77705@hoeg.nl> <905EFCFB-7362-4F54-B9E7-69C0B4699A37@lakerest.net> <20100122172525.GA37884@stack.nl> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-hackers@freebsd.org, Randall Stewart , Ivan Voras Subject: Re: Greetings... a patch I would like your comments on... X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Daniel Eischen List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Jan 2010 17:58:41 -0000 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 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