From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by ozlabs.org (Postfix) with ESMTP id 3CEBE2C00CB for ; Tue, 1 Oct 2013 00:51:51 +1000 (EST) Message-ID: <52499003.1090405@suse.de> Date: Mon, 30 Sep 2013 16:51:47 +0200 From: Alexander Graf MIME-Version: 1.0 To: "Aneesh Kumar K.V" Subject: Re: [RFC PATCH 07/11] kvm: powerpc: book3s: pr: move PR related tracepoints to a separate header References: <1380276233-17095-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1380276233-17095-8-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <87wqm29zxk.fsf@linux.vnet.ibm.com> <87mwmubh70.fsf@linux.vnet.ibm.com> In-Reply-To: <87mwmubh70.fsf@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Cc: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 09/30/2013 02:57 PM, Aneesh Kumar K.V wrote: > Alexander Graf writes: > >> On 27.09.2013, at 15:06, Aneesh Kumar K.V wrote: >> >>> Alexander Graf writes: >>> >>>> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote: >>>> >>>>> From: "Aneesh Kumar K.V" >>>>> >>>>> This patch moves PR related tracepoints to a separate header. This >>>>> enables in converting PR to a kernel module which will be done in >>>>> later patches >>>>> >>>>> Signed-off-by: Aneesh Kumar K.V >>>>> --- >>>>> arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +- >>>>> arch/powerpc/kvm/book3s_mmu_hpte.c | 2 +- >>>>> arch/powerpc/kvm/book3s_pr.c | 3 +- >>>>> arch/powerpc/kvm/trace.h | 234 +-------------------------- >>>>> arch/powerpc/kvm/trace_pr.h | 297 ++++++++++++++++++++++++++++++++++ >>>>> 5 files changed, 308 insertions(+), 230 deletions(-) >>>>> create mode 100644 arch/powerpc/kvm/trace_pr.h >>>>> >>>>> diff --git a/arch/powerpc/kvm/book3s_64_mmu_host.c b/arch/powerpc/kvm/book3s_64_mmu_host.c >>>>> index 329a978..fd5b393 100644 >>>>> --- a/arch/powerpc/kvm/book3s_64_mmu_host.c >>>>> +++ b/arch/powerpc/kvm/book3s_64_mmu_host.c >>>>> @@ -27,7 +27,7 @@ >>>>> #include >>>>> #include >>>>> #include >>>>> -#include "trace.h" >>>>> +#include "trace_pr.h" >>>>> >>>>> #define PTE_SIZE 12 >>>>> >>>>> diff --git a/arch/powerpc/kvm/book3s_mmu_hpte.c b/arch/powerpc/kvm/book3s_mmu_hpte.c >>>>> index d2d280b..4556168 100644 >>>>> --- a/arch/powerpc/kvm/book3s_mmu_hpte.c >>>>> +++ b/arch/powerpc/kvm/book3s_mmu_hpte.c >>>>> @@ -28,7 +28,7 @@ >>>>> #include >>>>> #include >>>>> >>>>> -#include "trace.h" >>>>> +#include "trace_pr.h" >>>>> >>>>> #define PTE_SIZE 12 >>>>> >>>>> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c >>>>> index 2a97279..99d0839 100644 >>>>> --- a/arch/powerpc/kvm/book3s_pr.c >>>>> +++ b/arch/powerpc/kvm/book3s_pr.c >>>>> @@ -41,7 +41,8 @@ >>>>> #include >>>>> #include >>>>> >>>>> -#include "trace.h" >>>>> +#define CREATE_TRACE_POINTS >>>>> +#include "trace_pr.h" >>>>> >>>>> /* #define EXIT_DEBUG */ >>>>> /* #define DEBUG_EXT */ >>>>> diff --git a/arch/powerpc/kvm/trace.h b/arch/powerpc/kvm/trace.h >>>>> index a088e9a..7d5a136 100644 >>>>> --- a/arch/powerpc/kvm/trace.h >>>>> +++ b/arch/powerpc/kvm/trace.h >>>>> @@ -85,6 +85,12 @@ TRACE_EVENT(kvm_ppc_instr, >>>>> {41, "HV_PRIV"} >>>>> #endif >>>>> >>>>> +#ifndef CONFIG_KVM_BOOK3S_PR >>>>> +/* >>>>> + * For pr we define this in trace_pr.h since it pr can be built as >>>>> + * a module >>>> Not sure I understand the need. If the config option is available, so >>>> should the struct field. Worst case that happens with HV is that we >>>> get empty shadow_srr1 values in our trace, no? >>> That is not the real reason. trace.h get built as part of kvm.ko or as >>> part of kernel. These trace functions actually get called from >>> kvm-pr.ko. To make they build i would either need EXPORT_SYMBOL or move >>> the definition of them to kvm-pr.ko. I did the later and moved only pr >>> related traces to kvm-pr.ko >> I fail to see why we wouldn't have a trace_hv.h file then, as that can >> also be built as a module, no? And at that point I don't see why we >> would need any conditionals at all in trace.h anymore, as it would >> only cover generic code. > Currently HV module is not using any tracepoints. Once it start using > tracepoints we would have trace_hv.h So why would there be an #ifndef in trace.h? Alex