<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 6/10/20 3:54 PM, Andrey Grodzovsky
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:b415e3d1-eed9-9b11-b8c1-c85c7b57eb93@amd.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <p><br>
      </p>
      <div class="moz-cite-prefix">On 6/10/20 6:15 AM, Thomas Hellström
        (Intel) wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:eb9e5896-1f16-2102-350a-1e64d9af7ea8@shipmail.org">
        <p><br>
        </p>
        <div class="moz-cite-prefix">On 6/9/20 7:21 PM, Koenig,
          Christian wrote:<br>
        </div>
        <blockquote type="cite"
          cite="mid:f36c1fa1-bbee-477a-9cb2-ed2726f27eef@email.android.com">
          <div dir="auto">
            <div><br>
              <div class="gmail_extra"><br>
                <div class="gmail_quote">Am 09.06.2020 18:37 schrieb
                  "Grodzovsky, Andrey" <a class="moz-txt-link-rfc2396E"
                    href="mailto:Andrey.Grodzovsky@amd.com"
                    moz-do-not-send="true"><Andrey.Grodzovsky@amd.com></a>:<br
                    type="attribution">
                  <blockquote class="quote" style="margin:0 0 0
                    .8ex;border-left:1px #ccc solid;padding-left:1ex">
                    <div><font size="2"><span style="font-size:11pt">
                          <div><br>
                            On 6/5/20 2:40 PM, Christian König wrote:<br>
                            > Am 05.06.20 um 16:29 schrieb Andrey
                            Grodzovsky:<br>
                            >><br>
                            >> On 5/11/20 2:45 AM, Christian König
                            wrote:<br>
                            >>> Am 09.05.20 um 20:51 schrieb
                            Andrey Grodzovsky:<br>
                            >>>> Signed-off-by: Andrey
                            Grodzovsky <a class="moz-txt-link-rfc2396E"
                              href="mailto:andrey.grodzovsky@amd.com"
                              moz-do-not-send="true"><andrey.grodzovsky@amd.com></a><br>
                            >>>> ---<br>
                            >>>>  
                            drivers/gpu/drm/ttm/ttm_bo.c    | 22
                            +++++++++++++++++++++-<br>
                            >>>>  
                            include/drm/ttm/ttm_bo_driver.h |  2 ++<br>
                            >>>>   2 files changed, 23
                            insertions(+), 1 deletion(-)<br>
                            >>>><br>
                            >>>> diff --git
                            a/drivers/gpu/drm/ttm/ttm_bo.c <br>
                            >>>>
                            b/drivers/gpu/drm/ttm/ttm_bo.c<br>
                            >>>> index c5b516f..eae61cc
                            100644<br>
                            >>>> ---
                            a/drivers/gpu/drm/ttm/ttm_bo.c<br>
                            >>>> +++
                            b/drivers/gpu/drm/ttm/ttm_bo.c<br>
                            >>>> @@ -1750,9 +1750,29 @@ void
                            ttm_bo_unmap_virtual(struct <br>
                            >>>> ttm_buffer_object *bo)<br>
                            >>>>      
                            ttm_bo_unmap_virtual_locked(bo);<br>
                            >>>>      
                            ttm_mem_io_unlock(man);<br>
                            >>>>   }<br>
                            >>>>
                            +EXPORT_SYMBOL(ttm_bo_unmap_virtual);<br>
                            >>>>   +void
                            ttm_bo_unmap_virtual_address_space(struct
                            ttm_bo_device *bdev)<br>
                            >>>> +{<!-- --><br>
                            >>>> +    struct
                            ttm_mem_type_manager *man;<br>
                            >>>> +    int i;<br>
                            >>>>  
                            -EXPORT_SYMBOL(ttm_bo_unmap_virtual);<br>
                            >>><br>
                            >>>> +    for (i = 0; i <
                            TTM_NUM_MEM_TYPES; i++) {<!-- --><br>
                            >>>> +        man =
                            &bdev->man[i];<br>
                            >>>> +        if
                            (man->has_type &&
                            man->use_type)<br>
                            >>>> +           
                            ttm_mem_io_lock(man, false);<br>
                            >>>> +    }<br>
                            >>><br>
                            >>> You should drop that it will
                            just result in a deadlock warning for <br>
                            >>> Nouveau and has no effect at
                            all.<br>
                            >>><br>
                            >>> Apart from that looks good to
                            me,<br>
                            >>> Christian.<br>
                            >><br>
                            >><br>
                            >> As I am considering to re-include
                            this in V2 of the patchsets, can <br>
                            >> you clarify please why this will
                            have no effect at all ?<br>
                            ><br>
                            > The locks are exclusive for Nouveau to
                            allocate/free the io address <br>
                            > space.<br>
                            ><br>
                            > Since we don't do this here we don't
                            need the locks.<br>
                            ><br>
                            > Christian.<br>
                            <br>
                            <br>
                            So basically calling unmap_mapping_range
                            doesn't require any extra <br>
                            locking around it and whatever locks are
                            taken within the function <br>
                            should be enough ?<br>
                          </div>
                        </span></font></div>
                  </blockquote>
                </div>
              </div>
            </div>
            <div dir="auto"><br>
            </div>
            <div dir="auto">
              <div class="gmail_extra">
                <div class="gmail_quote">
                  <blockquote class="quote" style="margin:0 0 0
                    .8ex;border-left:1px #ccc solid;padding-left:1ex">
                    <div><font size="2"><span style="font-size:11pt"> </span></font></div>
                  </blockquote>
                </div>
              </div>
            </div>
            <div dir="auto"><br>
            </div>
            <div dir="auto">I think so, yes.</div>
            <div dir="auto"><br>
            </div>
            <div dir="auto">Christian.</div>
          </div>
        </blockquote>
        <p>Yes, that's true. However, without the bo reservation,
          nothing stops a PTE from being immediately re-faulted back
          again. Even while unmap_mapping_range() is running. </p>
      </blockquote>
      <p><br>
      </p>
      <p>Can you explain more on this - specifically, which function to
        reserve the BO, why BO reservation would prevent re-fault of the
        PTE ?</p>
    </blockquote>
    <p>The bo reservation is taken in the TTM fault handler and
      temporarily blocks inserting a new PTE. So typically the bo
      reservation is held around unmap_mapping_range() and the buffer
      object operation that triggered it (typically a move or change of
      backing store). <br>
    </p>
    <p>/Thomas</p>
    <p><br>
    </p>
    <p><br>
    </p>
  </body>
</html>