[Spice-devel] [PATCH 0/8] Win32 driver fixes for 0.6.0

[Yonit Halperin]

On 08/20/2010 09:54 PM, alexl at redhat.com wrote:
> From: Alexander Larsson<alexl at redhat.com>
>
> This is a bunch of changes I think we we need before the 0.6.0 release.
>
> They do:
>
> * Make SurfaceInfo global to fix potential memory corruption (#29053)
> * Make malloc_sem global since it protects a the global mspace data
> * always free all queued resources before allocating to keep down
>    fragmentation
> * Speed up surface id allocation
>
> I additionally have some outstanding questions for the driver:
>
> There are a lot of data that are not protected by any locks that
> change during runtime. Both in the pdev (things like
> e.g. FPUSave, update_surface_id and update_area) and in the DevRes
> (like the caches and surfaces_used). With GDI apparently allowing
> multiple concurrent calls to different surfaces this looks pretty
> unsafe.
>

> With surfaces_info global we avoid the problem with late resource
> freeing accessing the wrong surface_info[id]. However, isn't there
> also a possible problem where the pdev is not only inactive, but also
> disabled. DrvDisablePDEV frees the pdev, which any outstanding
> surface_info points to, so the late free would access a freed
> pointer. Is there some way to have the surface resources ref the pdev
> to make it not be disabled? Or is something else guaranteeing this
> already?
>
In FreeDelSurface there is no problem, since you access surface_info 
through an active pdev.
Regarding DrvDeleteDeviceBitmap: I understood from Izik, at least from 
his observations, that before disabling a pdev (DrvDisablePDEV), Windows 
calls DrvDeleteDeviceBitmap for all the previously created device 
bitmaps. Since all the calls to GetSurfaceId should be preformed on 
valid device bitmaps, I assume their pdev is still alive.

> Alexander Larsson (8):
>    Move SurfaceInfo structure
>    Add helper functions for surface info<->  id mapping
>    Move SurfaceInfo to global data
>    Always release free resources when allocating
>    Make malloc_sem global
>    Move surface in use check to helper function
>    Store surfaces_used in a bit-array
>    Add simple cache to speed up surface allocation
>
>   display/driver.c  |   40 ++++----------------
>   display/qxldd.h   |   36 +++++++++---------
>   display/res.c     |  103 +++++++++++++++++++++++++++++++----------------------
>   display/surface.c |   10 +++---
>   display/surface.h |   92 +++++++++++++++++++++++++++++++++++++++--------
>   5 files changed, 168 insertions(+), 113 deletions(-)
>