<style>table.customTableClassName {margin-bottom: 10px;border-collapse: collapse;display: table;}.customTableClassName td, .customTableClassName th {border: 1px solid #ddd;}</style><p style="margin:0px;">hi<br><br><span style="text-decoration: underline; color: rgb(0, 176, 240);">I am sorry that I have not responded to your emails during those days. </span></p><p style="margin:0px;"><span style="text-decoration: underline; color: rgb(0, 176, 240);">I was quarantined for 14 days for suspected 2019-ncov pneumonia,I‘m free now. </span></p><p style="margin:0px;"><br>On 4/21/20 11:43 AM, Gerd Hoffmann wrote:<br>> On Sat, Apr 18, 2020 at 02:39:17PM +0800, Caicai wrote:<br>>> When a qxl resource is released, the list that needs to be released is<br>>> fetched from the linked list ring and cleared. When you empty the list,<br>>> instead of trying to determine whether the ttm buffer object for each<br>>> qxl in the list is locked, you release the qxl object and remove the<br>>> element from the list until the list is empty. It was found that the<br>>> linked list was cleared first, and that the lock on the corresponding<br>>> ttm Bo for the QXL had not been released, so that the new qxl could not<br>>> be locked when it used the TTM.<br>> <br>> So the dma_resv_reserve_shared() call in qxl_release_validate_bo() is<br>> unbalanced?  Because the dma_resv_unlock() call in<br>> qxl_release_fence_buffer_objects() never happens due to<br>> qxl_release_free_list() clearing the list beforehand?  Is that correct?<br><br>we observe similar issue: RHEL7 guests crashes in <br>qxl_draw_opaque_fb()<br> qxl_release_fence_buffer_objects() <br><br>crashdump investigation shows that qlx_object was freed and reused,<br>so its original content was re-written.<br><br>At the same time qxl_device have empty release_idr,<br>ant there are no allocated qxl_bo_list entries.<br>i.e. qxl_release_free was really called.<br><br><span style="color: rgb(0, 176, 240); text-decoration: none;">That's exactly right。I found that qxl_bo_list had empted, ttm_buffer_object ( qxl_bo->tbo) which still locked, </span></p><p style="margin:0px;"><span style="color: rgb(0, 176, 240); text-decoration: none;">then other new qxl which needed tbo cannot lockup.</span></p><p style="margin:0px;"><font color="#00b0f0"><br></font>> The only way I see for this to happen is that the guest is preempted<br>> between qxl_push_{cursor,command}_ring_release() and<br>> qxl_release_fence_buffer_objects() calls.  The host can complete the qxl<br>> command then, signal the guest, and the IRQ handler calls<br>> qxl_release_free_list() before qxl_release_fence_buffer_objects() runs.<br><br>We think the same: qxl_release was freed by garbage collector before<br>original thread had called qxl_release_fence_buffer_objects().<br><br>> Looking through the code I think it should be safe to simply swap the<br>> qxl_release_fence_buffer_objects() +<br>> qxl_push_{cursor,command}_ring_release() calls to close that race<br>> window.  Can you try that and see if it fixes the bug for you?<br><br>I'm going to prepare and test such patch but I have one question here:<br>qxl_push_*_ring_release can be called with  interruptible=true and fail<br>How to correctly handle this case? Is the hunk below correct from your POV?</p><p style="margin:0px;"><br></p><p style="margin:0px;"><br>--- a/drivers/gpu/drm/qxl/qxl_ioctl.c<br>+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c<br>@@ -261,12 +261,8 @@ static int qxl_process_single_command(struct qxl_device *qdev,<br>                        apply_surf_reloc(qdev, &reloc_info[i]);<br>        }<br> <br>+       qxl_release_fence_buffer_objects(release);<br>        ret = qxl_push_command_ring_release(qdev, release, cmd->type, true);<br>-       if (ret)<br>-               qxl_release_backoff_reserve_list(release);  <<<< ????<br>-       else<br>-               qxl_release_fence_buffer_objects(release);<br>-<br> out_free_bos:<br> out_free_release:<br><br><span style="color: rgb(0, 176, 240); text-decoration-line: underline;">I'll  test this such patch.</span><br><br>Thank you,<br>  Vasily Averin</p><p style="margin:0px;"><br></p><p style="margin:0px;">Thanks so much for your help.</p>