[lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Mathieu Desnoyers via lttng-dev]

Hi Sean, Hi Paolo,

I have a question regarding a unexport of kvm_x86_ops that made its
way into 5.18-rc (commit dfc4e6ca04 ("KVM: x86: Unexport kvm_x86_ops").
This is in the context of tracing. Especially, LTTng implements probes
for x86 kvm events, e.g. x86 kvm_exit. It receives a struct kvm_vcpu *
as parameter, and uses kvm_x86_ops.get_exit_info() to translate this
into meaningful fields.

LTTng is an out of tree kernel module, which currently relies on the export.
Indeed, arch/x86/kvm/x86.c exports a set of tracepoints to kernel modules, e.g.:

EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_entry)

But any probe implementation hooking on that tracepoint would need kvm_x86_ops
to translate the struct kvm_vcpu * into meaningful tracing data.

I could work-around this on my side in ugly ways, but I would like to discuss
how kernel module tracers are expected to implement kvm events probes without
the kvm_x86_ops symbol ? Perhaps there is an alternative way to convert the
fields in this structure to meaningful information without using the
kvm_x86_ops callbacks that I am not aware of ?

The LTTng kernel tracer uses get_exit_info() and get_segment_base() callbacks
from kvm_x86_ops.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Paolo Bonzini via lttng-dev]

On 4/8/22 17:36, Mathieu Desnoyers wrote:
> LTTng is an out of tree kernel module, which currently relies on the export.
> Indeed, arch/x86/kvm/x86.c exports a set of tracepoints to kernel modules, e.g.:
> 
> EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_entry)
> 
> But any probe implementation hooking on that tracepoint would need kvm_x86_ops
> to translate the struct kvm_vcpu * into meaningful tracing data.
> 
> I could work-around this on my side in ugly ways, but I would like to discuss
> how kernel module tracers are expected to implement kvm events probes without
> the kvm_x86_ops symbol ?

The conversion is done in the TP_fast_assign snippets, which are part of 
kvm.ko and therefore do not need the export.  As I understand it, the 
issue is that LTTng cannot use the TP_fast_assign snippets, because they 
are embedded in the trace_event_raw_event_* symbols?

We cannot do the extraction before calling trace_kvm_exit, because it's 
expensive.

Paolo

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Mathieu Desnoyers via lttng-dev]

----- On Apr 8, 2022, at 12:24 PM, Paolo Bonzini pbonzini@redhat.com wrote:

> On 4/8/22 17:36, Mathieu Desnoyers wrote:
>> LTTng is an out of tree kernel module, which currently relies on the export.
>> Indeed, arch/x86/kvm/x86.c exports a set of tracepoints to kernel modules, e.g.:
>> 
>> EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_entry)
>> 
>> But any probe implementation hooking on that tracepoint would need kvm_x86_ops
>> to translate the struct kvm_vcpu * into meaningful tracing data.
>> 
>> I could work-around this on my side in ugly ways, but I would like to discuss
>> how kernel module tracers are expected to implement kvm events probes without
>> the kvm_x86_ops symbol ?
> 
> The conversion is done in the TP_fast_assign snippets, which are part of
> kvm.ko and therefore do not need the export.  As I understand it, the
> issue is that LTTng cannot use the TP_fast_assign snippets, because they
> are embedded in the trace_event_raw_event_* symbols?

Indeed, the fact that the TP_fast_assign snippets are embedded in the
trace_event_raw_event_* symbols is an issue for LTTng. This ties those
to ftrace.

AFAIK, TP_fast_assign copies directly into ftrace ring buffers, and then
afterwards things like dynamic filters are applied, which then "uncommits" the
events if need be (and if possible). Also, TP_fast_assign is tied to the
ftrace ring buffer event layout. The fact that the TP_STRUCT__entry() (description)
and TP_fast_assign() (open-coded C) are separate fields really focuses on a
use-case where all data is serialized to a ring buffer.

In LTTng, the event fields are made available to a filter interpreter prior to
being copied into LTTng's ring buffer. This is made possible by implementing
our own LTTNG_TRACEPOINT_EVENT code generation headers. In addition, we have
recently released an event notification mechanism (lttng 2.13) which captures
specific event fields to send with an immediate notification (thus bypassing the
tracer buffering). We are also currently working on a LTTng trace hit counters
mechanism, which performs aggregation through per-cpu counters, which doesn't
even allocate a ring buffer.

For those reasons, LTTng reimplements its own tracepoint probe callbacks. All
those sit within LTTng kernel modules, which means we currently need the exported
kvm_x86_ops callbacks.

> We cannot do the extraction before calling trace_kvm_exit, because it's
> expensive.

I suspect that extracting relevant data prior to calling trace_kvm_exit
is too expensive because it cannot be skipped when the tracepoint is
disabled. This is because trace_kvm_exit() is a static inline function,
and the check to figure out if the event is enabled is within that function.
Unfortunately, even if the tracepoint is disabled, the side-effects of the
parameters passed to trace_kvm_exit() must happen.

I've solved this in LTTng-UST by implementing a lttng_ust_tracepoint()
macro, which basically "lifts" the tracepoint enabled check before the
evaluation of the arguments.

You could achieve something similar by using trace_kvm_exit_enabled() in the
kernel like so:

  if (trace_kvm_exit_enabled())
      trace_kvm_exit(....);

Which would skip evaluation of the argument side-effects when the tracepoint is
disabled.

By doing that, when multiple tracers are attached to a kvm tracepoint, the
translation from pointer-to-internal-structure to meaningful fields would only
need to be done once when a tracepoint is hit. And this would remove the need
for using kvm_x86_ops callbacks from tracer probe functions.

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Mathieu Desnoyers via lttng-dev]

----- On Apr 8, 2022, at 2:06 PM, Mathieu Desnoyers via lttng-dev lttng-dev@lists.lttng.org wrote:

> ----- On Apr 8, 2022, at 12:24 PM, Paolo Bonzini pbonzini@redhat.com wrote:
> 
>> On 4/8/22 17:36, Mathieu Desnoyers wrote:
>>> LTTng is an out of tree kernel module, which currently relies on the export.
>>> Indeed, arch/x86/kvm/x86.c exports a set of tracepoints to kernel modules, e.g.:
>>> 
>>> EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_entry)
>>> 
>>> But any probe implementation hooking on that tracepoint would need kvm_x86_ops
>>> to translate the struct kvm_vcpu * into meaningful tracing data.
>>> 
>>> I could work-around this on my side in ugly ways, but I would like to discuss
>>> how kernel module tracers are expected to implement kvm events probes without
>>> the kvm_x86_ops symbol ?
>> 
>> The conversion is done in the TP_fast_assign snippets, which are part of
>> kvm.ko and therefore do not need the export.  As I understand it, the
>> issue is that LTTng cannot use the TP_fast_assign snippets, because they
>> are embedded in the trace_event_raw_event_* symbols?
> 
> Indeed, the fact that the TP_fast_assign snippets are embedded in the
> trace_event_raw_event_* symbols is an issue for LTTng. This ties those
> to ftrace.
> 
> AFAIK, TP_fast_assign copies directly into ftrace ring buffers, and then
> afterwards things like dynamic filters are applied, which then "uncommits" the
> events if need be (and if possible). Also, TP_fast_assign is tied to the
> ftrace ring buffer event layout. The fact that the TP_STRUCT__entry()
> (description)
> and TP_fast_assign() (open-coded C) are separate fields really focuses on a
> use-case where all data is serialized to a ring buffer.
> 
> In LTTng, the event fields are made available to a filter interpreter prior to
> being copied into LTTng's ring buffer. This is made possible by implementing
> our own LTTNG_TRACEPOINT_EVENT code generation headers. In addition, we have
> recently released an event notification mechanism (lttng 2.13) which captures
> specific event fields to send with an immediate notification (thus bypassing the
> tracer buffering). We are also currently working on a LTTng trace hit counters
> mechanism, which performs aggregation through per-cpu counters, which doesn't
> even allocate a ring buffer.
> 
> For those reasons, LTTng reimplements its own tracepoint probe callbacks. All
> those sit within LTTng kernel modules, which means we currently need the
> exported
> kvm_x86_ops callbacks.
> 
>> We cannot do the extraction before calling trace_kvm_exit, because it's
>> expensive.
> 
> I suspect that extracting relevant data prior to calling trace_kvm_exit
> is too expensive because it cannot be skipped when the tracepoint is
> disabled. This is because trace_kvm_exit() is a static inline function,
> and the check to figure out if the event is enabled is within that function.
> Unfortunately, even if the tracepoint is disabled, the side-effects of the
> parameters passed to trace_kvm_exit() must happen.
> 
> I've solved this in LTTng-UST by implementing a lttng_ust_tracepoint()
> macro, which basically "lifts" the tracepoint enabled check before the
> evaluation of the arguments.
> 
> You could achieve something similar by using trace_kvm_exit_enabled() in the
> kernel like so:
> 
>  if (trace_kvm_exit_enabled())
>      trace_kvm_exit(....);
> 
> Which would skip evaluation of the argument side-effects when the tracepoint is
> disabled.
> 
> By doing that, when multiple tracers are attached to a kvm tracepoint, the
> translation from pointer-to-internal-structure to meaningful fields would only
> need to be done once when a tracepoint is hit. And this would remove the need
> for using kvm_x86_ops callbacks from tracer probe functions.
> 
> Thoughts ?

Hi Paolo,

We are at 5.18-rc4 now. Should I expect this unexport to stay in place
for 5.18 final and go ahead with using kallsyms to find this symbol from
lttng-modules instead ?

Thanks,

Mathieu

> 
> Thanks,
> 
> Mathieu
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Paolo Bonzini via lttng-dev]

On 4/25/22 15:00, Mathieu Desnoyers wrote:
> We are at 5.18-rc4 now. Should I expect this unexport to stay in place
> for 5.18 final and go ahead with using kallsyms to find this symbol from
> lttng-modules instead ?

Yes, I don't think honestly that there's any reason to have the symbols 
in place for out-of-tree modules.  I would have hoped Steven to chime 
in, but this also does not seem to be one of the cases where using 
trace_*_enabled() is the way to go.

Paolo

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Steven Rostedt via lttng-dev]

On Fri, 8 Apr 2022 14:06:53 -0400 (EDT)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
> Indeed, the fact that the TP_fast_assign snippets are embedded in the
> trace_event_raw_event_* symbols is an issue for LTTng. This ties those
> to ftrace.

Not just ftrace, perf does it too.

Now another solution is to make the fast assigns available to anyone, and
to allow you to simply pass in a pointer and size to have the data written
into it. That is, you get the results of the TRACE_EVENT and not have to
depend on internal data from the tracepoint.

-- Steve
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Mathieu Desnoyers via lttng-dev]

----- On Apr 25, 2022, at 10:04 AM, Steven Rostedt rostedt@goodmis.org wrote:

> On Fri, 8 Apr 2022 14:06:53 -0400 (EDT)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>> 
>> Indeed, the fact that the TP_fast_assign snippets are embedded in the
>> trace_event_raw_event_* symbols is an issue for LTTng. This ties those
>> to ftrace.
> 
> Not just ftrace, perf does it too.
> 
> Now another solution is to make the fast assigns available to anyone, and
> to allow you to simply pass in a pointer and size to have the data written
> into it. That is, you get the results of the TRACE_EVENT and not have to
> depend on internal data from the tracepoint.

If the fast assign can then be used on a field-per-field basis, maybe this
could work, but AFAIK the fast-assign macro is open-coded C, which makes this
not straightforward.

If it's a all-or-nothing approach where the fast-assign needs to serialize all
fields, this would require that lttng copies the data into a temporary area,
which is something I want to avoid for filtering and event notification per-field
payload capture purposes.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Steven Rostedt via lttng-dev]

On Mon, 25 Apr 2022 12:02:52 -0400 (EDT)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> If the fast assign can then be used on a field-per-field basis, maybe this
> could work, but AFAIK the fast-assign macro is open-coded C, which makes this
> not straightforward.

Yeah, that's not going to be feasible, without changing all 1000's of
events.

> 
> If it's a all-or-nothing approach where the fast-assign needs to serialize all
> fields, this would require that lttng copies the data into a temporary area,
> which is something I want to avoid for filtering and event notification per-field
> payload capture purposes.

Right. Not sure how to fix this then.

-- Steve
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] Unexport of kvm_x86_ops vs tracer modules

[Paolo Bonzini via lttng-dev]

On 4/25/22 18:02, Mathieu Desnoyers wrote:
>> Now another solution is to make the fast assigns available to anyone, and
>> to allow you to simply pass in a pointer and size to have the data written
>> into it. That is, you get the results of the TRACE_EVENT and not have to
>> depend on internal data from the tracepoint.
> If the fast assign can then be used on a field-per-field basis, maybe this
> could work, but AFAIK the fast-assign macro is open-coded C, which makes this
> not straightforward.
> 
> If it's a all-or-nothing approach where the fast-assign needs to serialize all
> fields, this would require that lttng copies the data into a temporary area,
> which is something I want to avoid for filtering and event notification per-field
> payload capture purposes.

It wouldn't be great; but it wouldn't be any worse than doing any 
complex calculations inside an "if (trace_xyz_enabled())", and passing 
them as arguments to the tracepoint function, which is what you 
suggested earlier.

Paolo

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev