<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 26, 2017 at 3:15 PM, Frediano Ziglio <span dir="ltr"><<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div style="font-family:"times new roman","new york",times,serif;font-size:12pt;color:rgb(0,0,0)"><div>The warning is mainly there to notify the guest is sending commands for a not existing surface.<br></div><div>This should not create issues unless the surface is used as source for other commands.<br></div></div></div></blockquote><div><br></div><div>Practically, Win10 does not produce 'move' operations. However, Win8.1 does. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div style="font-family:"times new roman","new york",times,serif;font-size:12pt;color:rgb(0,0,0)"><div></div><div>The warning was removed as this allows the guest to fill server logs and some drivers send<br></div><div>some spurious commands.<br></div><div>There are no leaks in either 0.12 and 0.13.<br></div></div></div></blockquote><div><br></div><div>I saw good reproducibility on 0.12.4 and no reproducibility on 0.12.8</div><div>Do you have any idea why this can happen? </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div style="font-family:"times new roman","new york",times,serif;font-size:12pt;color:rgb(0,0,0)"><div></div><div>Saying that however I consider this something to be fixed. Considering this sequence:<br></div><div>1- system draw something<br></div><div>2- system change resolution<br></div><div>3- system draw something else<br></div><div>is possible that command generated from 1 is deferred after the resolution is changed basically<br></div><div>doing the draw on a missing or wrong surface. This potentially can lead so some wrong rendering.<br></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div style="font-family:"times new roman","new york",times,serif;font-size:12pt;color:rgb(0,0,0)"><div>Your patches seems to me a bit overwhelming and complex for the task.<br></div><div>Considering that, as you reported, without deferring the drawing the commands are serialized<br></div><div>and there are no warning this means that commands queued in step 3 cannot be executed before<br></div><div>2 finishes (as resolution change is not deferred) </div></div></div></blockquote><div><br></div><div>This is another question (potential existing problem) not related to rendering thread and the degradation it</div><div>can cause. I saw several reports,  for example </div><div><a href="https://bugs.freedesktop.org/show_bug.cgi?id=94270">https://bugs.freedesktop.org/show_bug.cgi?id=94270</a></div><div><br></div><div> <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><div style="font-family:"times new roman","new york",times,serif;font-size:12pt;color:rgb(0,0,0)"><div>so the only problem you can have is that</div><div>commands queued in 1 are executed after/while step 2. I would simply add a "generation" field<br></div><div>(increased before executing resolution change) attached to every command and checked in<br></div><div>the working thread (so being able to ignore a command is the command generation is not<br></div><div>the current one). This solution would be much easier to implement and also could be used<br></div><div>to ignoring command executed after the stop command (just increase the generation during<br></div><div>the stop request).<br></div><div><br></div></div></div></blockquote><div>This can reduce the frequency of the problem, but does not solve the root cause.</div><div>Assume:</div><div>A1. (thread) is checking this number exactly before sending the command down</div><div>A2. (thread)  the check os OK, decides to send the command</div><div>B1. (system callback) the driver receives change of mode<br></div><div>A3. (thread) sends the command</div><div><br></div><div>As threads A and B are independent, the driver can never be sure the check it did before</div><div>sending command is still correct when it sends the command.</div><div><br></div><div> <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><div style="font-family:"times new roman","new york",times,serif;font-size:12pt;color:rgb(0,0,0)"><div></div><div>Frediano<br></div><div><br></div><hr id="gmail-m_-1753628170294443058zwchr"><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:helvetica,arial,sans-serif;font-size:12pt"><div><div class="gmail-h5"><br><div dir="ltr">It would be very helpful to receive a feedback regarding this problem, especially:<div>- Do you see the reason why this erroneous flow on 'old' spice server (0.12.4, for example) can lead to significant device memory leak. </div><div>- If yes: Is spice server 0.12.4 still actual in the field?</div><div>- The warning was removed from 0.13 server - does it mean this flow does not present any problem anymore? </div><div>- Finally - do we need this kind of fix in the driver or not?</div><div><br></div><div>Thanks,</div><div>Yuri</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Apr 16, 2017 at 10:43 PM, Yuri Benditovich <span dir="ltr"><<a href="mailto:yuri.benditovich@daynix.com" target="_blank">yuri.benditovich@daynix.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This is result of investigation of HCK tests "WHQL FPO optimization check for kernel video drivers" on<br>
latest upstream driver.<br>
These tests execute in aloop fast change of display mode and full screen rendering between changes.<br>
When they run on qxl-wddm-dod driver with separate thread that pushes drawables to the host and with<br>
spice server 0.12.4 (rhel-7.2) there are warning printouts of spice server:<br>
"rendering incorrect from now on: get_drawable" and sometimes (3/10) the guest system<br>
stops responding (qxl-wddm-dod driver is in infinite wait for memory allocation).<br>
In case of fast change of display mode it is possible that the driver creates drawable objects<br>
in selected display mode but sends them down in the middle of the change or after change of display mode.<br>
This can cause failure in validation of drawable in spice server.<br>
The same tests do not produce any warnings and system stuck is not reproduced with 0.16 release of the<br>
driver (before implementation of separate thread)<br>
When these tests run on spice server 0.12.8 there are the same warnings but guest stuck was not<br>
reproduced in 10 runs.<br>
In spice server 0.13 these warnings removed from spice server code and guest stuck also was not<br>
reproduced in 10 runs.<br>
It is unclear which commit in spice server 0.12 makes the change, if any.<br>
To avoid inconsistency between drawables and current display mode this set of patches implements<br>
synchronization between display mode change and sending drawables to the host.<br>
Each successful processing of rendering callback creates array of drawables, places it to internal<br>
ring, increments counter of outstanding operations and triggers processing in separate thread.<br>
The thread processes drawables, pushes them to command ring and decrements the counter of outstanding<br>
operations.<br>
When the OS changes display mode, the driver waits until the thread pushes all the queued drawables to the<br>
host, disables queuing of drawables, executes display mode switch and then enables queuing of drawables.<br>
<br>
Yuri Benditovich (3):<br>
  Move code for discarding drawable to separate procedure<br>
  Implement rendering state machine<br>
  Synchronize display mode change and pushing drawables<br>
<br>
 qxldod/QxlDod.cpp |  35 +++++++++++++++----<br>
 qxldod/QxlDod.h   | 102 ++++++++++++++++++++++++++++++<wbr>++++++++++++++++++++++++<br>
 2 files changed, 130 insertions(+), 7 deletions(-)<br>
<span class="gmail-m_-1753628170294443058HOEnZb"><span style="color:rgb(136,136,136)"><br>
--<br>
2.7.0.windows.1<br>
<br>
</span></span></blockquote></div><br></div>
<br></div></div>______________________________<wbr>_________________<br>Spice-devel mailing list<br><a href="mailto:Spice-devel@lists.freedesktop.org" target="_blank">Spice-devel@lists.freedesktop.<wbr>org</a><br><a href="https://lists.freedesktop.org/mailman/listinfo/spice-devel" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/spice-devel</a><br></blockquote><div><br></div></div></div></blockquote></div><br></div></div>