Re: [PATCH] Fix kernel version macros for revision numbers over 255

[lijiang <lijiang@xxxxxxxxxx>]

On Thu, May 11, 2023 at 8:38 AM HAGIO KAZUHITO(萩尾　一仁) <k-hagio-ab@xxxxxxx> wrote:

On 2023/05/10 18:03, lijiang wrote:
> Hi, Kazu
> Thank you for the patch.
> On Wed, May 10, 2023 at 3:09 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx>
> wrote:
>
>> From: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
>>
>> The current comparison macros for kernel version shift minor number only
>> 8 bits.  This can cause an unexpected result on kernels with revision
>> number over 255, e.g. Linux 4.14.314.
>>
>>
> For this case, I saw kernel deal with it as below:
>
> #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + ((c) > 255 ?
> 255 : (c)))
>
> Can you try to imitate the above macro definition and help to confirm if it
> can also work for your case? If yes, it should be good to follow up kernel
> change.

um, if we need to check a revision number over 255, the KERNEL_VERSION
macro will not work.  For example,

 

Do you mean the following changes can not work?

#define THIS_KERNEL_VERSION ((kt->kernel_version[0] << 16) + \
                             (kt->kernel_version[1] << 8) + \
                             ((kt->kernel_version[2] > 255) ? 255 : (kt->kernel_version[2])))
#define LINUX(x,y,z) (((uint)(x) << 16) + ((uint)(y) << 8) + ((uint)(z) > 255 ? 255 : (uint)(z)))

   if (THIS_KERNEL_VERSION < LINUX(4,14,300))

and yet, why do you think it is good to follow the kernel one?

I saw that similar cases were handled in the kernel, and the kernel did not change the bit shift operations.

... 

Makefile:1334: echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) +  \

        ((c) > 255 ? 255 : (c)))';                                       \

tools/lib/bpf/bpf_helpers.h:74:#define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + ((c) > 255 ? 255 : (c)))

... 

Thanks.

Lianbo

(Although I don't know the purpose and usage of the macro in kernel,
probably it's enough for kernel or tools.)

btw, makedumpfile has the same shift values as the patch since 2007.

#define KVER_MAJ_SHIFT 24
#define KVER_MIN_SHIFT 16
#define KERNEL_VERSION(x,y,z) (((x) << KVER_MAJ_SHIFT) | ((y) << KVER_MIN_SHIFT) | (z))

Thanks,
Kazu

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki