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

From: Mirsad Goran Todorovac
Date: Mon Apr 03 2023 - 03:33:28 EST


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

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