<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2021-11-09 11:16 p.m., Felix
      Kuehling wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:d27056e0-c563-ced9-326d-b1b35237e77c@amd.com">On
      2021-11-09 6:04 p.m., Philip Yang wrote:
      <br>
      <blockquote type="cite">restore pages work can not find kfd
        process or mm struct if process is
        <br>
        destroyed before drain retry fault work schedule to run, this is
        not
        <br>
        failure, return 0 to avoid dump GPU vm fault kernel log.
        <br>
      </blockquote>
      I wonder if this could also be solved by draining page fault
      interrupts in kfd_process_notifier_release before we remove the
      process from the hash table. Because that function runs while
      holding the mmap lock, we'd need to detect the draining condition
      for the process in the page fault handler before it tries to take
      the mmap lock. Maybe that's even a helpful optimization that
      speeds up interrupt draining by just ignoring all retry faults
      during that time.
      <br>
      <br>
    </blockquote>
    Call svm_range_drain_retry_fault in kfd_process_notifier_release
    before removing the process from hash table solve this race
    condition.<br>
    <blockquote type="cite" cite="mid:d27056e0-c563-ced9-326d-b1b35237e77c@amd.com">That would
      also allow draining faults in svm_range_unmap_from_cpu instead of
      the delayed worker. And I believe that would also elegantly fix
      the vma removal race.
      <br>
    </blockquote>
    <p>vma maybe removed from rbtree before unmap call back (refer to
      below __do_munmap), draining fault in svm_range_unmap_from_cpu can
      not fix vma removal race, also cause mmap write and read lock
      deadlock issue, will change to check if svms->drain_pagefaults
      is set, restore_pages returns true if no VMA is found.<br>
    </p>
    <p>int __do_munmap</p>
    <p>............<br>
    </p>
    <p>    /* Detach vmas from rbtree */<br>
          if (!detach_vmas_to_be_unmapped(mm, vma, prev, end))<br>
              downgrade = false;<br>
      <br>
          if (downgrade)<br>
              mmap_write_downgrade(mm);<br>
      <br>
          unmap_region(mm, vma, prev, start, end);<br>
      <br>
          /* Fix up all other VM information */<br>
          remove_vma_list(mm, vma);<br>
      <br>
      Regards,</p>
    <p>Philip<br>
    </p>
    <blockquote type="cite" cite="mid:d27056e0-c563-ced9-326d-b1b35237e77c@amd.com">
      <br>
      Regards,
      <br>
        Felix
      <br>
      <br>
      <br>
      <blockquote type="cite">
        <br>
        Signed-off-by: Philip Yang <a class="moz-txt-link-rfc2396E" href="mailto:Philip.Yang@amd.com"><Philip.Yang@amd.com></a>
        <br>
        ---
        <br>
          drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 4 ++--
        <br>
          1 file changed, 2 insertions(+), 2 deletions(-)
        <br>
        <br>
        diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
        b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
        <br>
        index 8f77d5746b2c..2083a10b35c2 100644
        <br>
        --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
        <br>
        +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
        <br>
        @@ -2596,7 +2596,7 @@ svm_range_restore_pages(struct
        amdgpu_device *adev, unsigned int pasid,
        <br>
              p = kfd_lookup_process_by_pasid(pasid);
        <br>
              if (!p) {
        <br>
                  pr_debug("kfd process not founded pasid 0x%x\n",
        pasid);
        <br>
        -        return -ESRCH;
        <br>
        +        return 0;
        <br>
              }
        <br>
              if (!p->xnack_enabled) {
        <br>
                  pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
        <br>
        @@ -2610,7 +2610,7 @@ svm_range_restore_pages(struct
        amdgpu_device *adev, unsigned int pasid,
        <br>
              mm = get_task_mm(p->lead_thread);
        <br>
              if (!mm) {
        <br>
                  pr_debug("svms 0x%p failed to get mm\n", svms);
        <br>
        -        r = -ESRCH;
        <br>
        +        r = 0;
        <br>
                  goto out;
        <br>
              }
        <br>
          </blockquote>
    </blockquote>
  </body>
</html>