<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Arial;font-size:10pt;color:#0078D7;margin:15pt;" align="Left">
[AMD Official Use Only - Internal Distribution Only]<br>
</p>
<br>
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Christian,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<font size="2"><span style="font-size:11pt">wait_for_completion_interruptible_timeout() would decrease autodump.dumping.done to UINT_MAX-1.</span></font></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
<font size="2"><span style="font-size:11pt"></span></font></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<font size="2"><span style="font-size:11pt">complete_all() here would restore autodump.dumping to the state as in
<font size="2"><span style="font-size:11pt">amdgpu_debugfs_autodump_init</span></font>().</span></font></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<font size="2"><span style="font-size:11pt"><br>
</span></font></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<font size="2"><span style="font-size:11pt">I want to make sure every open() deals with the same situation.</span></font></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<font size="2"><span style="font-size:11pt"><br>
</span></font></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<font size="2"><span style="font-size:11pt">Jiange</span></font><br>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Christian König <ckoenig.leichtzumerken@gmail.com><br>
<b>Sent:</b> Thursday, May 14, 2020 3:01 PM<br>
<b>To:</b> Zhao, Jiange <Jiange.Zhao@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Pelloux-prayer, Pierre-eric <Pierre-eric.Pelloux-prayer@amd.com>; Zhao, Jiange <Jiange.Zhao@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Liu, Monk
 <Monk.Liu@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com><br>
<b>Subject:</b> Re: [PATCH] drm/amdgpu: Add autodump debugfs node for gpu reset v4</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Am 14.05.20 um 07:29 schrieb jianzh@amd.com:<br>
> From: Jiange Zhao <Jiange.Zhao@amd.com><br>
><br>
> When GPU got timeout, it would notify an interested part<br>
> of an opportunity to dump info before actual GPU reset.<br>
><br>
> A usermode app would open 'autodump' node under debugfs system<br>
> and poll() for readable/writable. When a GPU reset is due,<br>
> amdgpu would notify usermode app through wait_queue_head and give<br>
> it 10 minutes to dump info.<br>
><br>
> After usermode app has done its work, this 'autodump' node is closed.<br>
> On node closure, amdgpu gets to know the dump is done through<br>
> the completion that is triggered in release().<br>
><br>
> There is no write or read callback because necessary info can be<br>
> obtained through dmesg and umr. Messages back and forth between<br>
> usermode app and amdgpu are unnecessary.<br>
><br>
> v2: (1) changed 'registered' to 'app_listening'<br>
>      (2) add a mutex in open() to prevent race condition<br>
><br>
> v3 (chk): grab the reset lock to avoid race in autodump_open,<br>
>            rename debugfs file to amdgpu_autodump,<br>
>            provide autodump_read as well,<br>
>            style and code cleanups<br>
><br>
> v4: add 'bool app_listening' to differentiate situations, so that<br>
>      the node can be reopened; also, there is no need to wait for<br>
>      completion when no app is waiting for a dump.<br>
><br>
> v5: change 'bool app_listening' to 'enum amdgpu_autodump_state'<br>
>      add 'app_state_mutex' for race conditions:<br>
>        (1)Only 1 user can open this file node<br>
>        (2)wait_dump() can only take effect after poll() executed.<br>
>        (3)eliminated the race condition between release() and<br>
>           wait_dump()<br>
><br>
> v6: removed 'enum amdgpu_autodump_state' and 'app_state_mutex'<br>
>      removed state checking in amdgpu_debugfs_wait_dump<br>
>      Improve on top of version 3 so that the node can be reopened.<br>
><br>
> v7: move reinit_completion into open() so that only one user<br>
>      can open it.<br>
><br>
> Signed-off-by: Jiange Zhao <Jiange.Zhao@amd.com><br>
> ---<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         |  2 +<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 79 ++++++++++++++++++++-<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h |  6 ++<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  2 +<br>
>   4 files changed, 88 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> index 2a806cb55b78..9e8eeddfe7ce 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> @@ -992,6 +992,8 @@ struct amdgpu_device {<br>
>        char                            product_number[16];<br>
>        char                            product_name[32];<br>
>        char                            serial[16];<br>
> +<br>
> +     struct amdgpu_autodump          autodump;<br>
>   };<br>
>   <br>
>   static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev)<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c<br>
> index 1a4894fa3693..efee3f1adecf 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c<br>
> @@ -27,7 +27,7 @@<br>
>   #include <linux/pci.h><br>
>   #include <linux/uaccess.h><br>
>   #include <linux/pm_runtime.h><br>
> -<br>
> +#include <linux/poll.h><br>
>   #include <drm/drm_debugfs.h><br>
>   <br>
>   #include "amdgpu.h"<br>
> @@ -74,8 +74,83 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,<br>
>        return 0;<br>
>   }<br>
>   <br>
> +int amdgpu_debugfs_wait_dump(struct amdgpu_device *adev)<br>
> +{<br>
> +#if defined(CONFIG_DEBUG_FS)<br>
> +     unsigned long timeout = 600 * HZ;<br>
> +     int ret;<br>
> +<br>
> +     wake_up_interruptible(&adev->autodump.gpu_hang);<br>
> +<br>
> +     ret = wait_for_completion_interruptible_timeout(&adev->autodump.dumping, timeout);<br>
> +     complete_all(&adev->autodump.dumping);<br>
<br>
Sorry that I'm mentioning this only now. But what is this complete_all() <br>
here good for?<br>
<br>
I mean we already waited for completion, didn't we?<br>
<br>
Christian.<br>
<br>
> +     if (ret == 0) {<br>
> +             pr_err("autodump: timeout, move on to gpu recovery\n");<br>
> +             return -ETIMEDOUT;<br>
> +     }<br>
> +#endif<br>
> +     return 0;<br>
> +}<br>
> +<br>
>   #if defined(CONFIG_DEBUG_FS)<br>
>   <br>
> +static int amdgpu_debugfs_autodump_open(struct inode *inode, struct file *file)<br>
> +{<br>
> +     struct amdgpu_device *adev = inode->i_private;<br>
> +     int ret;<br>
> +<br>
> +     file->private_data = adev;<br>
> +<br>
> +     mutex_lock(&adev->lock_reset);<br>
> +     if (adev->autodump.dumping.done) {<br>
> +             reinit_completion(&adev->autodump.dumping);<br>
> +             ret = 0;<br>
> +     } else {<br>
> +             ret = -EBUSY;<br>
> +     }<br>
> +     mutex_unlock(&adev->lock_reset);<br>
> +<br>
> +     return ret;<br>
> +}<br>
> +<br>
> +static int amdgpu_debugfs_autodump_release(struct inode *inode, struct file *file)<br>
> +{<br>
> +     struct amdgpu_device *adev = file->private_data;<br>
> +<br>
> +     complete_all(&adev->autodump.dumping);<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static unsigned int amdgpu_debugfs_autodump_poll(struct file *file, struct poll_table_struct *poll_table)<br>
> +{<br>
> +     struct amdgpu_device *adev = file->private_data;<br>
> +<br>
> +     poll_wait(file, &adev->autodump.gpu_hang, poll_table);<br>
> +<br>
> +     if (adev->in_gpu_reset)<br>
> +             return POLLIN | POLLRDNORM | POLLWRNORM;<br>
> +<br>
> +     return 0;<br>
> +}<br>
> +<br>
> +static const struct file_operations autodump_debug_fops = {<br>
> +     .owner = THIS_MODULE,<br>
> +     .open = amdgpu_debugfs_autodump_open,<br>
> +     .poll = amdgpu_debugfs_autodump_poll,<br>
> +     .release = amdgpu_debugfs_autodump_release,<br>
> +};<br>
> +<br>
> +static void amdgpu_debugfs_autodump_init(struct amdgpu_device *adev)<br>
> +{<br>
> +     init_completion(&adev->autodump.dumping);<br>
> +     complete_all(&adev->autodump.dumping);<br>
> +     init_waitqueue_head(&adev->autodump.gpu_hang);<br>
> +<br>
> +     debugfs_create_file("amdgpu_autodump", 0600,<br>
> +             adev->ddev->primary->debugfs_root,<br>
> +             adev, &autodump_debug_fops);<br>
> +}<br>
> +<br>
>   /**<br>
>    * amdgpu_debugfs_process_reg_op - Handle MMIO register reads/writes<br>
>    *<br>
> @@ -1434,6 +1509,8 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)<br>
>   <br>
>        amdgpu_ras_debugfs_create_all(adev);<br>
>   <br>
> +     amdgpu_debugfs_autodump_init(adev);<br>
> +<br>
>        return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,<br>
>                                        ARRAY_SIZE(amdgpu_debugfs_list));<br>
>   }<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h<br>
> index de12d1101526..2803884d338d 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h<br>
> @@ -31,6 +31,11 @@ struct amdgpu_debugfs {<br>
>        unsigned                num_files;<br>
>   };<br>
>   <br>
> +struct amdgpu_autodump {<br>
> +     struct completion               dumping;<br>
> +     struct wait_queue_head          gpu_hang;<br>
> +};<br>
> +<br>
>   int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);<br>
>   int amdgpu_debugfs_init(struct amdgpu_device *adev);<br>
>   void amdgpu_debugfs_fini(struct amdgpu_device *adev);<br>
> @@ -40,3 +45,4 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,<br>
>   int amdgpu_debugfs_fence_init(struct amdgpu_device *adev);<br>
>   int amdgpu_debugfs_firmware_init(struct amdgpu_device *adev);<br>
>   int amdgpu_debugfs_gem_init(struct amdgpu_device *adev);<br>
> +int amdgpu_debugfs_wait_dump(struct amdgpu_device *adev);<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> index cc41e8f5ad14..545beebcf43e 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
> @@ -3927,6 +3927,8 @@ static int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,<br>
>        int i, r = 0;<br>
>        bool need_full_reset  = *need_full_reset_arg;<br>
>   <br>
> +     amdgpu_debugfs_wait_dump(adev);<br>
> +<br>
>        /* block all schedulers and reset given job's ring */<br>
>        for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {<br>
>                struct amdgpu_ring *ring = adev->rings[i];<br>
<br>
</div>
</span></font></div>
</div>
</body>
</html>