From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43820) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VBgFH-0001tJ-7g for qemu-devel@nongnu.org; Tue, 20 Aug 2013 03:21:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VBgF8-0008Ao-8A for qemu-devel@nongnu.org; Tue, 20 Aug 2013 03:20:51 -0400 Received: from e28smtp05.in.ibm.com ([122.248.162.5]:39387) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VBgF7-00089y-EZ for qemu-devel@nongnu.org; Tue, 20 Aug 2013 03:20:42 -0400 Received: from /spool/local by e28smtp05.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 20 Aug 2013 12:44:13 +0530 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id E75581258053 for ; Tue, 20 Aug 2013 12:50:21 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r7K7M4jj43712702 for ; Tue, 20 Aug 2013 12:52:04 +0530 Received: from d28av02.in.ibm.com (localhost [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r7K7KZjg003510 for ; Tue, 20 Aug 2013 12:50:36 +0530 Message-ID: <5213186F.3050900@linux.vnet.ibm.com> Date: Tue, 20 Aug 2013 15:19:11 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1376598879-18976-1-git-send-email-alex@alex.org.uk> <1376598879-18976-14-git-send-email-alex@alex.org.uk> <5212DDCA.5060906@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCHv11 13/31] aio / timers: aio_ctx_prepare sets timeout from AioContext timers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Bligh Cc: Kevin Wolf , Anthony Liguori , Jan Kiszka , qemu-devel@nongnu.org, liu ping fan , Stefan Hajnoczi , Paolo Bonzini , MORITA Kazutaka , rth@twiddle.net 于 2013-8-20 14:48, Alex Bligh 写道: >>> >>> for (bh = ctx->first_bh; bh; bh = bh->next) { >>> if (!bh->deleted && bh->scheduled) { >>> if (bh->idle) { >>> /* idle bottom halves will be polled at least >>> * every 10ms */ >>> - *timeout = 10; >>> + *timeout = qemu_soonest_timeout(*timeout, 10); >> glib will not set *timeout to a meaningful value before calling >> aio_ctx_prepare(), right? If so, "*timeout = 10" should be used. > > > I don't think that's correct. Each _prepare routine is called > and has the abilitiy to alter the timeout but need not > set it at all. Indeed it should not set it as there may already > be a shorter timeout there. > > Here's the code before I touched it: > > aio_ctx_prepare(GSource *source, gint *timeout) > { > AioContext *ctx = (AioContext *) source; > QEMUBH *bh; > > for (bh = ctx->first_bh; bh; bh = bh->next) { > if (!bh->deleted && bh->scheduled) { > if (bh->idle) { > /* idle bottom halves will be polled at least > * every 10ms */ > *timeout = 10; > } else { > /* non-idle bottom halves will be executed > * immediately */ > *timeout = 0; > return true; > } > } > } > > return false; > } > > You'll note that what this does is: > a) if there are no bottom halves, leaves *timeout as is > b) if there is a non-idle bottom half, set timeout to 0 > c) else set timeout to 10 if there are only idle bottom > halves. > > Arguably (c) is a bug if timeout was shorter than 10 > on entry but the whole of idle bhs are a bit of a bodge. > This is fixed by my series. > > If you are asking WHERE it gets set to -1, I don't claim > to be a glib expert but I believe it's the line > gint source_timeout = -1 > around line 2816 in glib/gmain.c > Thanks for the explanation. It seems *timeout is always set to -1 before calling GSource's prepare(), so "*timeout = qemu_soonest_timeout(*timeout, 10);" will always get *timeout = 10, so this call can be saved. -- Best Regards Wenchao Xia