Hi Jiri,
CC linux-xfs
On Fri, 31 Jan 2025 at 08:05, Jiri Slaby <jirislaby@xxxxxxxxxx> wrote:
> On 30. 01. 25, 21:14, David Laight wrote:
> > On Thu, 30 Jan 2025 18:43:17 +0000
> > Easwar Hariharan <eahariha@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> >> While converting users of msecs_to_jiffies(), lkp reported that some
> >> range checks would always be true because of the mismatch between the
> >> implied int value of secs_to_jiffies() vs the unsigned long
> >> return value of the msecs_to_jiffies() calls it was replacing. Fix this
> >> by casting secs_to_jiffies() values as unsigned long.
> >
> > Surely 'unsigned long' can't be the right type ?
> > It changes between 32bit and 64bit systems.
> > Either it is allowed to wrap - so should be 32bit on both,
> > or wrapping is unexpected and it needs to be 64bit on both.
>
> But jiffies are really ulong.
That's a good reason to make the change.
E.g. msecs_to_jiffies() does return unsigned long.
Note that this change may cause fall-out, e.g.
int val = 5.
pr_debug("timeout = %u jiffies\n", secs_to_jiffies(val));
^^
must be changed to %lu
More importantly, I doubt this change is guaranteed to fix the
reported issue. The code[*] in retry_timeout_seconds_store() does:
int val;
...
if (val < -1 || val > 86400)
return -EINVAL;
...
if (val != -1)
ASSERT(secs_to_jiffies(val) < LONG_MAX);
As HZ is a known (rather small) constant, and val is range-checked
before, the compiler can still devise that the condition is always true.
So I think that assertion should just be removed.
[*] Before commit b524e0335da22473 ("xfs: convert timeouts to
secs_to_jiffies()"), which was applied to the MM tree only 3
days ago, the code used msecs_to_jiffies() * MSEC_PER_SEC,
which is more complex than a simple multiplication, and harder for
the compiler to analyze statically, thus not triggering the warning
that easily...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
