On 2023/05/15 12:07, lijiang wrote: > On Mon, May 15, 2023 at 9:51 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> > wrote: > >>> 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))) >> >> No, in the current crash, this will work. Because there is no >> if-conditional that checks a revision value over 255. >> >>> if (THIS_KERNEL_VERSION < LINUX(4,14,300)) >> >> But yes, when we need to check like this, the above cannot work. For >> example, THIS_KERNEL_VERSION on 4.14.310 equals to LINUX(4,14,300). >> >> > Thank you for the explanation, Kazu. > > >> So, my thought is that it would be better to change the shift amounts >> now for future changes with the small effort, rather than change it >> again in the future. >> >> > There is a similar discussion in the kernel here, but not sure why it was > not accepted. > https://lore.kernel.org/lkml/20210206035033.2036180-3-sashal@xxxxxxxxxx/ Thanks for the info. To me, it looks like (1) they changed the shift values but broke some userspace, so reverted. (2) the patch above was posted but not accepted because there was no user at that time. > > But anyway, it is just a small change, and also fine to me. So: Ack. Thanks, applied. https://github.com/crash-utility/crash/commit/040a56e9f9d0df15a2f8161ed3a0a907d70dda03 Kazu > > Thanks. > Lianbo > > > On Mon, May 15, 2023 at 9:51 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx <mailto:k-hagio-ab@xxxxxxx>> wrote: > > > 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))) > > No, in the current crash, this will work. Because there is no > if-conditional that checks a revision value over 255. > > > if (THIS_KERNEL_VERSION < LINUX(4,14,300)) > > But yes, when we need to check like this, the above cannot work. For > example, THIS_KERNEL_VERSION on 4.14.310 equals to LINUX(4,14,300). > > > Thank you for the explanation, Kazu. > > So, my thought is that it would be better to change the shift amounts > now for future changes with the small effort, rather than change it > again in the future. > > There is a similar discussion in the kernel here, but not sure why it was not accepted. > https://lore.kernel.org/lkml/20210206035033.2036180-3-sashal@xxxxxxxxxx/ <https://lore.kernel.org/lkml/20210206035033.2036180-3-sashal@xxxxxxxxxx/> > > But anyway, it is just a small change, and also fine to me. So: Ack. > > Thanks. > Lianbo -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
