* [PATCH] accel/rocket: fix ignored return value in rocket_ioctl_submit
@ 2026-05-10 20:00 Kartik Nair
2026-05-16 6:08 ` Claude review: " Claude Code Review Bot
2026-05-16 6:08 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Kartik Nair @ 2026-05-10 20:00 UTC (permalink / raw)
To: tomeu, ogabbay; +Cc: dri-devel, linux-kernel, Kartik Nair
rocket_ioctl_submit() calls rocket_ioctl_submit_job() in a loop but
ignores its return value. If any job submission fails, the error is
silently dropped and the function returns 0 (success) to userspace,
leaving subsequent jobs potentially in an inconsistent state.
Fix this by propagating the error and breaking out of the loop on
the first failure.
Signed-off-by: Kartik Nair <contact.kartikn@gmail.com>
---
drivers/accel/rocket/rocket_job.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index ac51bff39..abac26853 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -626,8 +626,11 @@ int rocket_ioctl_submit(struct drm_device *dev, void *data, struct drm_file *fil
}
- for (i = 0; i < args->job_count; i++)
- rocket_ioctl_submit_job(dev, file, &jobs[i]);
+ for (i = 0; i < args->job_count; i++) {
+ ret = rocket_ioctl_submit_job(dev, file, &jobs[i]);
+ if (ret)
+ goto exit;
+ }
exit:
kvfree(jobs);
--
2.50.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Claude review: accel/rocket: fix ignored return value in rocket_ioctl_submit
2026-05-10 20:00 [PATCH] accel/rocket: fix ignored return value in rocket_ioctl_submit Kartik Nair
2026-05-16 6:08 ` Claude review: " Claude Code Review Bot
@ 2026-05-16 6:08 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:08 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: accel/rocket: fix ignored return value in rocket_ioctl_submit
Author: Kartik Nair <contact.kartikn@gmail.com>
Patches: 1
Reviewed: 2026-05-16T16:08:43.322827
---
This is a single-patch fix for the `accel/rocket` driver where `rocket_ioctl_submit()` was ignoring the return value of `rocket_ioctl_submit_job()`. The bug is real — without this fix, a failed job submission is silently swallowed and userspace receives a success return code despite the failure. The fix is correct, minimal, and follows the existing error-handling pattern already used in the same function (the `copy_from_user` loop above uses the identical `goto exit` pattern).
**Verdict: The patch looks good.**
One minor observation worth noting, but not necessarily a blocker: when a job fails partway through a multi-job submission, the jobs that were already successfully submitted (pushed to the scheduler) will still execute. This is arguably the correct behavior — `rocket_ioctl_submit_job()` fully owns each job's lifecycle (it cleans up on error, and pushes to the scheduler on success) — but the caller has no way to cancel previously submitted jobs. Whether this is a problem depends on the userspace API contract. The original code had the same issue (it submitted all jobs regardless of failures), so this patch is strictly an improvement.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: accel/rocket: fix ignored return value in rocket_ioctl_submit
2026-05-10 20:00 [PATCH] accel/rocket: fix ignored return value in rocket_ioctl_submit Kartik Nair
@ 2026-05-16 6:08 ` Claude Code Review Bot
2026-05-16 6:08 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 6:08 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness: Good.** The fix correctly captures the return value of `rocket_ioctl_submit_job()` and breaks out of the loop on the first failure, propagating the error to userspace via the existing `exit` label.
The key code change:
```c
- for (i = 0; i < args->job_count; i++)
- rocket_ioctl_submit_job(dev, file, &jobs[i]);
+ for (i = 0; i < args->job_count; i++) {
+ ret = rocket_ioctl_submit_job(dev, file, &jobs[i]);
+ if (ret)
+ goto exit;
+ }
```
**No resource leak concerns.** Looking at `rocket_ioctl_submit_job()` (line 533–588), it is fully self-contained with respect to resource management: on error, it cleans up the scheduler job and puts the job reference before returning. On success, the job is pushed to the scheduler and also properly ref-counted. So skipping subsequent jobs after a failure doesn't leak anything — the `jobs` array is just userspace-copied data (`struct drm_rocket_job` values), freed at the `exit` label via `kvfree(jobs)`.
**Pattern consistency.** The fix mirrors the error-handling style of the `copy_from_user` loop immediately above (lines 617–625), which also uses `goto exit` on failure. This is clean and consistent.
**Commit message.** Clear and accurate. Describes the problem (ignored return value, silent error), the consequence (success returned to userspace, inconsistent state), and the fix (propagate error, break on failure).
**Reviewed-by worthy.** No issues found. This is a straightforward, correct bugfix.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-16 6:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10 20:00 [PATCH] accel/rocket: fix ignored return value in rocket_ioctl_submit Kartik Nair
2026-05-16 6:08 ` Claude review: " Claude Code Review Bot
2026-05-16 6:08 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox