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@nec.com>
> wrote:
>
>> From: Kazuhito Hagio <k-hagio-ab@nec.com>
>>
>> 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