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: Thu, 05 Mar 2026 13:55:43 +1000
Message-ID: <review-patch2-20260304-cmdq-continuation-v5-2-3f19d759ed93@nvidia.com>
In-Reply-To: <20260304-cmdq-continuation-v5-2-3f19d759ed93@nvidia.com>
References: <20260304-cmdq-continuation-v5-0-3f19d759ed93@nvidia.com> <20260304-cmdq-continuation-v5-2-3f19d759ed93@nvidia.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This replaces the immediate `EAGAIN` return with a `read_poll_timeout` loop, which is the right approach for supporting continuation records that can fill the queue.

The `driver_write_area_size` method is a good addition that cleanly computes the available space without needing to materialize the slices. The comment explaining the minimum value is helpful.

One observation: the `read_poll_timeout` uses `Delta::ZERO` for the poll interval:

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

A zero poll interval means busy-waiting. Other callers in the same tree (e.g., `wait_for_msg` at cmdq.rs:572) use `Delta::from_millis(1)`. This could be intentional since command queue space should free up very quickly after GSP processes a command, but it might be worth adding a brief comment explaining the choice or using a small interval to avoid CPU spinning under load.

The timeout of 1 second (`ALLOCATE_TIMEOUT`) matches nouveau's approach, which is good.

---
Generated by Claude Code Patch Reviewer