Re: [lttng-dev] [PATCH lttng-modules 0/1] Introduce configure script to describe changes in linux kernel interface

[Mathieu Desnoyers via lttng-dev <lttng-dev@lists.lttng.org>]

On 7/4/23 14:39, Roxana Nicolescu wrote:
> Hi,
> 
> Thanks a lot for your feedback.
> 
> I realize I did not say the reason why I did not go for 
> LTTNG_UBUNTU_KERNEL_RANGE. We deliver a bunch of different
> derivatives (inherited from the main kernel), each with its own
> version and it's impossible to use LTTNG_UBUNTU_KERNEL_RANGE alone. 
> Derivatives in the same cycle don't have the same version number, so
> I cannot rely on the version alone to determine when a change has
> happened. For example these are some kernels we released last cycle: 
> - linux (main kernel): 5.19.0-46 - linux-kvm: 5.19.0-1026 -
> linux-lowlatency: 5.19.0-1028 As you can see, linux-kvm and
> linux-lowlatency versions are not the same, and linux-lowlatency from
> 2 months ago version version number coincides with linux-kvm from
> now, but they don't match the same base. I hope that explains it.
> 
> Initially I thought about exposing the version of the main kernel in
> the kernel headers that can be later used in the module, but then I
> came across openvswitch and that's how I came up with the idea of an
> initial configure step.
> 
> But I totally understand if you think this is not worth it.

LTTng modules use the UTS_UBUNTU_RELEASE_ABI from the Ubuntu 
generated/utsrelease.h kernel headers to detect tracepoint 
instrumentation changes. I don't understand why many kernel flavors 
would have the same ABI number with different ABI semantics, but I guess 
that's just how things are now.

One way to solve this would be to detect the "-lowlatency" and "-kvm" 
suffixes in the string within generated/utsrelease.h UTS_RELEASE, e.g.:

#define UTS_RELEASE "5.15.0-76-lowlatency"

This could be done by LTTng modules by implementing a script similar to 
what we do for debian, fedora, rhel, and sle (see scripts/ in 
lttng-modules).

Then we could have:

* LTTNG_UBUNTU_KERNEL_RANGE for kernels where all flavors have the same
   kernel ABI.

* LTTNG_UBUNTU_GENERIC_KERNEL_RANGE for generic kernels only, for
   situations where the kernel ABI differ between flavors,

* LTTNG_UBUNTU_LOWLATENCY_KERNEL_RANGE for lowlatency kernels only, for
   situations where the kernel ABI differ between flavors,

* LTTNG_UBUNTU_KVM_KERNEL_RANGE for kvm kernels only, for situations
   where the kernel ABI differ between flavors.

It would all have been simpler if the UTS_UBUNTU_RELEASE_ABI would 
actually have been a versioned kernel ABI without different semantics 
across kernel flavors, but considering the current situation we will 
need to deal with this with scripts as we have done for other distributions.

Thanks,

Mathieu

> 
> All the best, Roxana
> 
> On 04/07/2023 20:07, Mathieu Desnoyers wrote:
>> On 7/4/23 11:35, Michael Jeanson via lttng-dev wrote:
>>> On 2023-07-03 14:28, Roxana Nicolescu via lttng-dev wrote:
>>>> This script described the changes in the linux kernel interface
>>>> that affect compatibility with lttng-modules.
>>>> 
>>>> It is introduced for a specific usecase where commit 
>>>> d87a7b4c77a9: "jbd2: use the correct print format" broke the
>>>> interface between the kernel and lttng-module. 3 variables 
>>>> changed their type to tid_t (transaction, head and tid) in
>>>> multiple function declarations. The lttng module was updated
>>>> properly to ensure backwards compatibility by using the version
>>>> of the kernel. But this change took into account only long term
>>>> supported versions. As an example, ubuntu 5.19 kernels picked
>>>> the linux kernel change from 5.15 without actually changing the
>>>> linux kernel upstream version. This means the current tooling
>>>> does not allow to fix the module for newer ubuntu 5.19
>>>> kernels.
>>>> 
>>>> This script is supposed to solve the problem mentioned above,
>>>> but to also make this change easier to integrate. We check the
>>>> linux kernel header (include/trace/events/jbd2.h) if the types
>>>> of tid, transaction and head variable have changed to tid_t
>>>> and define these 3 variables in 'include/generated/config.h': 
>>>> TID_IS_TID_T 1 TRANSACTION_IS_TID_T 1 HEAD_IS_TID_T 1
>>>> 
>>>> In 'include/instrumentation/events/jbd2.h' we then check these
>>>> to define the proper type of transaction, head and tid
>>>> variables that will be later used in the function declarations
>>>> that need them.
>>>> 
>>>> This change is meant to remove the dependency on linux kernel
>>>> version and the outcome is a bit cleaner that before. As with
>>>> the previous implementation, this may need changes in the 
>>>> future if the kernel interface changes again.
>>>> 
>>>> Note: This is a proposal for a simpler way of integrating linux
>>>> kernel changes in lttng-modules. The implementation is very
>>>> simple due to the fact that tid_t was introduced everywhere in
>>>> one commit in include/trace/events/jbd2.h. I would like to get
>>>> your opinion on this approach. If needed, it can be improved.
>>>> 
>>>> Roxana Nicolescu (1): Introduce configure script to describe
>>>> changes in linux kernel interface
>>>> 
>>>> README.md                             |   3 +- configure
>>>> |  36 +++++++++ include/instrumentation/events/jbd2.h | 110 
>>>> ++++++-------------------- 3 files changed, 61 insertions(+),
>>>> 88 deletions(-) create mode 100755 configure
>>>> 
>>> 
>>> Hi Roxana,
>>> 
>>> While I can see advantages to a configure script approach to
>>> detect kernel source changes I don't think it's worth the added
>>> complexity on top of our current kernel version range system.
>>> 
>>> We already have an Ubuntu specific kernel range macro that 
>>> supplements the upstream version with Ubuntu's kernel ABI
>>> number:
>>> 
>>> LTTNG_UBUNTU_KERNEL_RANGE(5,19,17,X, 6,0,0,0)
>>> 
>>> I'll let Mathieu make the final call but I think that would be
>>> the preferred approach.
>> 
>> Indeed, many of the kernel tracepoint code changes we had to deal
>> with in the past 10 years would not be easy to track with configure
>>  scripts, so we would end up with not just one, but with a
>> combination of two different mechanisms to adapt to kernel code
>> changes.
>> 
>> In order to keep things maintainable long-term, I prefer that we
>> stay with the version-based approach as recommended by Michael.
>> 
>> Thanks,
>> 
>> Mathieu
>> 
>>> Regards,
>>> 
>>> Michael _______________________________________________ lttng-dev
>>> mailing list lttng-dev@lists.lttng.org 
>>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

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