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;
Message-ID: <85c538b01efb6f3fa6ff05ed1a0bc3ff87df7a61.camel@gmail.com>
Subject: Re: [RFC PATCH 0/1] Add AMDGPU_INFO_GUILTY_APP ioctl
From:   Timur =?ISO-8859-1?Q?Krist=F3f?= <timur.kristof@gmail.com>
To:     Christian =?ISO-8859-1?Q?K=F6nig?= <christian.koenig@amd.com>
Cc:     =?ISO-8859-1?Q?Andr=E9?= Almeida <andrealmeid@igalia.com>,
        Alex Deucher <alexdeucher@gmail.com>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        amd-gfx list <amd-gfx@lists.freedesktop.org>,
        linux-kernel@vger.kernel.org,
        "Pelloux-Prayer, Pierre-Eric" <pierre-eric.pelloux-prayer@amd.com>,
        Marek =?UTF-8?Q?Ol=C5=A1=C3=A1k?= <maraeo@gmail.com>,
        michel.daenzer@mailbox.org,
        Samuel Pitoiset <samuel.pitoiset@gmail.com>,
        kernel-dev@igalia.com, Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>,
        "Deucher, Alexander" <alexander.deucher@amd.com>
Date:   Tue, 02 May 2023 15:34:59 +0200
In-Reply-To: <fcca2934-a556-797c-535d-a66fc67bbe30@amd.com>
References: <20230501185747.33519-1-andrealmeid@igalia.com>
         <CADnq5_NXj4W44F_etRQ7HWdVTnf5zARCM3Y_o3EiwWiHj8QMpA@mail.gmail.com>
         <6ab2ff76-4518-6fac-071e-5d0d5adc4fcd@igalia.com>
         <cb3d2590-a1f8-fe7e-0bba-638ee80719af@amd.com>
         <CAFF-SiV0=WNmRW-D9uYUuj68Zq0APxtGLya9KR6FfZ7v0Zf2RQ@mail.gmail.com>
         <fcca2934-a556-797c-535d-a66fc67bbe30@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.1 (3.48.1-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

Hi,

On Tue, 2023-05-02 at 13:14 +0200, Christian K=C3=B6nig wrote:
> >=20
> > Christian K=C3=B6nig <christian.koenig@amd.com> ezt =C3=ADrta (id=C5=91=
pont: 2023.
> > m=C3=A1j. 2., Ke 9:59):
> > =C2=A0
> > > Am 02.05.23 um 03:26 schrieb Andr=C3=A9 Almeida:
> > > =C2=A0> Em 01/05/2023 16:24, Alex Deucher escreveu:
> > > =C2=A0>> On Mon, May 1, 2023 at 2:58=E2=80=AFPM Andr=C3=A9 Almeida
> > > <andrealmeid@igalia.com>=20
> > > =C2=A0>> wrote:
> > > =C2=A0>>>
> > > =C2=A0>>> I know that devcoredump is also used for this kind of
> > > information,=20
> > > =C2=A0>>> but I believe
> > > =C2=A0>>> that using an IOCTL is better for interfacing Mesa + Linux
> > > rather=20
> > > =C2=A0>>> than parsing
> > > =C2=A0>>> a file that its contents are subjected to be changed.
> > > =C2=A0>>
> > > =C2=A0>> Can you elaborate a bit on that?=C2=A0 Isn't the whole point=
 of
> > > devcoredump
> > > =C2=A0>> to store this sort of information?
> > > =C2=A0>>
> > > =C2=A0>
> > > =C2=A0> I think that devcoredump is something that you could use to
> > > submit to=20
> > > =C2=A0> a bug report as it is, and then people can read/parse as they
> > > want,=20
> > > =C2=A0> not as an interface to be read by Mesa... I'm not sure that
> > > it's=20
> > > =C2=A0> something that I would call an API. But I might be wrong, if
> > > you know=20
> > > =C2=A0> something that uses that as an API please share.
> > > =C2=A0>
> > > =C2=A0> Anyway, relying on that for Mesa would mean that we would nee=
d
> > > to=20
> > > =C2=A0> ensure stability for the file content and format, making it
> > > less=20
> > > =C2=A0> flexible to modify in the future and probe to bugs, while the
> > > IOCTL is=20
> > > =C2=A0> well defined and extensible. Maybe the dump from Mesa +
> > > devcoredump=20
> > > =C2=A0> could be complementary information to a bug report.
> > > =C2=A0
> > > =C2=A0Neither using an IOCTL nor devcoredump is a good approach for
> > > this since=20
> > > =C2=A0the values read from the hw register are completely unreliable.
> > > They=20
> > > =C2=A0could not be available because of GFXOFF or they could be
> > > overwritten or=20
> > > =C2=A0not even updated by the CP in the first place because of a hang
> > > etc....
> > > =C2=A0
> > > =C2=A0If you want to track progress inside an IB what you do instead
> > > is to=20
> > > =C2=A0insert intermediate fence write commands into the IB. E.g.
> > > something=20
> > > =C2=A0like write value X to location Y when this executes.
> > > =C2=A0
> > > =C2=A0This way you can not only track how far the IB processed, but
> > > also in=20
> > > =C2=A0which stages of processing we where when the hang occurred. E.g=
.
> > > End of=20
> > > =C2=A0Pipe, End of Shaders, specific shader stages etc...
> > > =C2=A0
> > >=20
> > =C2=A0
> > Currently our biggest challenge in the userspace driver is
> > debugging "random" GPU hangs. We have many dozens of bug reports
> > from users which are like: "play the game for X hours and it will
> > eventually hang the GPU". With the currently available tools, it is
> > impossible for us to tackle these issues. Andr=C3=A9's proposal would b=
e
> > a step in improving this situation.
> >=20
> > We already do something like what you suggest, but there are
> > multiple problems with that approach:
> > =C2=A0
> > 1. we can only submit 1 command buffer at a time because we won't
> > know which IB hanged
> > 2. we can't use chaining because we don't know where in the IB it
> > hanged
> > 3. it needs userspace to insert (a lot of) extra commands such as
> > extra synchronization and memory writes
> > 4. It doesn't work when GPU recovery is enabled because the
> > information is already gone when we detect the hang
> >=20
> =C2=A0You can still submit multiple IBs and even chain them. All you need
> to do is to insert into each IB commands which write to an extra
> memory location with the IB executed and the position inside the IB.
>=20
> =C2=A0The write data command allows to write as many dw as you want (up t=
o
> multiple kb). The only potential problem is when you submit the same
> IB multiple times.
>=20
> =C2=A0And yes that is of course quite some extra overhead, but I think
> that should be manageable.

Thanks, this sounds doable and would solve the limitation of how many
IBs are submitted at a time. However it doesn't address the problem
that enabling this sort of debugging will still have extra overhead.

I don't mean the overhead from writing a couple of dwords for the
trace, but rather, the overhead from needing to emit flushes or top of
pipe events or whatever else we need so that we can tell which command
hung the GPU.

> =C2=A0
> > In my opinion, the correct solution to those problems would be if
> > the kernel could give userspace the necessary information about a
> > GPU hang before a GPU reset.
> > =C2=A0=C2=A0
> =C2=A0The fundamental problem here is that the kernel doesn't have that
> information either. We know which IB timed out and can potentially do
> a devcoredump when that happens, but that's it.


Is it really not possible to know such a fundamental thing as what the
GPU was doing when it hung? How are we supposed to do any kind of
debugging without knowing that?

I wonder what AMD's Windows driver team is doing with this problem,
surely they must have better tools to deal with GPU hangs?

Best regards,
Timur