From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: accel: ethosu: Remove redundant job_lock Date: Mon, 18 May 2026 16:46:56 +1000 Message-ID: In-Reply-To: <20260516144623.2582427-2-mcanal@igalia.com> References: <20260516144623.2582427-2-mcanal@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: accel: ethosu: Remove redundant job_lock Author: =3D?UTF-8?q?Ma=3DC3=3DADra=3D20Canal?=3D Patches: 1 Reviewed: 2026-05-18T16:46:56.799851 --- This is a single, well-reasoned patch that removes a redundant `job_lock` m= utex from the ethosu accelerator driver, replacing it with `READ_ONCE`/`WRI= TE_ONCE` on `dev->in_flight_job`. The commit message provides an exceptiona= lly thorough analysis of the three concurrency scenarios. The approach is **correct**. The key invariants that make it safe: 1. **`credit_limit =3D 1`** (line 296) =E2=80=94 only one job can be in-fli= ght, so `run_job()` is never called while another `run_job()` is active. 2. **Scheduler serializes `run_job()` and `timedout_job()`** =E2=80=94 they= are mutually exclusive callbacks. 3. **IRQ handler clears `in_flight_job` before `dma_fence_signal()`** =E2= =80=94 so any `run_job(next)` woken by the signal sees NULL and can safely = store the new job pointer. 4. **Memory ordering is sufficient** =E2=80=94 `writel()` at the end of `et= hosu_job_hw_submit()` includes a write barrier (ensuring `in_flight_job` is= visible before HW is kicked), and `dma_fence_signal()` uses spinlocks inte= rnally (providing acquire/release semantics that order the NULL store relat= ive to the wake-up of the next `run_job()`). 5. **`job_lock` is fully removed** =E2=80=94 grep confirms zero remaining r= eferences. **One caveat**: the author notes this is compile-tested only (no hardware).= Given the subtle concurrency changes, hardware testing or careful review f= rom the ethosu maintainer would be prudent before merging. **Verdict: Looks good.** The analysis is sound and the code change matches = the stated rationale. --- --- Generated by Claude Code Patch Reviewer