[PATCH 0/6] drm: Consolidate the wait timeout conversion helpers

[PATCH 0/6] drm: Consolidate the wait timeout conversion helpers

[Maíra Canal]

DRM wait ioctls express their timeout in one of two ways: as an absolute
deadline in ns, or as a duration in ns. The core has a helper for the first
form, drm_timeout_abs_to_jiffies(), which panfrost, lima, tegra, xe and
some accel drivers use. However, there is no equivalent for the second
form, so the drivers that take a duration convert it themselves.

For example, i915 has a local helper to handle the relative timeout case,
and v3d carries a copy of it. This series moves that conversion into a DRM
helper called drm_timeout_rel_to_jiffies(), next to the absolute variant,
and converts the drivers that were open-coding either form.

This series becomes particularly relevant as the new helper will be used
in the next version of the series "drm/vc4: Switch to DRM GPU scheduler"
[1]. With this series, we avoid another duplication of the
nsecs_to_jiffies_timeout() function (and thanks Tvrtko for the suggestion).

PATCH 1 moves drm_timeout_abs_to_jiffies() out of drm_syncobj.c and into a
file of its own, so that both helpers live together. PATCH 2 adds a helper
for the relative variant. Patches 3 to 6 convert amdgpu, v3d, i915 and vc4.

Let me know your thoughts about it.

[1] https://lore.kernel.org/dri-devel/20260712-vc4-drm-scheduler-v3-0-ed6dc4defa53@igalia.com/T/

Best regards,
- Maíra

---
Maíra Canal (6):
      drm: Move drm_timeout_abs_to_jiffies() to drm_timeout.c
      drm: Add drm_timeout_rel_to_jiffies()
      drm/amdgpu: Use drm_timeout_abs_to_jiffies()
      drm/v3d: Use drm_timeout_rel_to_jiffies()
      drm/i915: Use drm_timeout_rel_to_jiffies()
      drm/vc4: Use drm_timeout_rel_to_jiffies()

 drivers/gpu/drm/Makefile                 |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  | 16 +------
 drivers/gpu/drm/drm_syncobj.c            | 33 --------------
 drivers/gpu/drm/drm_timeout.c            | 76 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gem/i915_gem_wait.c | 17 ++-----
 drivers/gpu/drm/v3d/v3d_bo.c             |  3 +-
 drivers/gpu/drm/v3d/v3d_drv.h            | 10 -----
 drivers/gpu/drm/vc4/vc4_gem.c            |  3 +-
 include/drm/drm_utils.h                  |  1 +
 9 files changed, 87 insertions(+), 73 deletions(-)
---
base-commit: dc2f9f7fed1a8ea5290f9f60c6310d497e85e666
change-id: 20260809-drm-timeout-helpers-55473e4879d1

[PATCH 1/6] drm: Move drm_timeout_abs_to_jiffies() to drm_timeout.c

[Maíra Canal]

drm_timeout_abs_to_jiffies() began as a static helper inside the syncobj
wait implementation and was later exported in place, so that it could be
called by multiple drivers (e.g. panfrost, lima, tegra and accel
drivers). All of them reach it through drm_utils.h.

Give it a file of its own, so that timeout conversion helpers have a
home, which will be useful when we add new timeout handlers. The
declaration stays in drm_utils.h, so no caller changes.

No functional change.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/Makefile      |  1 +
 drivers/gpu/drm/drm_syncobj.c | 33 ------------------------------
 drivers/gpu/drm/drm_timeout.c | 47 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd783..0a0d7ea08347 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -68,6 +68,7 @@ drm-y := \
 	drm_rect.o \
 	drm_syncobj.o \
 	drm_sysfs.o \
+	drm_timeout.o \
 	drm_trace_points.o \
 	drm_vblank.o \
 	drm_vblank_work.o \
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 2fa170a29a62..cf03681de6f7 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1193,39 +1193,6 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
 	return timeout;
 }
 
-/**
- * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute value
- *
- * @timeout_nsec: timeout nsec component in ns, 0 for poll
- *
- * Calculate the timeout in jiffies from an absolute time in sec/nsec.
- */
-signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
-{
-	ktime_t abs_timeout, now;
-	u64 timeout_ns, timeout_jiffies64;
-
-	/* make 0 timeout means poll - absolute 0 doesn't seem valid */
-	if (timeout_nsec == 0)
-		return 0;
-
-	abs_timeout = ns_to_ktime(timeout_nsec);
-	now = ktime_get();
-
-	if (!ktime_after(abs_timeout, now))
-		return 0;
-
-	timeout_ns = ktime_to_ns(ktime_sub(abs_timeout, now));
-
-	timeout_jiffies64 = nsecs_to_jiffies64(timeout_ns);
-	/*  clamp timeout to avoid infinite timeout */
-	if (timeout_jiffies64 >= MAX_SCHEDULE_TIMEOUT - 1)
-		return MAX_SCHEDULE_TIMEOUT - 1;
-
-	return timeout_jiffies64 + 1;
-}
-EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);
-
 static int drm_syncobj_array_wait(struct drm_device *dev,
 				  struct drm_file *file_private,
 				  struct drm_syncobj_wait *wait,
diff --git a/drivers/gpu/drm/drm_timeout.c b/drivers/gpu/drm/drm_timeout.c
new file mode 100644
index 000000000000..78e9f65e5477
--- /dev/null
+++ b/drivers/gpu/drm/drm_timeout.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Timeout conversion helpers for wait ioctls.
+ *
+ * Copyright 2017 Red Hat
+ * Copyright 2016 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/export.h>
+#include <linux/jiffies.h>
+#include <linux/ktime.h>
+#include <linux/sched.h>
+
+#include <drm/drm_utils.h>
+
+/**
+ * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute value
+ *
+ * @timeout_nsec: timeout nsec component in ns, 0 for poll
+ *
+ * Calculate the timeout in jiffies from an absolute time in sec/nsec.
+ */
+signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
+{
+	ktime_t abs_timeout, now;
+	u64 timeout_ns, timeout_jiffies64;
+
+	/* make 0 timeout means poll - absolute 0 doesn't seem valid */
+	if (timeout_nsec == 0)
+		return 0;
+
+	abs_timeout = ns_to_ktime(timeout_nsec);
+	now = ktime_get();
+
+	if (!ktime_after(abs_timeout, now))
+		return 0;
+
+	timeout_ns = ktime_to_ns(ktime_sub(abs_timeout, now));
+
+	timeout_jiffies64 = nsecs_to_jiffies64(timeout_ns);
+	/*  clamp timeout to avoid infinite timeout */
+	if (timeout_jiffies64 >= MAX_SCHEDULE_TIMEOUT - 1)
+		return MAX_SCHEDULE_TIMEOUT - 1;
+
+	return timeout_jiffies64 + 1;
+}
+EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);

-- 
2.55.0

[PATCH 2/6] drm: Add drm_timeout_rel_to_jiffies()

[Maíra Canal]

drm_timeout_abs_to_jiffies() covers the drivers whose wait UAPI takes an
absolute deadline, but there is no equivalent for the drivers that
express a wait as a duration. Drivers such as i915 and v3d convert the
value themselves.

Converting a nanosecond duration to jiffies needs some care.
nsecs_to_jiffies() returns unsigned long, so on 32-bit a large
userspace-supplied timeout overflows its range and is silently truncated.

i915 already handles both cases in a local helper, which v3d has a copy
of. Add the same conversion to the core, so that it is available to any
driver and both copies can be dropped.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_timeout.c | 29 +++++++++++++++++++++++++++++
 include/drm/drm_utils.h       |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/drm_timeout.c b/drivers/gpu/drm/drm_timeout.c
index 78e9f65e5477..30ad3e8ebc92 100644
--- a/drivers/gpu/drm/drm_timeout.c
+++ b/drivers/gpu/drm/drm_timeout.c
@@ -9,6 +9,7 @@
 #include <linux/export.h>
 #include <linux/jiffies.h>
 #include <linux/ktime.h>
+#include <linux/math64.h>
 #include <linux/sched.h>
 
 #include <drm/drm_utils.h>
@@ -45,3 +46,31 @@ signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
 	return timeout_jiffies64 + 1;
 }
 EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);
+
+/**
+ * drm_timeout_rel_to_jiffies - calculate jiffies timeout from relative value
+ *
+ * @timeout_nsec: relative timeout in ns, 0 for poll
+ *
+ * Calculate the timeout in jiffies from a relative timeout in ns, for drivers
+ * whose UAPI expresses a wait as a duration rather than as a deadline.
+ *
+ * The result is clamped to MAX_JIFFY_OFFSET. That keeps it positive once it is
+ * converted to the signed long taken by dma_fence_wait_timeout() and friends,
+ * which matters on 32-bit, and keeps it distinct from MAX_SCHEDULE_TIMEOUT so
+ * that a finite wait is never understood as an infinite one.
+ */
+unsigned long drm_timeout_rel_to_jiffies(u64 timeout_nsec)
+{
+	/* make 0 timeout means poll, as for the absolute variant */
+	if (timeout_nsec == 0)
+		return 0;
+
+	/* nsecs_to_jiffies64() does not guard against overflow */
+	if ((NSEC_PER_SEC % HZ) != 0 &&
+	    div_u64(timeout_nsec, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
+		return MAX_JIFFY_OFFSET;
+
+	return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(timeout_nsec) + 1);
+}
+EXPORT_SYMBOL(drm_timeout_rel_to_jiffies);
diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h
index 6a46f755daba..8c0cc9835413 100644
--- a/include/drm/drm_utils.h
+++ b/include/drm/drm_utils.h
@@ -25,5 +25,6 @@ const struct drm_panel_backlight_quirk *
 drm_get_panel_backlight_quirk(const struct drm_edid *edid);
 
 signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec);
+unsigned long drm_timeout_rel_to_jiffies(u64 timeout_nsec);
 
 #endif

-- 
2.55.0

[PATCH 3/6] drm/amdgpu: Use drm_timeout_abs_to_jiffies()

[Maíra Canal]

amdgpu_gem_timeout() converts an absolute deadline in ns into jiffies,
which is what drm_timeout_abs_to_jiffies() already does for the other
drivers whose wait UAPI takes a deadline. Use the shared helper and keep
only the part that is specific to amdgpu.

Two details change along this conversion: the helper rounds up rather than
truncating, so a deadline less than a tick away now waits for one jiffy
instead of returning 0. It also uses nsecs_to_jiffies64(), so the
conversion no longer truncates on 32-bit, where a large deadline could
previously be reduced to an arbitrary shorter one.

Signed-off-by: Maíra Canal <mcanal@igalia.com>

---

As a note, this patch can be merged independently to the AMD tree
without any dependencies.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 6a0699746fbc..84b509a484b0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -25,7 +25,6 @@
  *          Alex Deucher
  *          Jerome Glisse
  */
-#include <linux/ktime.h>
 #include <linux/module.h>
 #include <linux/overflow.h>
 #include <linux/pagemap.h>
@@ -40,6 +39,7 @@
 #include <drm/drm_gem_ttm_helper.h>
 #include <drm/ttm/ttm_tt.h>
 #include <drm/drm_syncobj.h>
+#include <drm/drm_utils.h>
 
 #include "amdgpu.h"
 #include "amdgpu_display.h"
@@ -622,23 +622,11 @@ int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
  */
 unsigned long amdgpu_gem_timeout(uint64_t timeout_ns)
 {
-	unsigned long timeout_jiffies;
-	ktime_t timeout;
-
 	/* clamp timeout if it's to large */
 	if (((int64_t)timeout_ns) < 0)
 		return MAX_SCHEDULE_TIMEOUT;
 
-	timeout = ktime_sub(ns_to_ktime(timeout_ns), ktime_get());
-	if (ktime_to_ns(timeout) < 0)
-		return 0;
-
-	timeout_jiffies = nsecs_to_jiffies(ktime_to_ns(timeout));
-	/*  clamp timeout to avoid unsigned-> signed overflow */
-	if (timeout_jiffies > MAX_SCHEDULE_TIMEOUT)
-		return MAX_SCHEDULE_TIMEOUT - 1;
-
-	return timeout_jiffies;
+	return drm_timeout_abs_to_jiffies(timeout_ns);
 }
 
 int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,

-- 
2.55.0

[PATCH 4/6] drm/v3d: Use drm_timeout_rel_to_jiffies()

[Maíra Canal]

v3d carried an exact copy of i915's nsecs_to_jiffies_timeout(). Now
that the conversion lives in "drm_timeout.c", drop the copy and use it.

The core helper also returns 0 for a zero timeout. i915 has that check in
to_wait_timeout(), but v3d only ever copied the conversion, so
DRM_IOCTL_V3D_WAIT_BO with timeout_ns == 0 was turned into a one jiffy
wait and could block for a tick before reporting -ETIME. It now returns
without waiting.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_bo.c  |  3 ++-
 drivers/gpu/drm/v3d/v3d_drv.h | 10 ----------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index a847d2f0ccf5..919bdff1036e 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -19,6 +19,7 @@
 #include <linux/vmalloc.h>
 
 #include <drm/drm_print.h>
+#include <drm/drm_utils.h>
 
 #include "v3d_drv.h"
 #include "uapi/drm/v3d_drm.h"
@@ -281,7 +282,7 @@ v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
 	ktime_t start = ktime_get();
 	u64 delta_ns;
 	unsigned long timeout_jiffies =
-		nsecs_to_jiffies_timeout(args->timeout_ns);
+		drm_timeout_rel_to_jiffies(args->timeout_ns);
 
 	if (args->pad != 0)
 		return -EINVAL;
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 8c46ed09f5c4..595c61a955e8 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -578,16 +578,6 @@ struct v3d_submit_ext {
 						   (Wmax))
 #define wait_for(COND, MS)		_wait_for((COND), (MS) * 1000, 10, 1000)
 
-static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
-{
-	/* nsecs_to_jiffies64() does not guard against overflow */
-	if ((NSEC_PER_SEC % HZ) != 0 &&
-	    div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
-		return MAX_JIFFY_OFFSET;
-
-	return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
-}
-
 /* v3d_bo.c */
 struct drm_gem_object *v3d_create_object(struct drm_device *dev, size_t size);
 void v3d_free_object(struct drm_gem_object *gem_obj);

-- 
2.55.0

[PATCH 5/6] drm/i915: Use drm_timeout_rel_to_jiffies()

[Maíra Canal]

nsecs_to_jiffies_timeout() originated here and has since been copied into
another driver. It is now available in "drm_timeout.c", so use it and drop
the local definition.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_wait.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
index 2893df65c359..1c189d4d7ad8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
@@ -7,6 +7,8 @@
 #include <linux/dma-fence-chain.h>
 #include <linux/jiffies.h>
 
+#include <drm/drm_utils.h>
+
 #include "gt/intel_engine.h"
 #include "gt/intel_rps.h"
 
@@ -184,25 +186,12 @@ i915_gem_object_wait(struct drm_i915_gem_object *obj,
 	return !timeout ? -ETIME : 0;
 }
 
-static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
-{
-	/* nsecs_to_jiffies64() does not guard against overflow */
-	if ((NSEC_PER_SEC % HZ) != 0 &&
-	    div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
-		return MAX_JIFFY_OFFSET;
-
-	return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
-}
-
 static unsigned long to_wait_timeout(s64 timeout_ns)
 {
 	if (timeout_ns < 0)
 		return MAX_SCHEDULE_TIMEOUT;
 
-	if (timeout_ns == 0)
-		return 0;
-
-	return nsecs_to_jiffies_timeout(timeout_ns);
+	return drm_timeout_rel_to_jiffies(timeout_ns);
 }
 
 /**

-- 
2.55.0

[PATCH 6/6] drm/vc4: Use drm_timeout_rel_to_jiffies()

[Maíra Canal]

vc4_wait_bo_ioctl() converts its timeout with usecs_to_jiffies(), which
takes an unsigned int. The microsecond value derived from the u64
timeout_ns is narrowed to fit, so any timeout above UINT_MAX microseconds,
around ~71 minutes, wraps into a much shorter one. Unlike the equivalent
problem in the other drivers, this one is not limited to 32-bit.

Use the shared helper, which takes the duration in ns and clamps the
result to MAX_JIFFY_OFFSET.

Fixes: 21bd85f82ec2 ("drm/vc4: Use DMA Resv to implement VC4 wait BO IOCTL")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index e231c906709c..781aeb8967b1 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -32,6 +32,7 @@
 #include <drm/drm_exec.h>
 #include <drm/drm_print.h>
 #include <drm/drm_syncobj.h>
+#include <drm/drm_utils.h>
 
 #include "vc4_drv.h"
 #include "vc4_regs.h"
@@ -970,7 +971,7 @@ vc4_wait_bo_ioctl(struct drm_device *dev, void *data,
 	int ret;
 	struct drm_vc4_wait_bo *args = data;
 	unsigned long timeout_jiffies =
-		usecs_to_jiffies(div_u64(args->timeout_ns, 1000));
+		drm_timeout_rel_to_jiffies(args->timeout_ns);
 	ktime_t start = ktime_get();
 	u64 delta_ns;
 

-- 
2.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The overflow guard in `drm_timeout_rel_to_jiffies()` is ineffective on 64-bit architectures, failing to prevent internal `u64` wrap-around in `nsecs_to_jiffies64()` for specific inputs.
--

--- Patch [2]: [PATCH 2/6] drm: Add drm_timeout_rel_to_jiffies() ---
commit c015d0990b1688d1fc81fe1000d64e7430d1d234
Author: Maíra Canal <mcanal@igalia.com>

drm: Add drm_timeout_rel_to_jiffies()

This commit introduces drm_timeout_rel_to_jiffies() to convert relative 
nanosecond timeouts into jiffies. It is intended for drivers whose wait UAPI 
takes a duration instead of an absolute deadline, and includes an overflow 
check to safely handle large values.

> diff --git a/drivers/gpu/drm/drm_timeout.c b/drivers/gpu/drm/drm_timeout.c
> index 78e9f65e54779..30ad3e8ebc92a 100644
> --- a/drivers/gpu/drm/drm_timeout.c
> +++ b/drivers/gpu/drm/drm_timeout.c
[ ... ]
> @@ -45,3 +46,31 @@ signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
>  	return timeout_jiffies64 + 1;
>  }
>  EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);
> +
> +/**
> + * drm_timeout_rel_to_jiffies - calculate jiffies timeout from relative value
[ ... ]
> +unsigned long drm_timeout_rel_to_jiffies(u64 timeout_nsec)
> +{
> +	/* make 0 timeout means poll, as for the absolute variant */
> +	if (timeout_nsec == 0)
> +		return 0;
> +
> +	/* nsecs_to_jiffies64() does not guard against overflow */
> +	if ((NSEC_PER_SEC % HZ) != 0 &&
> +	    div_u64(timeout_nsec, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
> +		return MAX_JIFFY_OFFSET;

[Severity: Medium]
Is this overflow guard effective on 64-bit architectures?

On 64-bit systems, MAX_JIFFY_OFFSET is around ~4.6e18, so MAX_JIFFY_OFFSET / HZ
is roughly ~4.6e16. Since timeout_nsec is a u64, div_u64(timeout_nsec, NSEC_PER_SEC)
can never exceed ~1.8e10, meaning this condition is mathematically impossible to
trigger on 64-bit platforms.

Because the guard can never be met, can highly specific inputs cause
nsecs_to_jiffies64() to silently wrap around internally?

For example, when HZ makes nsecs_to_jiffies64() use the n * 9 calculation, a
carefully chosen timeout_nsec (like ~64.9 years, or around 2049638230412172402
ns) could wrap around internally to a near-zero jiffies value, resulting in a
premature timeout.

> +
> +	return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(timeout_nsec) + 1);
> +}
> +EXPORT_SYMBOL(drm_timeout_rel_to_jiffies);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-drm-timeout-helpers-v1-0-2de67405a145@igalia.com?part=2

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

[Christian König]

On 8/9/26 21:23, Maíra Canal wrote:
> drm_timeout_abs_to_jiffies() covers the drivers whose wait UAPI takes an
> absolute deadline, but there is no equivalent for the drivers that
> express a wait as a duration. Drivers such as i915 and v3d convert the
> value themselves.
> 
> Converting a nanosecond duration to jiffies needs some care.
> nsecs_to_jiffies() returns unsigned long, so on 32-bit a large
> userspace-supplied timeout overflows its range and is silently truncated.
> 
> i915 already handles both cases in a local helper, which v3d has a copy
> of. Add the same conversion to the core, so that it is available to any
> driver and both copies can be dropped.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/drm_timeout.c | 29 +++++++++++++++++++++++++++++
>  include/drm/drm_utils.h       |  1 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_timeout.c b/drivers/gpu/drm/drm_timeout.c
> index 78e9f65e5477..30ad3e8ebc92 100644
> --- a/drivers/gpu/drm/drm_timeout.c
> +++ b/drivers/gpu/drm/drm_timeout.c
> @@ -9,6 +9,7 @@
>  #include <linux/export.h>
>  #include <linux/jiffies.h>
>  #include <linux/ktime.h>
> +#include <linux/math64.h>
>  #include <linux/sched.h>
>  
>  #include <drm/drm_utils.h>
> @@ -45,3 +46,31 @@ signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec)
>  	return timeout_jiffies64 + 1;
>  }
>  EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);
> +
> +/**
> + * drm_timeout_rel_to_jiffies - calculate jiffies timeout from relative value
> + *
> + * @timeout_nsec: relative timeout in ns, 0 for poll
> + *
> + * Calculate the timeout in jiffies from a relative timeout in ns, for drivers
> + * whose UAPI expresses a wait as a duration rather than as a deadline.
> + *
> + * The result is clamped to MAX_JIFFY_OFFSET. That keeps it positive once it is
> + * converted to the signed long taken by dma_fence_wait_timeout() and friends,
> + * which matters on 32-bit, and keeps it distinct from MAX_SCHEDULE_TIMEOUT so
> + * that a finite wait is never understood as an infinite one.

Please add a comment that it is strongly discouraged to use relative timeouts in uAPIs.

The background is that relative timeouts doesn't work with restarting IOCTLs.

I think that's also part of the reason why we don't have a common helper function for that.

Regards,
Christian.

> + */
> +unsigned long drm_timeout_rel_to_jiffies(u64 timeout_nsec)
> +{
> +	/* make 0 timeout means poll, as for the absolute variant */
> +	if (timeout_nsec == 0)
> +		return 0;
> +
> +	/* nsecs_to_jiffies64() does not guard against overflow */
> +	if ((NSEC_PER_SEC % HZ) != 0 &&
> +	    div_u64(timeout_nsec, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
> +		return MAX_JIFFY_OFFSET;
> +
> +	return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(timeout_nsec) + 1);
> +}
> +EXPORT_SYMBOL(drm_timeout_rel_to_jiffies);
> diff --git a/include/drm/drm_utils.h b/include/drm/drm_utils.h
> index 6a46f755daba..8c0cc9835413 100644
> --- a/include/drm/drm_utils.h
> +++ b/include/drm/drm_utils.h
> @@ -25,5 +25,6 @@ const struct drm_panel_backlight_quirk *
>  drm_get_panel_backlight_quirk(const struct drm_edid *edid);
>  
>  signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec);
> +unsigned long drm_timeout_rel_to_jiffies(u64 timeout_nsec);
>  
>  #endif
> 

Re: [PATCH 3/6] drm/amdgpu: Use drm_timeout_abs_to_jiffies()

[Christian König]

On 8/9/26 21:23, Maíra Canal wrote:
> amdgpu_gem_timeout() converts an absolute deadline in ns into jiffies,
> which is what drm_timeout_abs_to_jiffies() already does for the other
> drivers whose wait UAPI takes a deadline. Use the shared helper and keep
> only the part that is specific to amdgpu.
> 
> Two details change along this conversion: the helper rounds up rather than
> truncating, so a deadline less than a tick away now waits for one jiffy
> instead of returning 0. It also uses nsecs_to_jiffies64(), so the
> conversion no longer truncates on 32-bit, where a large deadline could
> previously be reduced to an arbitrary shorter one.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> 
> ---
> 
> As a note, this patch can be merged independently to the AMD tree
> without any dependencies.
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 6a0699746fbc..84b509a484b0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -25,7 +25,6 @@
>   *          Alex Deucher
>   *          Jerome Glisse
>   */
> -#include <linux/ktime.h>
>  #include <linux/module.h>
>  #include <linux/overflow.h>
>  #include <linux/pagemap.h>
> @@ -40,6 +39,7 @@
>  #include <drm/drm_gem_ttm_helper.h>
>  #include <drm/ttm/ttm_tt.h>
>  #include <drm/drm_syncobj.h>
> +#include <drm/drm_utils.h>
>  
>  #include "amdgpu.h"
>  #include "amdgpu_display.h"
> @@ -622,23 +622,11 @@ int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
>   */
>  unsigned long amdgpu_gem_timeout(uint64_t timeout_ns)

Please completely nuke that function and replace it with calls to drm_timeout_abs_to_jiffies().

>  {
> -	unsigned long timeout_jiffies;
> -	ktime_t timeout;
> -
>  	/* clamp timeout if it's to large */
>  	if (((int64_t)timeout_ns) < 0)
>  		return MAX_SCHEDULE_TIMEOUT;

That check was actually never correct at all as far as I can see.

We need to make sure that when timeout_ns is larger than represent-able in long jiffies (especially on 32bit systems) then MAX_SCHEDULE_TIMEOUT is returned by drm_timeout_abs_to_jiffies().

And I hope that drm_timeout_abs_to_jiffies() does that correctly already, if not this seriously needs fixing anyway.

Regards,
Christian.

>  
> -	timeout = ktime_sub(ns_to_ktime(timeout_ns), ktime_get());
> -	if (ktime_to_ns(timeout) < 0)
> -		return 0;
> -
> -	timeout_jiffies = nsecs_to_jiffies(ktime_to_ns(timeout));
> -	/*  clamp timeout to avoid unsigned-> signed overflow */
> -	if (timeout_jiffies > MAX_SCHEDULE_TIMEOUT)
> -		return MAX_SCHEDULE_TIMEOUT - 1;
> -
> -	return timeout_jiffies;
> +	return drm_timeout_abs_to_jiffies(timeout_ns);
>  }
>  
>  int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
> 

Re: [PATCH 3/6] drm/amdgpu: Use drm_timeout_abs_to_jiffies()

[Maíra Canal]

Hi Christian,

On 10/08/26 10:34, Christian König wrote:
> On 8/9/26 21:23, Maíra Canal wrote:
>> amdgpu_gem_timeout() converts an absolute deadline in ns into jiffies,
>> which is what drm_timeout_abs_to_jiffies() already does for the other
>> drivers whose wait UAPI takes a deadline. Use the shared helper and keep
>> only the part that is specific to amdgpu.
>>
>> Two details change along this conversion: the helper rounds up rather than
>> truncating, so a deadline less than a tick away now waits for one jiffy
>> instead of returning 0. It also uses nsecs_to_jiffies64(), so the
>> conversion no longer truncates on 32-bit, where a large deadline could
>> previously be reduced to an arbitrary shorter one.
>>
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>>
>> ---
>>
>> As a note, this patch can be merged independently to the AMD tree
>> without any dependencies.
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++--------------
>>   1 file changed, 2 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 6a0699746fbc..84b509a484b0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -25,7 +25,6 @@
>>    *          Alex Deucher
>>    *          Jerome Glisse
>>    */
>> -#include <linux/ktime.h>
>>   #include <linux/module.h>
>>   #include <linux/overflow.h>
>>   #include <linux/pagemap.h>
>> @@ -40,6 +39,7 @@
>>   #include <drm/drm_gem_ttm_helper.h>
>>   #include <drm/ttm/ttm_tt.h>
>>   #include <drm/drm_syncobj.h>
>> +#include <drm/drm_utils.h>
>>   
>>   #include "amdgpu.h"
>>   #include "amdgpu_display.h"
>> @@ -622,23 +622,11 @@ int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
>>    */
>>   unsigned long amdgpu_gem_timeout(uint64_t timeout_ns)
> 
> Please completely nuke that function and replace it with calls to drm_timeout_abs_to_jiffies().
> 
>>   {
>> -	unsigned long timeout_jiffies;
>> -	ktime_t timeout;
>> -
>>   	/* clamp timeout if it's to large */
>>   	if (((int64_t)timeout_ns) < 0)
>>   		return MAX_SCHEDULE_TIMEOUT;
> 
> That check was actually never correct at all as far as I can see.
> 

I'm not sure about that... Taking a look at other UAPIs (like Panfrost),
I see that they use a s64 timeout_ns, which matches the ktime_t the
deadline is converted through. Contrary to that, AMD exposes a __u64 in
the UAPI and defines AMDGPU_TIMEOUT_INFINITE = 0xffffffffffffffffull in
libdrm. Therefore the check looks correct to me: it detects the values
above S64_MAX and maps them to an infinite wait.

Having said that, the comment is quite misleading. How about?

       /* Map anything that doesn't fit in a s64 to an infinite wait */

To make it even clearer, we could use `if (timeout_ns > S64_MAX)`.

> We need to make sure that when timeout_ns is larger than represent-able in long jiffies (especially on 32bit systems) then MAX_SCHEDULE_TIMEOUT is returned by drm_timeout_abs_to_jiffies().
> 

Actually, what drm_timeout_abs_to_jiffies() does is clamp the
nsecs_to_jiffies64() return to MAX_SCHEDULE_TIMEOUT - 1, so that a
finite deadline is never turned into an infinite wait. So it is safe on
32-bit, but it's not doing exactly what you described.

Best regards,
- Maíra

> And I hope that drm_timeout_abs_to_jiffies() does that correctly already, if not this seriously needs fixing anyway.
> 
> Regards,
> Christian.
> 
>>   
>> -	timeout = ktime_sub(ns_to_ktime(timeout_ns), ktime_get());
>> -	if (ktime_to_ns(timeout) < 0)
>> -		return 0;
>> -
>> -	timeout_jiffies = nsecs_to_jiffies(ktime_to_ns(timeout));
>> -	/*  clamp timeout to avoid unsigned-> signed overflow */
>> -	if (timeout_jiffies > MAX_SCHEDULE_TIMEOUT)
>> -		return MAX_SCHEDULE_TIMEOUT - 1;
>> -
>> -	return timeout_jiffies;
>> +	return drm_timeout_abs_to_jiffies(timeout_ns);
>>   }
>>   
>>   int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
>>
> 

Re: [PATCH 3/6] drm/amdgpu: Use drm_timeout_abs_to_jiffies()

[Christian König]

On 8/10/26 17:32, Maíra Canal wrote:
> Hi Christian,
> 
> On 10/08/26 10:34, Christian König wrote:
>> On 8/9/26 21:23, Maíra Canal wrote:
>>> amdgpu_gem_timeout() converts an absolute deadline in ns into jiffies,
>>> which is what drm_timeout_abs_to_jiffies() already does for the other
>>> drivers whose wait UAPI takes a deadline. Use the shared helper and keep
>>> only the part that is specific to amdgpu.
>>>
>>> Two details change along this conversion: the helper rounds up rather than
>>> truncating, so a deadline less than a tick away now waits for one jiffy
>>> instead of returning 0. It also uses nsecs_to_jiffies64(), so the
>>> conversion no longer truncates on 32-bit, where a large deadline could
>>> previously be reduced to an arbitrary shorter one.
>>>
>>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>>>
>>> ---
>>>
>>> As a note, this patch can be merged independently to the AMD tree
>>> without any dependencies.
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++--------------
>>>   1 file changed, 2 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 6a0699746fbc..84b509a484b0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -25,7 +25,6 @@
>>>    *          Alex Deucher
>>>    *          Jerome Glisse
>>>    */
>>> -#include <linux/ktime.h>
>>>   #include <linux/module.h>
>>>   #include <linux/overflow.h>
>>>   #include <linux/pagemap.h>
>>> @@ -40,6 +39,7 @@
>>>   #include <drm/drm_gem_ttm_helper.h>
>>>   #include <drm/ttm/ttm_tt.h>
>>>   #include <drm/drm_syncobj.h>
>>> +#include <drm/drm_utils.h>
>>>     #include "amdgpu.h"
>>>   #include "amdgpu_display.h"
>>> @@ -622,23 +622,11 @@ int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
>>>    */
>>>   unsigned long amdgpu_gem_timeout(uint64_t timeout_ns)
>>
>> Please completely nuke that function and replace it with calls to drm_timeout_abs_to_jiffies().
>>
>>>   {
>>> -    unsigned long timeout_jiffies;
>>> -    ktime_t timeout;
>>> -
>>>       /* clamp timeout if it's to large */
>>>       if (((int64_t)timeout_ns) < 0)
>>>           return MAX_SCHEDULE_TIMEOUT;
>>
>> That check was actually never correct at all as far as I can see.
>>
> 
> I'm not sure about that... Taking a look at other UAPIs (like Panfrost),
> I see that they use a s64 timeout_ns, which matches the ktime_t the
> deadline is converted through. Contrary to that, AMD exposes a __u64 in
> the UAPI and defines AMDGPU_TIMEOUT_INFINITE = 0xffffffffffffffffull in
> libdrm. Therefore the check looks correct to me: it detects the values
> above S64_MAX and maps them to an infinite wait.
> 
> Having said that, the comment is quite misleading. How about?
> 
>       /* Map anything that doesn't fit in a s64 to an infinite wait */
> 
> To make it even clearer, we could use `if (timeout_ns > S64_MAX)`.

Yeah that is pretty much what I had in mind as well. That would be much better already.

>> We need to make sure that when timeout_ns is larger than represent-able in long jiffies (especially on 32bit systems) then MAX_SCHEDULE_TIMEOUT is returned by drm_timeout_abs_to_jiffies().
>>
> 
> Actually, what drm_timeout_abs_to_jiffies() does is clamp the
> nsecs_to_jiffies64() return to MAX_SCHEDULE_TIMEOUT - 1, so that a
> finite deadline is never turned into an infinite wait. So it is safe on
> 32-bit, but it's not doing exactly what you described.

Mhm, I'm not sure if that behavior is a good idea.

IIRC MAX_SCHEDULE_TIMEOUT-1 is only ~50 days on 32bit systems, but that is way lower than the potentially requested time here. I mean we basically changing a wait of more than 250 years into a 50 days wait...

Most likely not relevant in practice, but I would still say that returning an infinity wait for timeout_ns > S64_MAX is more defensive.

Regards,
Christian.

> 
> Best regards,
> - Maíra
> 
>> And I hope that drm_timeout_abs_to_jiffies() does that correctly already, if not this seriously needs fixing anyway.
>>
>> Regards,
>> Christian.
>>
>>>   -    timeout = ktime_sub(ns_to_ktime(timeout_ns), ktime_get());
>>> -    if (ktime_to_ns(timeout) < 0)
>>> -        return 0;
>>> -
>>> -    timeout_jiffies = nsecs_to_jiffies(ktime_to_ns(timeout));
>>> -    /*  clamp timeout to avoid unsigned-> signed overflow */
>>> -    if (timeout_jiffies > MAX_SCHEDULE_TIMEOUT)
>>> -        return MAX_SCHEDULE_TIMEOUT - 1;
>>> -
>>> -    return timeout_jiffies;
>>> +    return drm_timeout_abs_to_jiffies(timeout_ns);
>>>   }
>>>     int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
>>>
>>
>