Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 28 Nov 2022 11:25:28 +0200
From:   Pekka Paalanen <ppaalanen@gmail.com>
To:     =?UTF-8?B?QW5kcsOp?= Almeida <andrealmeid@igalia.com>
Cc:     dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, kernel-dev@igalia.com,
        alexander.deucher@amd.com, contactshashanksharma@gmail.com,
        amaranath.somalapuram@amd.com, christian.koenig@amd.com,
        pierre-eric.pelloux-prayer@amd.com,
        Simon Ser <contact@emersion.fr>,
        Rob Clark <robdclark@gmail.com>,
        Andrey Grodzovsky <andrey.grodzovsky@amd.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Daniel Stone <daniel@fooishbar.org>,
        'Marek =?UTF-8?B?T2zFocOhayc=?= <maraeo@gmail.com>,
        Dave Airlie <airlied@gmail.com>,
        "Pierre-Loup A . Griffais" <pgriffais@valvesoftware.com>
Subject: Re: [PATCH v3 0/2] drm: Add GPU reset sysfs
Message-ID: <20221128112528.1206b1f5@eldfell>
In-Reply-To: <20221125175203.52481-1-andrealmeid@igalia.com>
References: <20221125175203.52481-1-andrealmeid@igalia.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="Sig_/j29UQcp8cD1U5AeBSxdSDmF";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Precedence: bulk

--Sig_/j29UQcp8cD1U5AeBSxdSDmF
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Fri, 25 Nov 2022 14:52:01 -0300
Andr=C3=A9 Almeida <andrealmeid@igalia.com> wrote:

> This patchset adds a udev event for DRM device's resets.

Hi,

this seems a good idea to me.

> Userspace apps can trigger GPU resets by misuse of graphical APIs or driv=
er
> bugs. Either way, the GPU reset might lead the system to a broken state[1=
], that
> might be recovered if user has access to a tty or a remote shell. Arguabl=
y, this
> recovery could happen automatically by the system itself, thus this is th=
e goal
> of this patchset.
>=20
> For debugging and report purposes, device coredump support was already ad=
ded
> for amdgpu[2], but it's not suitable for programmatic usage like this one=
 given
> the uAPI not being stable and the need for parsing.
>=20
> GL/VK is out of scope for this use, giving that we are dealing with device
> resets regardless of API.

I see that the reported PID is intended to be the culprit, the process
that caused the GPU to crash or hang, if identified. Hence, killing
that process perhaps makes sense, even if it could recover on its own
through GL/VK "device lost" mechanism.

"VRAM lost" is interesting. Innocent processes essentially lost the GPU
in that case, I suppose, but that's no reason to kill them and restart
the whole graphics stack outright. Those that actually handle GL/VK
device lost should theoretically be fine, right?

Display servers can make more enlightened decisions on whether they
need to restart or not, if they are implemented to handle that.

The example gpu-resetd [3] behaviour in that case seems sub-optimal.
Could it do better? How would it know, or avoid knowing, which
processes handled the GPU reset fine and which need external restarting?

Maybe gpu-resetd should kill the culprit only if it causes resets
repeatedly? But if the culprit does not handle device lost and also
does not die... how do you know you need to kill it?


Thanks,
pq

>=20
> A basic userspace daemon is provided at [3] showing how the interface is =
used
> to recovery from resets.
>=20
> [1] A search for "reset" in DRM/AMD issue tracker shows reports of resets
> making the system unusable:
> https://gitlab.freedesktop.org/drm/amd/-/issues/?search=3Dreset
>=20
> [2] https://lore.kernel.org/amd-gfx/20220602081538.1652842-2-Amaranath.So=
malapuram@amd.com/
>=20
> [3] https://gitlab.freedesktop.org/andrealmeid/gpu-resetd
>=20
> v2: https://lore.kernel.org/dri-devel/20220308180403.75566-1-contactshash=
anksharma@gmail.com/
>=20
> Andr=C3=A9 Almeida (1):
>   drm/amdgpu: Add work function for GPU reset event
>=20
> Shashank Sharma (1):
>   drm: Add GPU reset sysfs event
>=20
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  4 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30 ++++++++++++++++++++++
>  drivers/gpu/drm/drm_sysfs.c                | 26 +++++++++++++++++++
>  include/drm/drm_sysfs.h                    | 13 ++++++++++
>  4 files changed, 73 insertions(+)
>=20


--Sig_/j29UQcp8cD1U5AeBSxdSDmF
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEJQjwWQChkWOYOIONI1/ltBGqqqcFAmOEfogACgkQI1/ltBGq
qqdHTBAAtPNGC5EntpMSAxvoB+an5jcdGflRL7GEC3ZhtzD9TY3kpnXDQmirAq2O
MYjXpb5VdG9xcLXk+FFSjUVuNSDdaZbyhobhsJswr6GMi5LyxIfvmdnFsGaq2+JR
84ST3YTc1eNY5lUKxhMK3YpvcFtvlnZ+D5P3JgIUGo86CeLzc5t1ipwC5e1RzdMj
nic+Mz6Yfk+aMKzle1yBRGWnALzRpGQo1eUTukYrkU76LEI4h+KGYV0Tru/6pbI4
1AVeIiZY38gga7scTpx94HKqiJBdWSZjvwb6zc1LdoCXShvU8bMjuDrI8qmOnYpB
G1TG8qOFed8dNFTfefIHHTouzgaKusQIjpwzq6d5x6jg5LTzhV5YnAyommuxr7+P
d38ASP3lUKgG5V+E+fM6piA2HW8+ec0r2kBBMaHFhjkXJ+mvfhO4m4PadA22JFkm
jgU7QFoXW1dXT/6isHfZHQgFCyxmLRbd4jHHCQCRuJqIBoJ951qopRkJTM3Xg5Uk
rlig5oGWhPGpTsHyIvc/Cqe7Z1L5M35TwidcJgDrekpPlb2TcZ4/keDLeJfYcPPt
FP2FJYfgAzovZnp+HQ+UBlQQvH2E57H4s9NlwQM+X2rMwKBOdoQ3ZDCdVcRgGSkl
qWfeAbQujQSiVBO0UqgHV2zDAZREd/+dLNEaZTsCOyK3uRnWlSs=
=RAkP
-----END PGP SIGNATURE-----

--Sig_/j29UQcp8cD1U5AeBSxdSDmF--