Re: git: fd27f86dd71b - main - LinuxKPI: switch jiffies and timer->expire to unsigned long
Date: Tue, 07 Jan 2025 23:37:12 UTC
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. The real solution IMO is have a
native 64-bit tick counter that we can use directly in the linuxkpi
layer.
> #define jiffies_64 ticks
> -#define jiffies_to_msecs(x) ((unsigned int)(((int64_t)(int)(x)) * 1000 / hz))
> +#define jiffies_to_msecs(x) ((unsigned int)(((int64_t)(unsigned long)(x)) * 1000 / hz))
>
> #define MAX_JIFFY_OFFSET ((INT_MAX >> 1) - 1)
>
> @@ -68,7 +68,7 @@ extern uint64_t lkpi_msec2hz_rem;
> extern uint64_t lkpi_msec2hz_div;
> extern uint64_t lkpi_msec2hz_max;
>
> -static inline int
> +static inline unsigned long
> timespec_to_jiffies(const struct timespec *ts)
> {
> u64 result;
> @@ -78,10 +78,10 @@ timespec_to_jiffies(const struct timespec *ts)
> if (result > MAX_JIFFY_OFFSET)
> result = MAX_JIFFY_OFFSET;
>
> - return ((int)result);
> + return ((unsigned long)result);
> }
>
> -static inline int
> +static inline unsigned long
> msecs_to_jiffies(uint64_t msec)
> {
> uint64_t result;
> @@ -92,10 +92,10 @@ msecs_to_jiffies(uint64_t msec)
> if (result > MAX_JIFFY_OFFSET)
> result = MAX_JIFFY_OFFSET;
>
> - return ((int)result);
> + return ((unsigned long)result);
> }
>
> -static inline int
> +static inline unsigned long
> usecs_to_jiffies(uint64_t usec)
> {
> uint64_t result;
> @@ -106,7 +106,7 @@ usecs_to_jiffies(uint64_t usec)
> if (result > MAX_JIFFY_OFFSET)
> result = MAX_JIFFY_OFFSET;
>
> - return ((int)result);
> + return ((unsigned long)result);
> }
>
> static inline uint64_t
> @@ -133,17 +133,17 @@ nsecs_to_jiffies(uint64_t nsec)
> }
>
> static inline uint64_t
> -jiffies_to_nsecs(int j)
> +jiffies_to_nsecs(const unsigned long j)
> {
>
> - return ((1000000000ULL / hz) * (uint64_t)(unsigned int)j);
> + return ((1000000000ULL / hz) * (const uint64_t)j);
> }
>
> static inline uint64_t
> -jiffies_to_usecs(int j)
> +jiffies_to_usecs(const unsigned long j)
> {
>
> - return ((1000000ULL / hz) * (uint64_t)(unsigned int)j);
> + return ((1000000ULL / hz) * (const uint64_t)j);
> }
>
> static inline uint64_t
> @@ -153,10 +153,10 @@ get_jiffies_64(void)
> return ((uint64_t)(unsigned int)ticks);
> }
>
> -static inline int
> -linux_timer_jiffies_until(int expires)
> +static inline unsigned long
> +linux_timer_jiffies_until(unsigned long expires)
> {
> - int delta = expires - jiffies;
> + unsigned long delta = expires - jiffies;
> /* guard against already expired values */
> if (delta < 1)
> delta = 1;
> diff --git a/sys/compat/linuxkpi/common/include/linux/timer.h b/sys/compat/linuxkpi/common/include/linux/timer.h
> index 8bea082c3e6c..f9c76222795c 100644
> --- a/sys/compat/linuxkpi/common/include/linux/timer.h
> +++ b/sys/compat/linuxkpi/common/include/linux/timer.h
> @@ -42,7 +42,7 @@ struct timer_list {
> void (*function_415) (struct timer_list *);
> };
> unsigned long data;
> - int expires;
> + unsigned long expires;
> };
>
> extern unsigned long linux_timer_hz_mask;
> @@ -76,7 +76,7 @@ extern unsigned long linux_timer_hz_mask;
> callout_init(&(timer)->callout, 1); \
> } while (0)
>
> -extern int mod_timer(struct timer_list *, int);
> +extern int mod_timer(struct timer_list *, unsigned long);
> extern void add_timer(struct timer_list *);
> extern void add_timer_on(struct timer_list *, int cpu);
> extern int del_timer(struct timer_list *);
> diff --git a/sys/compat/linuxkpi/common/src/linux_compat.c b/sys/compat/linuxkpi/common/src/linux_compat.c
> index ec3ccb16b47d..35cb2fc2f3d7 100644
> --- a/sys/compat/linuxkpi/common/src/linux_compat.c
> +++ b/sys/compat/linuxkpi/common/src/linux_compat.c
> @@ -1938,7 +1938,7 @@ linux_timer_callback_wrapper(void *context)
> }
>
> int
> -mod_timer(struct timer_list *timer, int expires)
> +mod_timer(struct timer_list *timer, unsigned long expires)
> {
> int ret;
>