From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40022 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pn55d-0003I9-CO for qemu-devel@nongnu.org; Wed, 09 Feb 2011 03:07:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pn55b-00007B-EV for qemu-devel@nongnu.org; Wed, 09 Feb 2011 03:07:53 -0500 Received: from fmmailgate02.web.de ([217.72.192.227]:49052) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pn55b-000071-4R for qemu-devel@nongnu.org; Wed, 09 Feb 2011 03:07:51 -0500 Message-ID: <4D524B56.1050900@web.de> Date: Wed, 09 Feb 2011 09:07:50 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1e7a4f5097aafb5da844c1b9ff8661539d0d10e8.1297077506.git.jan.kiszka@siemens.com> <20110208185056.GA13617@amt.cnet> In-Reply-To: <20110208185056.GA13617@amt.cnet> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig22DDE9D7CCBDEC216FA74CD9" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH 02/15] Refactor cpu_has_work/any_cpu_has_work in cpus.c List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcelo Tosatti Cc: Avi Kivity , kvm@vger.kernel.org, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig22DDE9D7CCBDEC216FA74CD9 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2011-02-08 19:50, Marcelo Tosatti wrote: > On Mon, Feb 07, 2011 at 12:19:13PM +0100, Jan Kiszka wrote: >> Avoid duplicate use of the function name cpu_has_work, it's confusing.= >> Refactor cpu_has_work to cpu_is_idle and do the same with >> any_cpu_has_work. >> >> Signed-off-by: Jan Kiszka >> --- >> cpus.c | 43 +++++++++++++++++++++++-------------------- >> 1 files changed, 23 insertions(+), 20 deletions(-) >> >> diff --git a/cpus.c b/cpus.c >> index d54ec7d..cd764f2 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -137,29 +137,30 @@ static int cpu_can_run(CPUState *env) >> return 1; >> } >> =20 >> -static int cpu_has_work(CPUState *env) >> +static bool cpu_is_idle(CPUState *env) >> { >> - if (env->stop) >> - return 1; >> - if (env->queued_work_first) >> - return 1; >> - if (env->stopped || !vm_running) >> - return 0; >> - if (!env->halted) >> - return 1; >> - if (qemu_cpu_has_work(env)) >> - return 1; >> - return 0; >> + if (env->stop || env->queued_work_first) { >> + return false; >> + } >> + if (env->stopped || !vm_running) { >> + return true; >> + } >> + if (!env->halted || qemu_cpu_has_work(env)) { >> + return false; >> + } >> + return true; >> } >=20 > Do you really find it easier to read evaluations grouped with || ? I > don't. I do, specifically as the old version was even more confusing in that important detail "return 0" vs. "return 1". But even the new benefits from the grouping IMHO. >=20 > Sorry but the name change does not feel right either: CPU is still idle= > if the vm is not running. But that's exactly what the function returns. Or is it confusing if we are talking about the vcpu or the whole thread here? What about "cpu_thread_is_idle" then? Jan --------------enig22DDE9D7CCBDEC216FA74CD9 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAk1SS1YACgkQitSsb3rl5xRCVwCgrjiEpBulp6GE4GeS3XwcDT+s 7vEAoIzhl6yESRBudW9389uPdekASKjG =MfUX -----END PGP SIGNATURE----- --------------enig22DDE9D7CCBDEC216FA74CD9--