Message ID | CABp_NWB7SQ9zz8F9UhKJjD_a274OBDjOvQ78Yvm-Hs1N_7zxLw@mail.gmail.com |
---|---|
State | Rejected, archived |
Headers |
From: mohamad.gebai at gmail.com (mo geb) Date: Mon, 27 Jun 2016 11:26:18 -0400 Subject: [lttng-dev] [PATCH] Fix: RHEL 7.2 kvm instrumentation Message-ID: <CABp_NWB7SQ9zz8F9UhKJjD_a274OBDjOvQ78Yvm-Hs1N_7zxLw@mail.gmail.com> |
Commit Message
Mohamad Gebai
June 27, 2016, 3:26 p.m. UTC
Signed-off-by: Mohamad Gebai <mohamad.gebai at gmail.com>
---
instrumentation/events/lttng-module/arch/x86/kvm/trace.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
ctf_integer(__u32, csbase,
kvm_x86_ops->get_segment_base(vcpu, VCPU_SREG_CS))
ctf_integer(__u8, len, vcpu->arch.emulate_ctxt._eip
Comments
----- On Jun 27, 2016, at 11:26 AM, mo geb <mohamad.gebai at gmail.com> wrote: Please use [PATCH lttng-modules] in the subject. > Signed-off-by: Mohamad Gebai < mohamad.gebai at gmail.com > > --- > instrumentation/events/lttng-module/arch/x86/kvm/trace.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > diff --git a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > index 1282bea..ef3d8aa 100644 > --- a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > +++ b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > @@ -429,7 +429,8 @@ LTTNG_TRACEPOINT_EVENT(kvm_emulate_insn, > ctf_integer(__u8, len, vcpu->arch.emulate_ctxt.decode.eip > - vcpu->arch.emulate_ctxt.decode.fetch.start) > ctf_array(__u8, insn, vcpu->arch.emulate_ctxt.decode.fetch.data, 15) > -#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0)) > +#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0) \ > + && !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) We don't use negation "!" in kernel version range tests. We also don't use "&&" typically, to make the code easier to maintain. Instead, we might want to swap the order in which ifdefs are done, and put newer versions at the top, e.g.: #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0)) \ || LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) ... #elif (LINUX_VERSION_CODE >= KERNEL_VERSION(3,1,0)) ... #else ... #endif Thanks, Mathieu > ctf_integer(__u64, rip, vcpu->arch.emulate_ctxt.fetch.start) > ctf_integer(__u32, csbase, kvm_x86_ops->get_segment_base(vcpu, VCPU_SREG_CS)) > ctf_integer(__u8, len, vcpu->arch.emulate_ctxt._eip > -- > 2.1.1 > _______________________________________________ > lttng-dev mailing list > lttng-dev at lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
On 06/27/2016 12:13 PM, Mathieu Desnoyers wrote: > > ----- On Jun 27, 2016, at 11:26 AM, mo geb <mohamad.gebai at gmail.com> > wrote: > > Please use [PATCH lttng-modules] in the subject. > Got it. > > Signed-off-by: Mohamad Gebai <mohamad.gebai at gmail.com> > --- > instrumentation/events/lttng-module/arch/x86/kvm/trace.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git > a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > index 1282bea..ef3d8aa 100644 > --- a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > +++ b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > @@ -429,7 +429,8 @@ LTTNG_TRACEPOINT_EVENT(kvm_emulate_insn, > ctf_integer(__u8, len, > vcpu->arch.emulate_ctxt.decode.eip > - > vcpu->arch.emulate_ctxt.decode.fetch.start) > ctf_array(__u8, insn, > vcpu->arch.emulate_ctxt.decode.fetch.data, 15) > -#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0)) > +#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0) \ > + && !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) > > > We don't use negation "!" in kernel version range tests. We also don't use > "&&" typically, to make the code easier to maintain. > > Instead, we might want to swap the order in which ifdefs are done, and put > newer versions at the top, e.g.: > > #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0)) \ > || LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) > ... > #elif (LINUX_VERSION_CODE >= KERNEL_VERSION(3,1,0)) > ... > #else > ... > #endif > > Thanks, > > Mathieu > Thanks for the comments, I'll make those changes and resubmit. I just want to point out that I used a check similar to what's in wrapper/trace-clock.h: #if ((LTTNG_KERNEL_RANGE(3,10,0, 3,10,14) && !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,0, 3,10,14,0,0)) \ || LTTNG_KERNEL_RANGE(3,11,0, 3,11,3)) #error "Linux kernels 3.10 and 3.11 introduce a deadlock in the timekeeping subsystem. Fixed by commit 7bd36014460f793c19e7d6c94dab67b0afcfcb7f \"timekeeping: Fix HRTICK related deadlock from ntp lock changes\" in Linux." #endif Mohamad -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.lttng.org/pipermail/lttng-dev/attachments/20160627/d9755b12/attachment-0001.html>
---- On Jun 27, 2016, at 1:40 PM, Mohamad <mohamad.gebai at gmail.com> wrote: > On 06/27/2016 12:13 PM, Mathieu Desnoyers wrote: >> ----- On Jun 27, 2016, at 11:26 AM, mo geb <mohamad.gebai at gmail.com> wrote: >> Please use [PATCH lttng-modules] in the subject. > Got it. >>> Signed-off-by: Mohamad Gebai < mohamad.gebai at gmail.com > >>> --- >>> instrumentation/events/lttng-module/arch/x86/kvm/trace.h | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> diff --git a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h >>> b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h >>> index 1282bea..ef3d8aa 100644 >>> --- a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h >>> +++ b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h >>> @@ -429,7 +429,8 @@ LTTNG_TRACEPOINT_EVENT(kvm_emulate_insn, >>> ctf_integer(__u8, len, vcpu->arch.emulate_ctxt.decode.eip >>> - vcpu->arch.emulate_ctxt.decode.fetch.start) >>> ctf_array(__u8, insn, vcpu->arch.emulate_ctxt.decode.fetch.data, 15) >>> -#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0)) >>> +#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0) \ >>> + && !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) >> We don't use negation "!" in kernel version range tests. We also don't use >> "&&" typically, to make the code easier to maintain. >> Instead, we might want to swap the order in which ifdefs are done, and put >> newer versions at the top, e.g.: >> #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0)) \ >> || LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) >> ... >> #elif (LINUX_VERSION_CODE >= KERNEL_VERSION(3,1,0)) >> ... >> #else >> ... >> #endif >> Thanks, >> Mathieu > Thanks for the comments, I'll make those changes and resubmit. I just want to > point out that I used a check similar to what's in wrapper/trace-clock.h: > #if ((LTTNG_KERNEL_RANGE(3,10,0, 3,10,14) && > !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,0, 3,10,14,0,0)) \ > || LTTNG_KERNEL_RANGE(3,11,0, 3,11,3)) > #error "Linux kernels 3.10 and 3.11 introduce a deadlock in the timekeeping > subsystem. Fixed by commit 7bd36014460f793c19e7d6c94dab67b0afcfcb7f > \"timekeeping: Fix HRTICK related deadlock from ntp lock changes\" in Linux." > #endif This is OK for a single #if/endif that surrounds an error condition. For the instrumentation, we tend to end up having multiple #if #elif #elif #elif #endif that accumulate over time. In order to keep this maintainable, I try to avoid having && and ! there. Thanks, Mathieu > Mohamad > _______________________________________________ > lttng-dev mailing list > lttng-dev at lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
Hi, This patch does not seem to target master or even 2.8 since the LTTNG_RHEL_KERNEL_RANGE macro was modified to take 6 arguments per RHEL kernel version. We usually only accept patches targeted at master that are then backported to supported versions. Regards, Michael ----- On Jun 27, 2016, at 11:26 AM, mo geb mohamad.gebai at gmail.com wrote: > Signed-off-by: Mohamad Gebai < mohamad.gebai at gmail.com > > --- > instrumentation/events/lttng-module/arch/x86/kvm/trace.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > > diff --git a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > index 1282bea..ef3d8aa 100644 > --- a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > +++ b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > @@ -429,7 +429,8 @@ LTTNG_TRACEPOINT_EVENT(kvm_emulate_insn, > ctf_integer(__u8, len, vcpu->arch.emulate_ctxt.decode.eip > - vcpu->arch.emulate_ctxt.decode.fetch.start) > ctf_array(__u8, insn, vcpu->arch.emulate_ctxt.decode.fetch.data, 15) > -#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0)) > +#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0) \ > + && !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) > ctf_integer(__u64, rip, vcpu->arch.emulate_ctxt.fetch.start) > ctf_integer(__u32, csbase, kvm_x86_ops->get_segment_base(vcpu, VCPU_SREG_CS)) > ctf_integer(__u8, len, vcpu->arch.emulate_ctxt._eip > -- > 2.1.1 > > _______________________________________________ > lttng-dev mailing list > lttng-dev at lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
You're right, sorry about that. I was on an experimental branch of ours (addons) and didn't think that macro had changed. I'll resubmit based on master. On Mon, Jun 27, 2016 at 2:01 PM, Michael Jeanson <mjeanson at efficios.com> wrote: > Hi, > > This patch does not seem to target master or even 2.8 since the > LTTNG_RHEL_KERNEL_RANGE macro was modified to take 6 arguments per > RHEL kernel version. > > We usually only accept patches targeted at master that are then > backported to supported versions. > > Regards, > > Michael > > ----- On Jun 27, 2016, at 11:26 AM, mo geb mohamad.gebai at gmail.com wrote: > > > Signed-off-by: Mohamad Gebai < mohamad.gebai at gmail.com > > > --- > > instrumentation/events/lttng-module/arch/x86/kvm/trace.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > > b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > > index 1282bea..ef3d8aa 100644 > > --- a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > > +++ b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h > > @@ -429,7 +429,8 @@ LTTNG_TRACEPOINT_EVENT(kvm_emulate_insn, > > ctf_integer(__u8, len, vcpu->arch.emulate_ctxt.decode.eip > > - vcpu->arch.emulate_ctxt.decode.fetch.start) > > ctf_array(__u8, insn, vcpu->arch.emulate_ctxt.decode.fetch.data, 15) > > -#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0)) > > +#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0) \ > > + && !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) > > ctf_integer(__u64, rip, vcpu->arch.emulate_ctxt.fetch.start) > > ctf_integer(__u32, csbase, kvm_x86_ops->get_segment_base(vcpu, > VCPU_SREG_CS)) > > ctf_integer(__u8, len, vcpu->arch.emulate_ctxt._eip > > -- > > 2.1.1 > > > > _______________________________________________ > > lttng-dev mailing list > > lttng-dev at lists.lttng.org > > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.lttng.org/pipermail/lttng-dev/attachments/20160628/66e4973f/attachment.html>
diff --git a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h index 1282bea..ef3d8aa 100644 --- a/instrumentation/events/lttng-module/arch/x86/kvm/trace.h +++ b/instrumentation/events/lttng-module/arch/x86/kvm/trace.h @@ -429,7 +429,8 @@ LTTNG_TRACEPOINT_EVENT(kvm_emulate_insn, ctf_integer(__u8, len, vcpu->arch.emulate_ctxt.decode.eip - vcpu->arch.emulate_ctxt.decode.fetch.start) ctf_array(__u8, insn, vcpu->arch.emulate_ctxt.decode.fetch.data, 15) -#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0)) +#elif (LINUX_VERSION_CODE < KERNEL_VERSION(3,17,0) \ + && !LTTNG_RHEL_KERNEL_RANGE(3,10,0,7,2, 3,11,0,0,0)) ctf_integer(__u64, rip, vcpu->arch.emulate_ctxt.fetch.start)