Re: [PATCH net-next v9 09/12] gve: introduce new methods to handle IRQ doorbells

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message of "gve: introduce new methods to handle IRQ
  doorbells" claims more than the code does and leaves out several…

--

gve: introduce new methods to handle IRQ doorbells

This commit adds request_db_info and release_db_resources callbacks to
struct gve_ctrl_ops. The AdminQ implementations issue the configure and
deconfigure device resources commands and set the per-block irq_db_index
pointers. Allocation of the irq_db_indices DMA buffer moves out of
gve_alloc_notify_blocks() into new helpers called from
gve_alloc_control_plane_resources().

> Admin Queue and Mailbox modes. All behaviors related to the IRQ doorbell
> indices will be managed by these new methods instead of occurring
> directly in notify_block setup/teardown methods.

[Severity: Low]

Is this description accurate for what the patch does?

gve_adminq_request_db_info() only sends the configure device resources
command, using the counter_array_bus and irq_db_indices_bus that are
already allocated. It then sets ntfy_blocks[i].irq_db_index.
gve_adminq_release_db_resources() only sends the deconfigure command.

The DMA buffer that holds the doorbell indices is allocated and freed by
two new static helpers in gve_main.c: gve_alloc_irq_db_indices() and
gve_free_irq_db_indices(). They are called from
gve_alloc_control_plane_resources() and gve_free_control_plane_resources(),
not from the new ops.

The next sentence says GQ ring counters will be managed in
request_db_info. However, the counter array is still allocated by
gve_alloc_counter_array() in gve_alloc_control_plane_resources().

The v3 changelog says this split was intentional. The "all behaviors"
wording looks like it is left over from the earlier design, even after the
v6 commit message update.

The commit message also leaves out a few other changes in the patch:

- gve_alloc_control_plane_resources() now allocates the counter array
  after the notify blocks and stats report, followed by irq_db_indices.
  gve_free_control_plane_resources() changes its free order to match.

- A new gve_ntfy_to_msix_idx() helper is now used for the msix_vectors[]
  lookups in gve_setup_notify_blocks() and gve_teardown_notify_blocks().
  The "Failed to receive msix vector" message now prints msix_idx.

- The "the last vector" and "the first n-1 vectors" comments are removed,
  although mgmt_msix_idx is still num_ntfy_blks.

None of these seem to change behavior in AdminQ mode. Could the commit
message be updated to match the code and mention these changes?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930190428.1130007-1-hramamurthy%40google.com