<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 2022-05-16 11:08, Christian König
wrote:<br>
</div>
<blockquote type="cite" cite="mid:6cf84065-6341-3f96-4e09-ab71796241ec@amd.com">
Am 16.05.22 um 16:12 schrieb Andrey Grodzovsky:<br>
<blockquote type="cite" cite="mid:cdce4608-9ade-ac23-b957-6d38a3e2b55a@amd.com">
<p>Ping <br>
</p>
</blockquote>
<br>
Ah, yes sorry.<br>
<br>
<blockquote type="cite" cite="mid:cdce4608-9ade-ac23-b957-6d38a3e2b55a@amd.com">
<p> </p>
<p>Andrey<br>
</p>
<div class="moz-cite-prefix">On 2022-05-13 11:41, Andrey
Grodzovsky wrote:<br>
</div>
<blockquote type="cite" cite="mid:0b2657b0-41b6-53b1-f087-1fa0fb821839@amd.com">
<blockquote type="cite" style="color: #007cff;">Yes, exactly
that's the idea. <br>
<br>
Basically the reset domain knowns which amdgpu devices it
needs to reset together. <br>
<br>
If you then represent that so that you always have a hive
even when you only have one device in it, or if you put an
array of devices which needs to be reset together into the
reset domain doesn't matter. <br>
<br>
Maybe go for the later approach, that is probably a bit
cleaner and less code to change. <br>
<br>
Christian. <br>
</blockquote>
<br>
<br>
Unfortunately this approach raises also a few difficulties -
<br>
First - if holding array of devices in reset_domain then when
you come to GPU reset function you don't really know which
adev is the one triggered the reset and this is actually
essential to some procedures like emergency restart. <br>
</blockquote>
</blockquote>
<br>
What is "emergency restart"? That's not some requirement I know
about.<br>
</blockquote>
<p><br>
</p>
<p>Emergency restart is something u can see at the beginning of
amdgpu_gpu_recover function - it's a historical work around for
some type of ASICs who weren't able to do full reset I think. We
should eventually remove it bu for now I thin it's still in use. <br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:6cf84065-6341-3f96-4e09-ab71796241ec@amd.com"> <br>
<blockquote type="cite" cite="mid:cdce4608-9ade-ac23-b957-6d38a3e2b55a@amd.com">
<blockquote type="cite" cite="mid:0b2657b0-41b6-53b1-f087-1fa0fb821839@amd.com"> <br>
Second - in XGMI case we must take into account that one of
the hive members might go away in runtime (i could do echo 1
> /sysfs/pci_id/remove on it for example at any moment) -
so now we need to maintain this array and mark such entry with
NULL probably on XGMI node removal , and then there might be
hot insertion and all this adds more complications. <br>
<br>
I now tend to prefer your initial solution for it's simplicity
and the result will be what we need - <br>
<br>
"E.g. in the reset code (either before or after the reset,
that's debatable) you do something like this: <br>
<br>
for (i = 0; i < num_ring; ++i) <br>
cancel_delayed_work(ring[i]->scheduler....) <br>
cancel_work(adev->ras_work); <br>
cancel_work(adev->iofault_work); <br>
cancel_work(adev->debugfs_work); <br>
" <br>
</blockquote>
</blockquote>
<br>
Works for me. I already expected that switching over the reset to
be based on the reset context wouldn't be that easy.<br>
<br>
Regards,<br>
Christian.<br>
</blockquote>
<p><br>
</p>
<p>Ok - i will resend a patch.</p>
<p>Andrey</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:6cf84065-6341-3f96-4e09-ab71796241ec@amd.com"> <br>
<blockquote type="cite" cite="mid:cdce4608-9ade-ac23-b957-6d38a3e2b55a@amd.com">
<blockquote type="cite" cite="mid:0b2657b0-41b6-53b1-f087-1fa0fb821839@amd.com"> <br>
Let me know what you think. <br>
<br>
Andrey </blockquote>
</blockquote>
<br>
</blockquote>
</body>
</html>