From patchwork Fri Feb 10 17:07:43 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 140694 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4BA97B6F13 for ; Sat, 11 Feb 2012 04:05:08 +1100 (EST) Received: from localhost ([::1]:53587 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RvtuE-0001R9-29 for incoming@patchwork.ozlabs.org; Fri, 10 Feb 2012 12:05:06 -0500 Received: from eggs.gnu.org ([140.186.70.92]:39251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rvtu2-0001Pr-88 for qemu-devel@nongnu.org; Fri, 10 Feb 2012 12:05:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rvttw-0001jN-35 for qemu-devel@nongnu.org; Fri, 10 Feb 2012 12:04:54 -0500 Received: from smtp.ctxuk.citrix.com ([62.200.22.115]:58757 helo=SMTP.EU.CITRIX.COM) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rvttv-0001j8-Pv for qemu-devel@nongnu.org; Fri, 10 Feb 2012 12:04:48 -0500 X-IronPort-AV: E=Sophos;i="4.73,396,1325462400"; d="scan'208";a="10628245" Received: from lonpmailmx01.citrite.net ([10.30.203.162]) by LONPIPO01.EU.CITRIX.COM with ESMTP/TLS/RC4-MD5; 10 Feb 2012 17:04:14 +0000 Received: from kaball.uk.xensource.com (10.80.2.59) by LONPMAILMX01.citrite.net (10.30.203.162) with Microsoft SMTP Server id 8.3.213.0; Fri, 10 Feb 2012 17:04:14 +0000 Date: Fri, 10 Feb 2012 17:07:43 +0000 From: Stefano Stabellini X-X-Sender: sstabellini@kaball-desktop To: Jan Kiszka In-Reply-To: <4F350047.4030507@siemens.com> Message-ID: References: <201202100952.26104.paul@codesourcery.com> <4F34F567.3040309@redhat.com> <201202101109.03374.paul@codesourcery.com> <4F34FCF3.4080300@redhat.com> <4F350047.4030507@siemens.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 62.200.22.115 Cc: "xen-devel@lists.xensource.com" , Stefano Stabellini , "qemu-devel@nongnu.org" , Paul Brook , Paolo Bonzini , "avi@redhat.com" Subject: Re: [Qemu-devel] [PATCH v3 6/6] qemu_calculate_timeout: increase minimum timeout to 1h X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Fri, 10 Feb 2012, Jan Kiszka wrote: > On 2012-02-10 12:18, Paolo Bonzini wrote: > > On 02/10/2012 12:19 PM, Stefano Stabellini wrote: > >> I think you are right and the right thing to do would be blocking > >> indefinitely. > >> However if slirp doesn't support it, we could have a timeout of 1000 if > >> CONFIG_SLIRP, otherwise block indefinitely. > > > > You could add a similar hack to qemu_bh_update_timeout for > > slirp_update_timeout. > > Real solutions would be preferred, but I know that the code is hairy. In > any case, please no CONFIG_SLIRP code forks. I tried to follow your suggestions, what do you guys think about the approach of the patch below? --- main_loop_wait: block indefinitely - remove qemu_calculate_timeout; - explicitly size timeout to uint32_t; - introduce slirp_update_timeout; - pass NULL as timeout argument to select in case timeout is the maximum value; Signed-off-by: Stefano Stabellini diff --git a/async.c b/async.c index 332d511..ecdaf15 100644 --- a/async.c +++ b/async.c @@ -120,7 +120,7 @@ void qemu_bh_delete(QEMUBH *bh) bh->deleted = 1; } -void qemu_bh_update_timeout(int *timeout) +void qemu_bh_update_timeout(uint32_t *timeout) { QEMUBH *bh; diff --git a/main-loop.c b/main-loop.c index 692381c..4a105e9 100644 --- a/main-loop.c +++ b/main-loop.c @@ -366,7 +366,7 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque) } } -static void os_host_main_loop_wait(int *timeout) +static void os_host_main_loop_wait(uint32_t *timeout) { int ret, ret2, i; PollingEntry *pe; @@ -410,7 +410,7 @@ static void os_host_main_loop_wait(int *timeout) *timeout = 0; } #else -static inline void os_host_main_loop_wait(int *timeout) +static inline void os_host_main_loop_wait(uint32_t *timeout) { } #endif @@ -419,21 +419,17 @@ int main_loop_wait(int nonblocking) { fd_set rfds, wfds, xfds; int ret, nfds; - struct timeval tv; - int timeout; + struct timeval tv, *tvarg = NULL; + uint32_t timeout = UINT32_MAX; if (nonblocking) { timeout = 0; } else { - timeout = qemu_calculate_timeout(); qemu_bh_update_timeout(&timeout); } os_host_main_loop_wait(&timeout); - tv.tv_sec = timeout / 1000; - tv.tv_usec = (timeout % 1000) * 1000; - /* poll any events */ /* XXX: separate device handlers from system ones */ nfds = -1; @@ -442,16 +438,23 @@ int main_loop_wait(int nonblocking) FD_ZERO(&xfds); #ifdef CONFIG_SLIRP + slirp_update_timeout(&timeout); slirp_select_fill(&nfds, &rfds, &wfds, &xfds); #endif qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds); glib_select_fill(&nfds, &rfds, &wfds, &xfds, &tv); + if (timeout < UINT32_MAX) { + tvarg = &tv; + tv.tv_sec = timeout / 1000; + tv.tv_usec = (timeout % 1000) * 1000; + } + if (timeout > 0) { qemu_mutex_unlock_iothread(); } - ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv); + ret = select(nfds + 1, &rfds, &wfds, &xfds, tvarg); if (timeout > 0) { qemu_mutex_lock_iothread(); diff --git a/main-loop.h b/main-loop.h index f971013..22c0dc9 100644 --- a/main-loop.h +++ b/main-loop.h @@ -352,6 +352,6 @@ void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int rc void qemu_bh_schedule_idle(QEMUBH *bh); int qemu_bh_poll(void); -void qemu_bh_update_timeout(int *timeout); +void qemu_bh_update_timeout(uint32_t *timeout); #endif diff --git a/qemu-timer.c b/qemu-timer.c index de20852..3e1ce08 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -821,8 +821,3 @@ fail: return err; } -int qemu_calculate_timeout(void) -{ - return 1000; -} - diff --git a/qemu-timer.h b/qemu-timer.h index de17f3b..f1386d5 100644 --- a/qemu-timer.h +++ b/qemu-timer.h @@ -62,7 +62,6 @@ uint64_t qemu_timer_expire_time_ns(QEMUTimer *ts); void qemu_run_all_timers(void); int qemu_alarm_pending(void); void configure_alarms(char const *opt); -int qemu_calculate_timeout(void); void init_clocks(void); int init_timer_alarm(void); diff --git a/slirp/libslirp.h b/slirp/libslirp.h index 890fd86..6af7534 100644 --- a/slirp/libslirp.h +++ b/slirp/libslirp.h @@ -42,4 +42,13 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr, size_t slirp_socket_can_recv(Slirp *slirp, struct in_addr guest_addr, int guest_port); +#ifdef CONFIG_SLIRP +static inline void slirp_update_timeout(uint32_t *timeout) +{ + *timeout = MIN(1000, *timeout); +} +#else +static inline void slirp_update_timeout(uint32_t *timeout) { } +#endif + #endif