[BUG] Problem with automatic kernel numbering

[BUG] Problem with automatic kernel numbering

[Mirsad Goran Todorovac]

Hi, Mr. Bagas, Sir!

I am talking about a problem with the CONFIG_LOCALVERSION_AUTO=y feature.

I thought of a way to make an exact account of which patches were applied in a build
i.e. adding patch checksum to 6.3.0-rc4-00034-gfcd476ea6a88-dirty, for currently the
command

# rpm -ivh --oldpackage <kernelname>-<build-no>.rpm

install the kernels

kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-24.x86_64.rpm
kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-25.x86_64.rpm
kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-26.x86_64.rpm

all overlapping (apparently everything after '-' [minus] sign is discarded,
so one has to reboot to another kernel, i.e. 6.1.15, remove the offending kernel,
and then install the new one in the sequence of testing.
The CONFIG_LOCALVERSION_AUTO=y rpm build script might add something that rpm
command sees in the install process so the files do not overlap (as kernel names
are being truncated at '-' sign).

A smaller hash of the applied patches would suffice, considering the limit
of 64 chars. Or using an underscore '_' instead of minus '-', so the rpm
installer doesn't treat them as the same version of kernel.

Is this a violation of the build process?

It would be time and energy efficient, for changing the .config and
CONFIG_LOCALVERSION causes much greater recompilation and touches more dependencies.

Optionally, a /proc/<applied-patches-to-build> or something like that could be
added to the running kernel, much like i.e. TuxCare has kcarectl --patch-info
for live patches?

Thank you very much for considering this problem report.

Kind regards,
Mirsad


-- 
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
 
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
The European Union

"I see something approaching fast ... Will it be friends with me?"

Re: [BUG] Problem with automatic kernel numbering

[Bagas Sanjaya]

On 4/1/23 18:54, Mirsad Goran Todorovac wrote:
> I am talking about a problem with the CONFIG_LOCALVERSION_AUTO=y feature.
> 
> I thought of a way to make an exact account of which patches were applied in a build
> i.e. adding patch checksum to 6.3.0-rc4-00034-gfcd476ea6a88-dirty, for currently the
> command
> 
> # rpm -ivh --oldpackage <kernelname>-<build-no>.rpm
> 
> install the kernels
> 
> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-24.x86_64.rpm
> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-25.x86_64.rpm
> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-26.x86_64.rpm
> 
First, Cc'ing Masahiro.

I think applying patches with `git am` should change the `git describe`
part of kernel version name. However, in this case, you have uncommitted
changes in your tree when building.

> all overlapping (apparently everything after '-' [minus] sign is discarded,
> so one has to reboot to another kernel, i.e. 6.1.15, remove the offending kernel,
> and then install the new one in the sequence of testing.
> The CONFIG_LOCALVERSION_AUTO=y rpm build script might add something that rpm
> command sees in the install process so the files do not overlap (as kernel names
> are being truncated at '-' sign).
> 

Patch number truncated?

> A smaller hash of the applied patches would suffice, considering the limit
> of 64 chars. Or using an underscore '_' instead of minus '-', so the rpm
> installer doesn't treat them as the same version of kernel.
> 

12 chars is minimum abbreviated hash length for Linux kernel, so it is
already sufficient. Personally, I bump to 14 chars to give more headroom in
case 12 chars give 50% collision in the (hopefully distant) future.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Re: [BUG] [RFC] Problem with automatic kernel numbering

[Mirsad Goran Todorovac]

On 2.4.2023. 15:23, Bagas Sanjaya wrote:
> On 4/1/23 18:54, Mirsad Goran Todorovac wrote:
>> I am talking about a problem with the CONFIG_LOCALVERSION_AUTO=y feature.
>>
>> I thought of a way to make an exact account of which patches were applied in a build
>> i.e. adding patch checksum to 6.3.0-rc4-00034-gfcd476ea6a88-dirty, for currently the
>> command
>>
>> # rpm -ivh --oldpackage <kernelname>-<build-no>.rpm
>>
>> install the kernels
>>
>> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-24.x86_64.rpm
>> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-25.x86_64.rpm
>> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-26.x86_64.rpm
>>
> First, Cc'ing Masahiro.

Thank you, Sir.

> I think applying patches with `git am` should change the `git describe`
> part of kernel version name. However, in this case, you have uncommitted
> changes in your tree when building.

Yes, this does create a unique commit hash. This apparently solves the problem.
Thanks for the hint.

>> all overlapping (apparently everything after '-' [minus] sign is discarded,
>> so one has to reboot to another kernel, i.e. 6.1.15, remove the offending kernel,
>> and then install the new one in the sequence of testing.
>> The CONFIG_LOCALVERSION_AUTO=y rpm build script might add something that rpm
>> command sees in the install process so the files do not overlap (as kernel names
>> are being truncated at '-' sign).

> Patch number truncated?

Indeed.

All of the

 >> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-24.x86_64.rpm
 >> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-25.x86_64.rpm
 >> kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty-26.x86_64.rpm

were treated like "kernel-6.3.0_rc4mt+20230330_00051_g8bb95a1662f8_dirty" by
the `rpm -ivh --oldpackage <package>.rpm` command.

This cause install collisions in filenames (thankfully, it did not break the
system, only prevented install until reboot in the third kernel, uninstall old
and install new - however, this is error prone and clumsy).

>> A smaller hash of the applied patches would suffice, considering the limit
>> of 64 chars. Or using an underscore '_' instead of minus '-', so the rpm
>> installer doesn't treat them as the same version of kernel.
> 
> 12 chars is minimum abbreviated hash length for Linux kernel, so it is
> already sufficient. Personally, I bump to 14 chars to give more headroom in
> case 12 chars give 50% collision in the (hopefully distant) future.

This is not a problem with the 12-chars truncated SHA-1 hash, I suppose, but
truncating "-24", "-25" and "-26" from the build id.

> Thanks.

Not at all, Sir.

It would be very useful if the kernel gave i.e. in /proc/applied-patches/*
the list of patches applied and against which build tree. If that is possible.
Do I make any sense?

Possibly, git may not be that smart to distinguish patches i.e. from the torvalds
from those manually applied with `git am`?

This might look like this:

# cat /proc/applied-patches/list
10de4cefccf7 (HEAD -> master) memstick: fix memory leak if card device is never registered
feeedf59897c platform/x86: think-lmi: Clean up display of current_value on Thinkstation
86cebdbfb8d2 platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings
ff9de97baa02 The command allocated to set exit latency LPM values need to be freed in case the command is never queued. This would 
be the case if there is no change in exit latency values, or device is missing.
2ac6d07f1a81 platform/x86: think-lmi: Fix memory leak when showing current settings
# cat /proc/applied-patches/2ac6d07f1a81
commit 2ac6d07f1a813facd03a0c011a48b317ed9f4654
Author: Armin Wolf <W_Armin@gmx.de>
Date:   Fri Mar 31 23:33:19 2023 +0200

     platform/x86: think-lmi: Fix memory leak when showing current settings

     When retriving a item string with tlmi_setting(), the result has to be
     freed using kfree(). In current_value_show() however, malformed
     item strings are not freed, causing a memory leak.
     Fix this by eliminating the early return responsible for this.

     Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
     Link: https://lore.kernel.org/platform-driver-x86/01e920bc-5882-ba0c-dd15-868bf0eca0b8@alu.unizg.hr/T/#t
     Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
     Fixes: 0fdf10e5fc96 ("platform/x86: think-lmi: Split current_value to reflect only the value")
     Signed-off-by: Armin Wolf <W_Armin@gmx.de>

However, I can't seem to find a git diff command to give me difference from the main branch?

Actually, I can, with some Googling:

mtodorov@domac:~/linux/kernel/linux_torvalds$ git log --oneline origin/master..master
10de4cefccf7 (HEAD -> master) memstick: fix memory leak if card device is never registered
feeedf59897c platform/x86: think-lmi: Clean up display of current_value on Thinkstation
86cebdbfb8d2 platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings
ff9de97baa02 The command allocated to set exit latency LPM values need to be freed in case the command is never queued. This would 
be the case if there is no change in exit latency values, or device is missing.
2ac6d07f1a81 platform/x86: think-lmi: Fix memory leak when showing current settings
mtodorov@domac:~/linux/kernel/linux_torvalds$

So, git knows the list of applied patches against the vanilla tree.

What would be handy for forensics would be to that that listed in /proc/applied-patches/list
or something, just like /boot/config-$(uname -r) is helpful.

However, this might not be so useful. Using "git am" solves the majority of the problem.

There is a warning in the kernel docs that the newbie developers have ideas that are
sometimes rejected.

I should do a lot of homework before making such suggestions :-/

Best regards,
Mirsad

-- 
Mirsad Todorovac
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb
Republic of Croatia, the European Union

Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu