<div dir="ltr">It turns out that no concurrent writes occur at all. I added mutex locks and tests with WARN messages to the descriptor creation functions and copy, but nothing at all showed up. The only issue is descriptors being written to while being copied. I sent a patch upstream which has couple of issues fixed. It doesn't slow down Metro Exodus, which is nice.<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 20, 2019 at 7:29 AM Derek Lesho <<a href="mailto:dlesho@codeweavers.com">dlesho@codeweavers.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div>
    <p>On 9/19/19 12:43 PM, Derek Lesho wrote:<br>
    </p>
    <blockquote type="cite">
      
      On 9/19/19 11:12 AM, Conor McCarthy wrote:<br>
      <blockquote type="cite">
        
        <div dir="ltr">
          <div class="gmail_quote">
            <div dir="ltr" class="gmail_attr">On Fri, Sep 20, 2019 at
              1:32 AM Derek Lesho <<a href="mailto:dlesho@codeweavers.com" target="_blank">dlesho@codeweavers.com</a>>
              wrote:<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Also, in your patch, I
              noticed that you intentionally unlock before <br>
              performing the copy or overwrite.  This puzzles me, if one
              thread waits <br>
              on the global mutex for access to a descriptor's view to
              dereference, <br>
              and we unlock the mutex for them when the view is NULL,
              they may crash <br>
              when trying to decrement the reference of the NULL view.<br>
            </blockquote>
            <div><br>
            </div>
            <div>Yes, it's missing a null check after locking the mutex.
              If it's null after locking then the function can return
              because it means another thread has destroyed the
              descriptor. I think the only remaining problem is the old
              one of a single mutex being used for all descriptors.
              Maybe performance can be improved.<br>
            </div>
          </div>
        </div>
      </blockquote>
      Even if one thread destroys a descriptor, and then we let both
      concurrently write their data, we will leak the Vulkan handle of
      written by the thread which writes it to the descriptor first and
      gets overwritten.<br>
    </blockquote>
    <p>I did some testing with your patch as is, and compared it with a
      solution where we destroy the vulkan handle after unlocking the
      global mutex.  The difference was within the margin of error.</p>
    <p>I also tried adding a dedicated mutex for this purpose, but that
      didn't have much of an affect on performance either.  A dedicated
      spinlock actually worsened performance, even without the Vulkan
      call inside the lock.<br>
    </p>
    <p>If we can confirm that no resource leakage is occurring, the
      patch you sent is probably fine as is.</p>
  </div>

</blockquote></div>