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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel