Re: [PATCH 1/2] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind
Vladislav Zaharov <[email protected]>
| Newsgroups | dev.linux.lists.nova-gpu,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
[email protected] wrote: > Does this code need a memory barrier (such as dma_rmb()) between reading > the 'put' pointer and copying the buffer contents? Not in the path this code is written for. snapshot() is called from the drop path of the log buffers, which is only reached after PinnedDrop for GspResources has run the GSP unload sequence, so the GSP is no longer writing to these buffers and there is nothing to order against. The one case where that does not hold is a Gsp::boot() that fails by timing out: the GSP may then still be alive and appending while the buffers are dropped. The copy is best-effort there - it cannot be made atomic either way - but reading a non-zero 'put' and then reading contents that predate it is indeed the pattern dma_rmb() exists for. I am happy to add it, but there is no dma_rmb() abstraction in rust/kernel at the moment: sync/barrier.rs only provides the smp_* family, and smp_rmb() is not a correct substitute, as on arm64 it uses the inner shareable domain rather than the outer shareable one that dma_rmb() needs. Would you prefer a small prerequisite patch adding dma_rmb() to rust/kernel, or is relying on the unload ordering acceptable, with the requirement spelled out in a comment on snapshot()? Thanks, Vladislav