Re: [PATCH 3/3] drm/amdgpu: add AMDGPURESET uevent on AMD GPU reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 18.01.22 um 11:40 schrieb S, Shirish:
Hi Shashank,


On 1/12/2022 6:30 PM, Sharma, Shashank wrote:


On 1/11/2022 12:26 PM, Christian König wrote:
Am 11.01.22 um 08:12 schrieb Somalapuram Amaranath:
AMDGPURESET uevent added to notify userspace,
collect dump_stack and amdgpu_reset_reg_dumps

Signed-off-by: Somalapuram Amaranath <Amaranath.Somalapuram@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/nv.c | 31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 2ec1ffb36b1f..41a2c37e825f 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -529,10 +529,41 @@ nv_asic_reset_method(struct amdgpu_device *adev)
      }
  }
+/**
+ * drm_sysfs_reset_event - generate a DRM uevent
+ * @dev: DRM device
+ *
+ * Send a uevent for the DRM device specified by @dev. Currently we only + * set AMDGPURESET=1 in the uevent environment, but this could be expanded to
+ * deal with other types of events.
+ *
+ * Any new uapi should be using the drm_sysfs_connector_status_event()
+ * for uevents on connector status change.
+ */
+void drm_sysfs_reset_event(struct drm_device *dev)
+{
+    char *event_string = "AMDGPURESET=1";
+    char *envp[2] = { event_string, NULL };
+
+ kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);

That won't work like this.

kobject_uevent_env() needs to allocate memory to send the event to userspace and that is not allowed while we do an reset. The Intel guys felt into the same trap already.

What we could maybe do is to teach kobject_uevent_env() gfp flags and make all allocations from the atomic pool.

Regards,
Christian.

Hi Amar,

I see another problem here,

We are sending the event at the GPU reset, but we are collecting the register values only when the corresponding userspace agent calls a read() on the respective sysfs entry.

Is the presumption here that gpu reset is always triggered within kernel & user space has to be made aware of it?

Yes, pretty much.


From what I know OS'/apps use GL extensions like robustness and other ways to detect hangs/gpu resets and flush out guilty contexts or take approp next steps.

BTW, is there any userspace infra already in place that have a task/thread listening  for reset events implemented, similar to hpd?

No, it's also completely different to HPD since we need to gather and save prerecorded data of the crash.


I believe there are several ways to make user space aware of reset via gpu_reset_counter etc, also if the objective is the have call trace upon reset or dump registers you can do it in the amdgpu_device_gpu_recover() but guard it with a proper CONFIG

that can be enabled in kernel's debug builds only, like tag along with KASAN etc.,

This way there will be lesser dependency on userspace.

That's a really bad idea. Gathering crash dump data should work on production kernels as well and is nothing we really need a compiler switch for.

Regards,
Christian.



Regards,

Shirish S


There is a very fair possibility that the register values are reset by the HW by then, and we are reading re-programmed values. At least there will be a race().

I think we should change this design in such a way:
1. Get into gpu_reset()
2. collect the register values and save this context into a separate file/node. Probably sending a trace_event here would be easiest way.
3. Send the drm event to the userspace client
4. The client reads from the trace file, and gets the data.

- Shashank


+}
+
+void amdgpu_reset_dumps(struct amdgpu_device *adev)
+{
+    struct drm_device *ddev = adev_to_drm(adev);
+    /* original raven doesn't have full asic reset */
+    if ((adev->apu_flags & AMD_APU_IS_RAVEN) &&
+        !(adev->apu_flags & AMD_APU_IS_RAVEN2))
+        return;
+    drm_sysfs_reset_event(ddev);
+    dump_stack();
+}
+
  static int nv_asic_reset(struct amdgpu_device *adev)
  {
      int ret = 0;
+    amdgpu_reset_dumps(adev);
      switch (nv_asic_reset_method(adev)) {
      case AMD_RESET_METHOD_PCI:
          dev_info(adev->dev, "PCI reset\n");





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux