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

[Lucas De Marchi]

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.

 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)

Lucas De Marchi

>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" ?

thanks
Lucas De Marchi

>
>> + * **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