From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56280 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P8YMW-0004LB-KC for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:05:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P8YML-0008Gq-Nz for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:05:39 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:46148) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P8YML-0008Gh-Lk for qemu-devel@nongnu.org; Wed, 20 Oct 2010 09:05:37 -0400 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by e6.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id o9KD5t9q001522 for ; Wed, 20 Oct 2010 09:05:55 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o9KD5LSP167100 for ; Wed, 20 Oct 2010 09:05:21 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o9KD5K9C013336 for ; Wed, 20 Oct 2010 11:05:21 -0200 Date: Wed, 20 Oct 2010 18:35:17 +0530 From: Balbir Singh Subject: Re: [Qemu-devel] [PATCH 1/3] Introduce threadlets Message-ID: <20101020130517.GR15844@balbir.in.ibm.com> References: <20101019173946.16514.62027.stgit@localhost6.localdomain6> <20101019174245.16514.14542.stgit@localhost6.localdomain6> <20101019183644.GI15844@balbir.in.ibm.com> <4CBE0F5F.3020204@codemonkey.ws> <20101020022217.GL15844@balbir.in.ibm.com> <4CBE661B.1010205@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <4CBE661B.1010205@linux.vnet.ibm.com> Reply-To: balbir@linux.vnet.ibm.com List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Venkateswararao Jujjuri (JV)" Cc: Arun R Bharadwaj , qemu-devel@nongnu.org * Venkateswararao Jujjuri (JV) [2010-10-19 20:46:35]: > >> I think this is a lot more fragile. You're relying on the fact that > >> signal will not cause the signalled thread to actually awaken until > >> we release the lock and doing work after signalling that the > >> signalled thread needs to be completed before it wakes up. > >> > >> I think you're a lot more robust in the long term if you treat > >> condition signalling as a hand off point because it makes the code a > >> lot more explicit about what's happening. > >> > > > > OK, here is a situation that can happen > > > > T1 T2 > > --- --- > > threadlet submit_threadletwork_to_queue > > (sees condition as no work) mutex_lock > > qemu_cond_timedwait add_work > > ... mutex_unlock > > > > T3 > > -- > > cancel_threadlet_work_on_queue > > mutex_lock (grabs it) before T1 can > > cancels the work > > > > > > qemu_cond_signal > > > > T1 > > -- > > Grabs mutex_lock (from within cond_timedwait) > > Now there is no work to do, the condition > > has changed before the thread wakes up > > So what? It won't find any work and goes back to sleep or exits. > Spurious wakeups are not good - they waste CPU cycles, consume energy. Beyond that if we look at generic design a. We want the thread condition to not change before it wakes up (reduce that window at-least) b. Although we don't care about thread priorities today in threadlet, if we ever did and by good design you'd want the thread your waking up to be contending for the mutex as soon the notifier releases the lock, otherwise a lower priority thread can starve the original sleeper. The code as posted today, does not have functional issues except for opening up the window for spurious wakeups. > idle_threads is decremented only in threadlet_worker(). Given that > we have a threadlet that is not doing anywork the assert should never hit unless > something horribly wrong . > -- Three Cheers, Balbir