[Spice-devel] [RFC qxl-wddm-dod 0/3] Synchronization of display mode change and pushing drawables

Thu Apr 27 10:39:24 UTC 2017

On Wed, Apr 26, 2017 at 3:15 PM, Frediano Ziglio <fziglio at redhat.com> 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 send
> some 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 something
> 2- system change resolution
> 3- system draw something else
> is possible that command generated from 1 is deferred after the resolution
> is changed basically
> doing 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 serialized
> and there are no warning this means that commands queued in step 3 cannot
> be executed before
> 2 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
https://bugs.freedesktop.org/show_bug.cgi?id=94270

> so the only problem you can have is that
> commands 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 in
> the working thread (so being able to ignore a command is the command
> generation is not
> the current one). This solution would be much easier to implement and also
> could be used
> to ignoring command executed after the stop command (just increase the
> generation during
> the 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,
> Yuri
>
> On Sun, Apr 16, 2017 at 10:43 PM, Yuri Benditovich <
> yuri.benditovich at daynix.com> 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170427/888d815d/attachment.html>