[Spice-devel] [PATCH 5/8] Make malloc_sem global

[Yonit Halperin]

On 08/20/2010 09:54 PM, alexl at redhat.com wrote:
> From: Alexander Larsson<alexl at redhat.com>
>
> It protects shared data (mspace info) so it needs to be shared.
> ---
>   display/driver.c |   22 ++++++----------------
>   display/qxldd.h  |    2 +-
>   display/res.c    |   17 +++++++++++++----
>   3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/display/driver.c b/display/driver.c
> index 742348c..46df8f2 100644
> --- a/display/driver.c
> +++ b/display/driver.c
> @@ -505,24 +505,19 @@ DHPDEV DrvEnablePDEV(DEVMODEW *dev_mode, PWSTR ignore1, ULONG ignore2, HSURF *ig
>           goto err1;
>       }
>
> -    if (!(pdev->malloc_sem = EngCreateSemaphore())) {
> -        DEBUG_PRINT((NULL, 0, "%s: create malloc sem failed\n", __FUNCTION__));
> -        goto err2;
> -    }
> -
>       if (!(pdev->print_sem = EngCreateSemaphore())) {
> -        DEBUG_PRINT((NULL, 0, "%s: create malloc sem failed\n", __FUNCTION__));
> -        goto err3;
> +        DEBUG_PRINT((NULL, 0, "%s: create print sem failed\n", __FUNCTION__));
> +        goto err2;
>       }
>
>       if (!(pdev->cmd_sem = EngCreateSemaphore())) {
>           DEBUG_PRINT((NULL, 0, "%s: create cmd sem failed\n", __FUNCTION__));
> -        goto err4;
> +        goto err3;
>       }
>
>       if (!ResInit(pdev)) {
>           DEBUG_PRINT((NULL, 0, "%s: init res failed\n", __FUNCTION__));
> -        goto err5;
> +        goto err4;
>       }
>
>       RtlCopyMemory(dev_caps,&gdi_info, dev_caps_size);
> @@ -531,14 +526,10 @@ DHPDEV DrvEnablePDEV(DEVMODEW *dev_mode, PWSTR ignore1, ULONG ignore2, HSURF *ig
>       DEBUG_PRINT((NULL, 1, "%s: 0x%lx\n", __FUNCTION__, pdev));
>       return(DHPDEV)pdev;
>
> -err5:
> -    EngDeleteSemaphore(pdev->cmd_sem);
>   err4:
> -    EngDeleteSemaphore(pdev->print_sem);
> -
> +    EngDeleteSemaphore(pdev->cmd_sem);
>   err3:
> -    EngDeleteSemaphore(pdev->malloc_sem);
> -
> +    EngDeleteSemaphore(pdev->print_sem);
>   err2:
>       DestroyPalette(pdev);
>
> @@ -556,7 +547,6 @@ VOID DrvDisablePDEV(DHPDEV in_pdev)
>       ResDestroy(pdev);
>       DestroyPalette(pdev);
>       EngDeleteSemaphore(pdev->cmd_sem);
> -    EngDeleteSemaphore(pdev->malloc_sem);
>       EngDeleteSemaphore(pdev->print_sem);
>       EngFreeMem(pdev);
>       DEBUG_PRINT((NULL, 1, "%s: 0x%lx exit\n", __FUNCTION__, pdev));
> diff --git a/display/qxldd.h b/display/qxldd.h
> index 9f09aa8..7984fc8 100644
> --- a/display/qxldd.h
> +++ b/display/qxldd.h
> @@ -176,6 +176,7 @@ typedef struct SurfaceInfo {
>
>   typedef struct DevRes {
>       MspaceInfo mspaces[NUM_MSPACES];
> +    HSEMAPHORE malloc_sem;
>
>       BOOL need_init;
>       UINT32 update_id;
> @@ -257,7 +258,6 @@ typedef struct PDev {
>       UINT8 *log_buf;
>       UINT32 *log_level;
>
> -    HSEMAPHORE malloc_sem;
>       HSEMAPHORE print_sem;
>       HSEMAPHORE cmd_sem;
>
> diff --git a/display/res.c b/display/res.c
> index b30951f..7773e40 100644
> --- a/display/res.c
> +++ b/display/res.c
> @@ -311,7 +311,7 @@ static void *__AllocMem(PDev* pdev, UINT32 mspace_type, size_t size, BOOL force)
>       ASSERT(pdev, pdev&&  pdev->Res.mspaces[mspace_type]._mspace);
>       DEBUG_PRINT((pdev, 12, "%s: 0x%lx size %u\n", __FUNCTION__, pdev, size));
>
> -    EngAcquireSemaphore(pdev->malloc_sem);
> +    EngAcquireSemaphore(pdev->Res.malloc_sem);
>
>       do {
>           /* Release all queued resources, before allocating, as we
> @@ -325,7 +325,7 @@ static void *__AllocMem(PDev* pdev, UINT32 mspace_type, size_t size, BOOL force)
>           }
>       } while (ptr == NULL&&  force);
>
> -    EngReleaseSemaphore(pdev->malloc_sem);
> +    EngReleaseSemaphore(pdev->Res.malloc_sem);
>       ASSERT(pdev, (!ptr&&  !force) || (ptr>= pdev->Res.mspaces[mspace_type].mspace_start&&
>                                         ptr<  pdev->Res.mspaces[mspace_type].mspace_end));
>       DEBUG_PRINT((pdev, 13, "%s: 0x%lx done 0x%x\n", __FUNCTION__, pdev, ptr));
> @@ -362,6 +362,10 @@ void CleanGlobalRes()
>                   EngFreeMem(global_res[i].surfaces_info);
>                   global_res[i].surfaces_info = NULL;
>               }
> +            if (global_res[i].malloc_sem) {
> +                EngDeleteSemaphore(global_res[i].malloc_sem);
> +                global_res[i].malloc_sem = NULL;
> +            }
>           }
>           EngFreeMem(global_res);
>           global_res = NULL;
> @@ -411,6 +415,11 @@ static void InitRes(PDev *pdev)
>           PANIC(pdev, "Res surfaces_info allocation failed\n");
>       }
>
> +    pdev->Res.malloc_sem = EngCreateSemaphore();
> +    if (!pdev->Res.malloc_sem) {
> +        PANIC(pdev, "Res malloc sem creation failed\n");
> +    }
> +
>       InitMspace(&pdev->Res, MSPACE_TYPE_DEVRAM, pdev->io_pages_virt, pdev->num_io_pages * PAGE_SIZE);
>       InitMspace(&pdev->Res, MSPACE_TYPE_VRAM, pdev->fb, pdev->fb_size);
>       pdev->Res.update_id = *pdev->dev_update_id;
> @@ -1326,12 +1335,12 @@ static CacheImage *AllocCacheImage(PDev* pdev)
>   {
>       RingItem *item;
>       while (!(item = RingGetTail(pdev,&pdev->Res.dynamic->cache_image_lru))) {
> -        EngAcquireSemaphore(pdev->malloc_sem);
> +        EngAcquireSemaphore(pdev->Res.malloc_sem);
>           if (SPICE_RING_IS_EMPTY(pdev->release_ring)) {
>               WaitForReleaseRing(pdev);
>           }
>           EmptyReleaseRing(pdev);
> -        EngReleaseSemaphore(pdev->malloc_sem);
> +        EngReleaseSemaphore(pdev->Res.malloc_sem);
>       }
>       RingRemove(pdev, item);
>       return CONTAINEROF(item, CacheImage, lru_link);

Don't see a problem with making it global, but not sure it is needed. 
Can alloc operations occur on an inactive pdev?