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