From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: gpu: nova-core: gsp: add mechanism to wait for space on command queue
Date: Tue, 03 Mar 2026 13:34:47 +1000
Message-ID: <review-patch2-20260302-cmdq-continuation-v4-2-c011f15aad58@nvidia.com>
In-Reply-To: <20260302-cmdq-continuation-v4-2-c011f15aad58@nvidia.com>
References: <20260302-cmdq-continuation-v4-0-c011f15aad58@nvidia.com> <20260302-cmdq-continuation-v4-2-c011f15aad58@nvidia.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

Adds `driver_write_area_size()` and changes `allocate_command()` to poll-wa=
it with a timeout instead of immediately returning `EAGAIN`.

The available-space calculation looks correct:

```rust
let slots =3D (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES;
num::u32_as_usize(slots) * GSP_PAGE_SIZE
```

The comment accurately describes the minimum case (rx=3D=3D0, tx=3D=3DMSGQ_=
NUM_PAGES-1 =E2=86=92 0 slots).

**Minor observation:** The poll interval is `Delta::ZERO`:

```rust
read_poll_timeout(
    || Ok(self.driver_write_area_size()),
    |available_bytes| *available_bytes >=3D size_of::<GspMsgElement>() + si=
ze,
    Delta::ZERO,
    timeout,
)?;
```

This means busy-polling with no sleep between checks. For the typical case =
where the queue has space, this returns immediately (one iteration). For th=
e rare case where the queue is full, this busy-spins for up to 1 second. De=
pending on how `read_poll_timeout` is implemented in the Rust kernel bindin=
gs, this could consume significant CPU. Consider whether a small sleep inte=
rval (e.g., `Delta::from_millis(1)`, matching the pattern used in `wait_for=
_msg`) would be more appropriate for the queue-full case.

The 1-second timeout at the call site (`Delta::from_secs(1)`) seems reasona=
ble and matches nouveau's approach.

---
Generated by Claude Code Patch Reviewer