<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><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>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>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>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) 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>Frediano<br></div><div><br></div><hr id="zwchr"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><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:0 0 0 .8ex;border-left:1px #ccc solid;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 ++++++++++++++++++++++++++++++++++++++++++++++++++++++<br> 2 files changed, 130 insertions(+), 7 deletions(-)<br> <span class="HOEnZb"><span data-mce-style="color: #888888;" style="color: #888888;"><br> --<br> 2.7.0.windows.1<br> <br> </span></span></blockquote></div><br></div> <br>_______________________________________________<br>Spice-devel mailing list<br>Spice-devel@lists.freedesktop.org<br>https://lists.freedesktop.org/mailman/listinfo/spice-devel<br></blockquote><div><br></div></div></body></html>