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

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