Re: git: fd27f86dd71b - main - LinuxKPI: switch jiffies and timer->expire to unsigned long

From: Bjoern A. Zeeb <bz_at_freebsd.org>
Date: Wed, 08 Jan 2025 07:58:05 UTC
On Tue, 7 Jan 2025, Mark Johnston wrote:

> On Tue, Jan 07, 2025 at 10:47:54PM +0000, Bjoern A. Zeeb wrote:
>> The branch main has been updated by bz:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=fd27f86dd71b7ff1df6981297095b88d1d29652e
>>
>> commit fd27f86dd71b7ff1df6981297095b88d1d29652e
>> Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
>> AuthorDate: 2024-12-28 09:57:56 +0000
>> Commit:     Bjoern A. Zeeb <bz@FreeBSD.org>
>> CommitDate: 2025-01-07 20:00:20 +0000
>>
>>     LinuxKPI: switch jiffies and timer->expire to unsigned long
>>
>>     It seems these functions work with unsigned long and not int in Linux.
>>     Start simply replacing the int where I came across it while debugging
>>     a wireless driver timer modification.  Also sprinkle in some "const".
>>
>>     Sponsored by:   The FreeBSD Foundation
>>     MFC after:      2 weeks
>>     Reviewed by:    emaste
>>     Differential Revision: https://reviews.freebsd.org/D48318
>> ---
>>  sys/compat/linuxkpi/common/include/linux/jiffies.h | 28 +++++++++++-----------
>>  sys/compat/linuxkpi/common/include/linux/timer.h   |  4 ++--
>>  sys/compat/linuxkpi/common/src/linux_compat.c      |  2 +-
>>  3 files changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/sys/compat/linuxkpi/common/include/linux/jiffies.h b/sys/compat/linuxkpi/common/include/linux/jiffies.h
>> index bd05a0db0767..8346e74fb830 100644
>> --- a/sys/compat/linuxkpi/common/include/linux/jiffies.h
>> +++ b/sys/compat/linuxkpi/common/include/linux/jiffies.h
>> @@ -38,7 +38,7 @@
>>
>>  #define	jiffies			ticks
>
> There is a fundamental incompatibility here: jiffies is an unsigned long
> but ticks is an int.  Historically that was the source of some very
> painful-to-find bugs in the IB stack, since the difference mostly arises
> when one has to deal with ticks rollover, a rare event.
>
> It doesn't make a lot of sense to me to partially convert some routines
> to using unsigned long if we're not going to do it everywhere,
> especially if there isn't a concrete bug being fixed.  With this diff,
> jiffies is still an int, and various macros like time_after() still cast
> their result to a 32-bit value, so at a glance it seems incomplete.  I
> also suspect that the delta < 1 check in linux_timer_jiffies_until() is
> now broken.
>
> I'd advise against a change like this unless you're very confident in
> it: it's easy to introduce rare bugs.

If I'll back it out (and deal with the conversion elsewhere as needed)
would you write a follow-up comment of your knowledge and add it to
jiffies.h so we can avoid this in the future?


>  The real solution IMO is have a
> native 64-bit tick counter that we can use directly in the linuxkpi
> layer.



-- 
Bjoern A. Zeeb                                                     r15:7