[PATCH] drm/i915/gt: Use IS_ERR() check for kernel_context() in selftests

[PATCH] drm/i915/gt: Use IS_ERR() check for kernel_context() in selftests

[Ingyu Jang]

kernel_context() returns an error pointer on failure, never NULL, so
the NULL checks on its return value are unreachable. On a real
allocation failure the subsequent dereference of the returned ERR_PTR
(e.g. ctx_hi->sched.priority = ...) would oops the test kernel.

Use IS_ERR() and propagate the actual error via PTR_ERR() in
live_preempt(), live_late_preempt(), live_preempt_timeout() and
preempt_client_init().

Signed-off-by: Ingyu Jang <ingyujang25@korea.ac.kr>
---
 drivers/gpu/drm/i915/gt/selftest_execlists.c | 28 ++++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index 21e5ed9f72a30..36f51805f74e3 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -1742,13 +1742,15 @@ static int live_preempt(void *arg)
 	int err = -ENOMEM;
 
 	ctx_hi = kernel_context(gt->i915, NULL);
-	if (!ctx_hi)
-		return -ENOMEM;
+	if (IS_ERR(ctx_hi))
+		return PTR_ERR(ctx_hi);
 	ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
-	if (!ctx_lo)
+	if (IS_ERR(ctx_lo)) {
+		err = PTR_ERR(ctx_lo);
 		goto err_ctx_hi;
+	}
 	ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
 
 	if (igt_spinner_init(&spin_hi, gt))
@@ -1834,12 +1836,14 @@ static int live_late_preempt(void *arg)
 	int err = -ENOMEM;
 
 	ctx_hi = kernel_context(gt->i915, NULL);
-	if (!ctx_hi)
-		return -ENOMEM;
+	if (IS_ERR(ctx_hi))
+		return PTR_ERR(ctx_hi);
 
 	ctx_lo = kernel_context(gt->i915, NULL);
-	if (!ctx_lo)
+	if (IS_ERR(ctx_lo)) {
+		err = PTR_ERR(ctx_lo);
 		goto err_ctx_hi;
+	}
 
 	if (igt_spinner_init(&spin_hi, gt))
 		goto err_ctx_lo;
@@ -1934,8 +1938,8 @@ struct preempt_client {
 static int preempt_client_init(struct intel_gt *gt, struct preempt_client *c)
 {
 	c->ctx = kernel_context(gt->i915, NULL);
-	if (!c->ctx)
-		return -ENOMEM;
+	if (IS_ERR(c->ctx))
+		return PTR_ERR(c->ctx);
 
 	if (igt_spinner_init(&c->spin, gt))
 		goto err_ctx;
@@ -3384,13 +3388,15 @@ static int live_preempt_timeout(void *arg)
 		return 0;
 
 	ctx_hi = kernel_context(gt->i915, NULL);
-	if (!ctx_hi)
-		return -ENOMEM;
+	if (IS_ERR(ctx_hi))
+		return PTR_ERR(ctx_hi);
 	ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY;
 
 	ctx_lo = kernel_context(gt->i915, NULL);
-	if (!ctx_lo)
+	if (IS_ERR(ctx_lo)) {
+		err = PTR_ERR(ctx_lo);
 		goto err_ctx_hi;
+	}
 	ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY;
 
 	if (igt_spinner_init(&spin_lo, gt))
-- 
2.34.1

Claude review: drm/i915/gt: Use IS_ERR() check for kernel_context() in selftests

[Claude Code Review Bot]

Overall Series Review

Subject: drm/i915/gt: Use IS_ERR() check for kernel_context() in selftests
Author: Ingyu Jang <ingyujang25@korea.ac.kr>
Patches: 1
Reviewed: 2026-05-16T10:29:02.406576

---

**Verdict: Correct fix, but incomplete — misses one call site in the same file.**

The patch correctly identifies a real bug: `kernel_context()` (defined in `gem/selftests/mock_context.c`) returns `ERR_PTR` on failure (via `ERR_CAST` from `proto_context_create()` or directly from `i915_gem_create_context()`), never `NULL`. The existing `!ctx` checks would silently pass an `ERR_PTR` through, leading to a kernel oops on the very next dereference (e.g., `ctx_hi->sched.priority = ...`).

The transformation is mechanically correct in all four locations touched. However, the patch misses one more instance of the same bug in the same file at line 3685-3687 in `live_preempt_smoke()`:

```c
smoke.contexts[n] = kernel_context(smoke.gt->i915, NULL);
if (!smoke.contexts[n])
    goto err_ctx;
```

This should also be converted to `IS_ERR()` / `PTR_ERR()` for completeness, and `err_ctx` cleanup would need to handle passing an `ERR_PTR` in the `smoke.contexts[]` array (the cleanup loop calls `kernel_context_close()` on each non-NULL entry, so an `ERR_PTR` value would also be dereferenced there).

There may also be similar incorrect NULL checks in other selftest files (e.g., `selftest_hangcheck.c`, `selftest_workarounds.c`, `selftest_lrc.c`, etc.) that call `kernel_context()`. A v2 with broader scope would be more useful.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/i915/gt: Use IS_ERR() check for kernel_context() in selftests

[Claude Code Review Bot]

Patch Review

**Correctness of the core fix: Good.** The commit message accurately describes the problem: `kernel_context()` returns an error pointer on failure, so `!ctx` checks are dead code that won't catch allocation failures. The fix is the right pattern.

**Four call sites fixed correctly:**

1. **`live_preempt()`** (line 1744): `ctx_hi` — straightforward early return, correct.
2. **`live_preempt()`** (line 1749): `ctx_lo` — correctly captures `PTR_ERR(ctx_lo)` into `err` before `goto err_ctx_hi`, since the previous code was relying on the `err = -ENOMEM` initializer which would have been wrong for a different error code.
3. **`live_late_preempt()`** (lines 1838, 1842): Same pattern, correct.
4. **`preempt_client_init()`** (line 1940): Direct return of `PTR_ERR`, correct.
5. **`live_preempt_timeout()`** (lines 3390, 3395): Same pattern as `live_preempt`, correct.

**Issue: Missed call site in the same file.**

At `selftest_execlists.c:3685`:
```c
smoke.contexts[n] = kernel_context(smoke.gt->i915, NULL);
if (!smoke.contexts[n])
    goto err_ctx;
```
This has the identical bug but was not fixed. The `err_ctx` cleanup path iterates `smoke.contexts[]` and calls `kernel_context_close()` on each, so storing an `ERR_PTR` there would also cause a crash during cleanup. The fix here requires both converting the check to `IS_ERR()` and clearing the pointer (or otherwise guarding the cleanup loop). Since the commit message claims to fix this pattern in the file, this omission should be addressed.

**Minor style note:** The added braces around single-statement `goto` blocks (e.g., `if (IS_ERR(ctx_lo)) { err = PTR_ERR(ctx_lo); goto err_ctx_hi; }`) are correct since these are now multi-statement blocks (assignment + goto).

**Recommendation:** Request a v2 that also fixes the `live_preempt_smoke()` site at line 3685, and ideally audits other selftest files for the same pattern.

---
Generated by Claude Code Patch Reviewer