[Spice-devel] UMS memory management

[Yonit Halperin]

On 03/27/2013 10:28 AM, Alon Levy wrote:
>> Hi,
>> On 03/27/2013 06:17 AM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 03/27/2013 09:07 AM, Søren Sandmann wrote:
>>>> Hello,
>>>>
>>>> The following is some notes on what I am planning to do for UMS
>>>> memory
>>>> management. It basically amounts to a rewrite of qxl-surface-ums.c
>>>
>>> Looks / sounds good, a few remarks below (note I'm not an expert on
>>> this
>>> piece
>>> of the spice code).
>>>
>>> <snip>
>>>
>>>>        "in host memory":
>>>>           - no associated ID
>>>>           - has associated pixmap
>>>>           - pixman image may not exist
>>>
>>> Hmm, I assume with host-memory you mean guest memory here, right?
>>> iow
>>> surfaces
>>> will be moved by the driver from video-memory to X-server allocated
>>> memory (which
>>> in a vm is guest memory). I'm assuming this is what you mean in the
>>> rest of
>>> my reply.
>>>
>>> <snip>
>>>
>>>
>>>> Details:
>>>>
>>>> ** IN_VIDEO => IN_HOST
>>>>
>>>> - update_area is issued for the surface in question
>>>> - data is copied to pixman image
>>>> - state is set to IN_HOST
>>>> - destroy command is issued for the ID
>>>
>>> I assume the main use-case for this is freeing up video memory,
>>> unfortunately due to glz compression and the drawing command tree
>>> kept in the spice-server, it is possible that the server itself
>>> still holds a reference to the surface, and sending the destroy
>>> will not end up freeing any memory (atleast not directly), this
>>> is esp. going to be a problem when doing these kind of evictions
>>> with the intention of freeing up a larger block of memory
>>> for a new surface.
>>>
>>> I've been discussing this with Alon when we were in Brno together,
>>> and I think we need a new qxl device io-command for this, where the
>>> driver can tell it wants to video memory to be releases *now*
>>> rather then when the server looses it last ref, and then the
>>> server would need to do an eviction of its own, moving the surface
>>> from (shared) video memory to some memory inside the qemu process.
>>>
>>> Note that we will want to limit the amount of memory which can be
>>> use spice-server-side by these evicted, to avoid this being used
>>> as some kind of DOS attack, but this should be a limit which is
>>> (hardly) ever reached in practice.
>>>
>>> Implementing this won't be trivial, but if we all agree it is
>>> something which we want to have, then I hope we can find someone
>>> in the spice team to start working on this.
>>
>> Actually, glz related drawables don't hold references to surfaces.
>> Surfaces references are kept for rendering purposes. In addition,
>> QXL_IO_DESTROY_SURFACE_WAIT, is aimed for releasing all the
>> references
>> of a surface, and eventually releasing the surface. The only thing
>> that
>> is missing there, is a call to worker->qxl->st->qif->flush_resources
>> for updating the release ring (it can be achieved by calling
>>    QXL_IO_FLUSH_RELEASE from the guest, but as Dave noticed it is
>>    buggy,
>> since qxl_push_free_res doesn't support being called from different
>> threads).
>
> So can we do this without affecting anything? i.e. just an internal server change, no api change, and I don't see how it would cause a big performance change - it should actually only help (it only adds things earlier to the release ring, no?)
>
I don't think there should be a problem with adding the call.
>
>>
>> As long as update_area is called before evicting a surface from the
>> vram, there is no need to copy and keep such surfaces in the server.
>> This will only be relevant if at some point we would like to avoid
>> the
>> rendering, and instead store the rendering commands that are related
>> to
>> the surface.
>>
>> p.s. I think that for wddm, and also for Dave's kms paging, we will
>> need
>> to add QXL_IO_DESTROY_BITMAP_WAIT or something similar.
>>>
>>> Regards,
>>>
>>> Hans
>>> _______________________________________________
>>> Spice-devel mailing list
>>> Spice-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>>