[lttng-dev] [PATCH] Fix: RHEL 7.2 kvm instrumentation

[Mathieu Desnoyers]

---- 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

-- 
Mathieu Desnoyers 
EfficiOS Inc. 
http://www.efficios.com 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.lttng.org/pipermail/lttng-dev/attachments/20160627/2b5213ba/attachment.html>