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

[Michael Jeanson via lttng-dev <lttng-dev@lists.lttng.org>]

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.

Regards,

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