Re: [PATCH 1/2] drm/xe: Improve devcoredump documentation

[Raag Jadav <raag.jadav@intel.com>]

On Fri, Nov 01, 2024 at 07:44:37AM -0500, Lucas De Marchi wrote:
> On Fri, Nov 01, 2024 at 07:47:54AM +0200, Raag Jadav wrote:
> > On Thu, Oct 31, 2024 at 11:29:15AM -0700, Lucas De Marchi wrote:
> > 
> > ...
> > 
> > > - * Snapshot at hang:
> > > - * The 'data' file is printed with a drm_printer pointer at devcoredump read
> > > - * time. For this reason, we need to take snapshots from when the hang has
> > > - * happened, and not only when the user is reading the file. Otherwise the
> > > - * information is outdated since the resets might have happened in between.
> > > + * The following characteristics are observed by xe when creating a device
> > > + * coredump:
> > >   *
> > > - * 'First' failure snapshot:
> > > - * In general, the first hang is the most critical one since the following hangs
> > > - * can be a consequence of the initial hang. For this reason we only take the
> > > - * snapshot of the 'first' failure and ignore subsequent calls of this function,
> > > - * at least while the coredump device is alive. Dev_coredump has a delayed work
> > > - * queue that will eventually delete the device and free all the dump
> > > - * information.
> > > + * **Snapshot at hang**:
> > > + *   The 'data' file contains a snapshot of the HW state at the time the hang
> > > + *   happened. Due to the driver recovering from resets/crashes, it may not
> > > + *   correspond to the state of when the file is read by userspace.
> > 
> > Does that mean the devcoredump will be present even after a successful recovery?
> 
> yes.... if it's not succesful then it's moved to the wedged state. Easy
> way to test is running this:
> 
> 	xe_exec_threads --r threads-hang-basic
> 
> You should see something like this in your dmesg:
> 
> 	[IGT] xe_exec_threads: starting subtest threads-hang-basic
> 	xe 0000:00:02.0: [drm] GT0: Engine reset: engine_class=rcs, logical_mask: 0x1, guc_id=34
> 	xe 0000:00:02.0: [drm] GT0: Engine reset: engine_class=bcs, logical_mask: 0x1, guc_id=32
> 	xe 0000:00:02.0: [drm] GT1: Engine reset: engine_class=vcs, logical_mask: 0x1, guc_id=18
> 	xe 0000:00:02.0: [drm] GT0: Timedout job: seqno=4294967169, lrc_seqno=4294967169, guc_id=34, flags=0x0 in xe_exec_threads [2636]
> 	xe 0000:00:02.0: [drm] GT1: Engine reset: engine_class=vecs, logical_mask: 0x1, guc_id=17
> 	xe 0000:00:02.0: [drm] GT1: Timedout job: seqno=4294967169, lrc_seqno=4294967169, guc_id=18, flags=0x0 in xe_exec_threads [2636]
> 	xe 0000:00:02.0: [drm] Xe device coredump has been created
> -->	xe 0000:00:02.0: [drm] Check your /sys/class/drm/card0/device/devcoredump/data
> 	xe 0000:00:02.0: [drm] GT1: Timedout job: seqno=4294967169, lrc_seqno=4294967169, guc_id=17, flags=0x0 in xe_exec_threads [2636]
> 	xe 0000:00:02.0: [drm] GT0: Timedout job: seqno=4294967169, lrc_seqno=4294967169, guc_id=32, flags=0x0 in xe_exec_threads [2636]
> 	xe 0000:00:02.0: [drm] GT0: Engine reset: engine_class=ccs, logical_mask: 0x1, guc_id=27
> 	xe 0000:00:02.0: [drm] GT0: Timedout job: seqno=4294967169, lrc_seqno=4294967169, guc_id=27, flags=0x0 in xe_exec_threads [2636]
> 	[IGT] xe_exec_threads: finished subtest threads-hang-basic, SUCCESS
> 
> 
> If you run it again, it won't overwrite the previous dump, until user
> cleans the previous dump or the timeout on the kernel side fires to
> release it.

Yes, which I think we're covering at later point in "First failure only".
So maybe establishing the mechanism itself before explaining reset/recovery
would be a bit neater...

> From a distro-integration pov, I think it should have a udev rule that
> fires when a devcoredump is created so the dump is copied to persistent
> storage. Just like it happens with cpu coredump (see systemd-coredump)
> 
> > Perhaps moving the 'release' part to above paragraph will add required context.
> 
> not sure I follow. Are you suggesting to swap the order of "First
> failure only" and "Snapshot at hang" ?

... in whichever way you think is best.

> > > + * **First failure only**:
> > > + *   In general, the first hang is the most critical one since the following
> > > + *   hangs can be a consequence of the initial hang. For this reason a snapshot
> > > + *   is taken only for the first failure. Until the devcoredump is released by
> > > + *   userspace or kernel, all subsequent hangs do not override the snapshot nor
> > > + *   create new ones. Devcoredump has a delayed work queue that will eventually
> > > + *   delete the file node and free all the dump information.

Raag