Re: [PATCH v3 2/7] drm: Add drm_timeout_rel_to_jiffies()

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 28.09.26 um 20:09 schrieb Maíra Canal:
> Hi Thomas,
>
> Thank you for your review!
>
> On 28/09/26 13:37, Thomas Zimmermann wrote:
>
> [...]
>
>>> +unsigned long drm_timeout_rel_to_jiffies(u64 timeout_nsec)
>>> +{
>>> +    u64 secs;
>>> +    u32 rem;
>>> +
>>> +    /* Make 0 timeout means poll, as for the absolute variant. */
>>> +    if (timeout_nsec == 0)
>>> +        return 0;
>>> +
>>> +    /*
>>> +     * As nsecs_to_jiffies64() does not guard against overflow, split
>>> +     * the timeout into whole seconds and nanoseconds. This way
>>> +     * nsecs_to_jiffies64() is always handed a value below a second.
>>> +     */
>>> +    secs = div_u64_rem(timeout_nsec, NSEC_PER_SEC, &rem);
>>> +    if (secs >= MAX_JIFFY_OFFSET / HZ)
>>> +        return MAX_JIFFY_OFFSET;
>>> +
>>> +    return min_t(u64, MAX_JIFFY_OFFSET,
>>> +             secs_to_jiffies(secs) + nsecs_to_jiffies64(rem) + 1);
>>
>> The timeout is controled by user space, right? Can these additions 
>> overflow?  I see that secs is tested against MAX_JIFFY_OFFSET, but is 
>> that sufficient?
>
> Although the timeout is controlled by user-space, the additions cannot
> overflow here. The secs test bounds the multiplication, which is why it
> is written as secs >= MAX_JIFFY_OFFSET / HZ rather than secs * HZ >=
> MAX_JIFFY_OFFSET (which could overflow).
>
> Past that check, we know that the remaining is smaller than one second
> and therefore, nsecs_to_jiffies64(rem) <= HZ - 1. So, the sum + 1 is at
> most MAX_JIFFY_OFFSET.
>
>>
>> BTW there was this NSEC % HZ test in the original code? What was it 
>> good for? It is no longer useful?
>>
>
> I believe that NSEC_PER_SEC % HZ was only useful to check if it was a
> plain division, which wouldn't overflow. With the current approach, I
> believe it's no longer needed.

Thanks for answering my questions.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas


>
> Best regards,
> - Maíra
>> Best regards
>> Thomas
>>
>>
>>> +}
>>> +EXPORT_SYMBOL(drm_timeout_rel_to_jiffies);
>>> diff --git a/include/drm/drm_timeout.h b/include/drm/drm_timeout.h
>>> index cd9621c52062..6ee222a3e97c 100644
>>> --- a/include/drm/drm_timeout.h
>>> +++ b/include/drm/drm_timeout.h
>>> @@ -12,5 +12,6 @@
>>>   #include <linux/types.h>
>>>   signed long drm_timeout_abs_to_jiffies(s64 timeout_nsec);
>>> +unsigned long drm_timeout_rel_to_jiffies(u64 timeout_nsec);
>>>   #endif
>>>
>>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)