From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33196) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WIdSy-00020b-Io for qemu-devel@nongnu.org; Wed, 26 Feb 2014 07:20:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WIdSq-0007Vf-2f for qemu-devel@nongnu.org; Wed, 26 Feb 2014 07:20:00 -0500 Received: from mail-pb0-x22b.google.com ([2607:f8b0:400e:c01::22b]:35131) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WIdSp-0007Va-O8 for qemu-devel@nongnu.org; Wed, 26 Feb 2014 07:19:51 -0500 Received: by mail-pb0-f43.google.com with SMTP id ma3so926365pbc.2 for ; Wed, 26 Feb 2014 04:19:50 -0800 (PST) Message-ID: <530DDBDE.1030603@gmail.com> Date: Wed, 26 Feb 2014 20:19:42 +0800 From: Xuebing wang MIME-Version: 1.0 References: <1393406718-16860-1-git-send-email-xbing6@gmail.com> <530DC326.7020806@redhat.com> In-Reply-To: <530DC326.7020806@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] translate: remove file translate-all.h List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Peter Maydell Cc: Blue Swirl , QEMU Developers , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Hi Peter/Paolo, Thanks for replying. 1) I am wondering if some slight refactoring (in the sense of superficial change of moving things around) can make tcg/translationblock/cpu/exec/memory/softmmu more understandable, especially for new engineers. My goal is to explore the possibility of implementing tcg multi-threading, in a similar fashion of kvm multi-threading. It's still at early stage of a work-in-progress. As I am new to qemu code base, I spent some time trying to figure out the design. Examples are: -- What does exec (as in the filename of exec.c) really mean? -- What does "all" mean as in kvm-all,c (xen-all.c, translate-all.c)? I assume "all" means "target independent". But, aren't other files (e.g. block.c, gdbstub.c) target independent? -- What is the difference between "all" and "common" as in include/exec/cpu-all.h and cpu-common.h? In my local repository, I did some refactoring. Examples are: -- I renamed cpu-exec.c => tcg.c, to reflect the design that it's about one of the 3 accelerators (tcg, kvm, xen). -- I renamed cpus.c => vcpu.c (this can be tricky). My idea of the object hierarchy is: [ qom/Object ] [ qdev/DeviceState ] [ qom/CPUState ] [ vcpu API (or qom/vcpu) ] [ CPUArchState ] (in the other words, I am trying to abstract/model vcpu) -- I renamed include/sysemu/cpus.h => include/exec/vcpu.h -- And other changes. Examples: 1) cpu_exec_init_all() in exec.c is about memory, and its name is kind of confusing 2) include/exec/address-spaces.h says below, but "git grep -nw address-spaces.h" reveals a lot. /* * Internal interfaces between memory.c/exec.c/vl.c. Do not #include unless * you're one of them. */ I am wondering if we should slightly re-factor the organization of files, thus new engineers will be easier to understand the design about tcg/exec. 2) Theoretically, include/exec/exec-all.h should work when included from *ANY files*, even mistakenly, right? I hope this patch does NOT do any harm. In my local repository, I split TranslationBlock related from it to be another file include/translate.h. I will see what is the best way to "git send-email" these refactoring for the community to review, without causing too much confusion. The purpose is to make the design easier to understand for new engineers. Thanks again. On 02/26/2014 06:34 PM, Paolo Bonzini wrote: > Il 26/02/2014 10:55, Peter Maydell ha scritto: >> On 26 February 2014 09:25, Xuebing Wang wrote: >>> This patch does below: >>> - Move the declaration of 2 translate functions from >>> translate-all.h into >>> include/exec/exec-all.h >>> - remove file translate-all.h >>> >>> "translate-all.h" => "exec/exec-all.h" can be done by: >>> git grep -w "translate-all.h" | cut -d: -f1 | >>> xargs sed -i 's/\/exec\/exec-all.h/g' >>> >>> Note: >>> 1) "exact whole word match" is considered. >>> 2) We may move translate related from include/exec/exec-all.h into >>> include/exec/translate.h later. >> >> Is there any particular benefit to this change? The function >> prototypes go from being in a header only included by the file >> that uses them to being in a header that's included by a lot >> of other code... > > Indeed, that's the point of translate-all.h. > > Paolo > > -- Thanks, Xuebing Wang