From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Vladislav Zaharov" <vladazaharova2018@gmail.com>
Cc: <dakr@kernel.org>, <jhubbard@nvidia.com>, <aliceryhl@google.com>,
<ttabi@nvidia.com>, <gary@garyguo.net>,
<nova-gpu@lists.linux.dev>, <dri-devel@lists.freedesktop.org>,
<linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>
Subject: Re: [PATCH v6 2/3] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind
Date: Sat, 07 Nov 2026 10:26:24 +0900 [thread overview]
Message-ID: <DMOL5YCIIWZ4.3QZ73GMLSITG@nvidia.com> (raw)
In-Reply-To: <20261014154554.231015-3-vladazaharova2018@gmail.com>
On Thu Oct 15, 2026 at 12:45 AM JST, Vladislav Zaharov wrote:
> The GSP-RM log buffers are exposed through debugfs, but the Scope that
> owns them lives in Gsp, inside GspResources, inside the Gpu built by
> probe(). They are DMA allocations of the device and cannot outlive it,
> so the entries go away as soon as the GPU is unbound - and, more to the
> point, as soon as probe() fails, which is exactly when the log of a GSP
> that did not come up is the thing one wants to read.
>
> Add a gsp_keep_logs module parameter. When it is set, dropping the log
> buffers copies whatever the GSP wrote into memory owned by the module
> and exposes the copies until the module is unloaded. A buffer whose
> "put" pointer is still zero was never written to and is skipped.
>
> The GSP has normally been stopped by the time the buffers are dropped,
> but a boot that timed out can leave it still appending, so a DMA read
> barrier orders the read of the "put" pointer before the copy, and that
> value, not the one the copy reads again, is what the copy keeps.
That paragraph is nice as a code comment, but that's an implementation
detail that doesn't belong to a commit log. The commit log should tell
us *why* you are doing this, not go into excruciating details of *how*
you did it - only the big lines to make review easier.
I'd appreciate if you could do a pass on the commit logs and code
comments to deslop them and make them shorter and to the point
(similarly to what I tried to do on patch 1). Right now this patch
is pretty heavy to read.
>
> The copies belong to the module data next to the debugfs root, and live
> in a "retained" directory created during module init rather than on
> first use, which keeps the teardown path of a device from having to
> create anything. Keeping them out of the directory used by bound GPUs
> also means a device coming back does not find its debugfs name taken by
> its own history; nouveau, which recreates the entries under the name of
> the GPU that just went away, has that problem.
What Nouveau does is irrevelant - that's the kind of unneeeded
information I'd like us to avoid.
>
> While at it, move the log buffer code out of gsp.rs into gsp/logbuffer.rs.
"While at it" means this should be two patches: a first one that moves
the code into `gsp/logbuffer.rs`, a second one that performs the changes
you need there.
Right now this moves things while changing them, making it more
difficult to review than it could be.
next prev parent reply other threads:[~2026-11-07 1:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-14 15:45 [PATCH v6 0/3] gpu: nova-core: retain the GSP-RM log buffers Vladislav Zaharov
2026-10-14 15:45 ` [PATCH v6 1/3] gpu: nova-core: move the debugfs root into the module data Vladislav Zaharov
2026-11-07 1:23 ` Alexandre Courbot
2026-10-14 15:45 ` [PATCH v6 2/3] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind Vladislav Zaharov
2026-11-07 1:26 ` Alexandre Courbot [this message]
2026-10-14 15:45 ` [PATCH v6 3/3] Documentation: nova: remove completed GSP log buffer task Vladislav Zaharov
2026-11-06 20:51 ` [PATCH v6 0/3] gpu: nova-core: retain the GSP-RM log buffers Timur Tabi
2026-11-06 22:52 ` Danilo Krummrich
2026-11-06 23:15 ` John Hubbard
2026-11-06 23:08 ` Alexandre Courbot
2026-11-06 23:15 ` Alexandre Courbot
2026-11-07 0:39 ` John Hubbard
2026-11-07 1:38 ` Danilo Krummrich
2026-11-07 1:40 ` John Hubbard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DMOL5YCIIWZ4.3QZ73GMLSITG@nvidia.com \
--to=acourbot@nvidia.com \
--cc=aliceryhl@google.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=jhubbard@nvidia.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nova-gpu@lists.linux.dev \
--cc=ttabi@nvidia.com \
--cc=vladazaharova2018@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®