Acked
Frediano
>
> From: "yuri.benditovich@xxxxxxxxxx" <yuri.benditovich@xxxxxxxxxx>
>
> Preparation for further scenarios when memory allocation failure
> can happen and we'll need to use fallback when possible.
> Memory allocation can fail in following cases:
> - when non-forced memory allocation used and the attempt to allocate
> the memory must be as fast as possible, without waits
> - when forced memory allocation used, but the driver already received
> stop command and waits for thread termination. Note that in case
> the VSync control is enabled stop command may happen even after the video
> subsystem executes switch to VGA mode. In such case QEMU will not return
> previously allocated objects (assuming the QXL driver already disabled
> and ignoring callbacks from Spice server in VGA mode).
>
> In case of forced memory allocation the allocation routine waits
> unpredictable time until the request is satisfied. In current commit
> we do not acquire m_MemLock mutex for all this time, but release it
> when entering long wait to allow another caller to try allocating memory.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> ---
> qxldod/QxlDod.cpp | 86
> +++++++++++++++++++++++++++++++++++++++++++++++++------
> qxldod/QxlDod.h | 3 +-
> 2 files changed, 79 insertions(+), 10 deletions(-)
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index aa70f39..f8440d4 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -3064,6 +3064,7 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
> m_FreeOutputs = 0;
> m_Pending = 0;
> m_PresentThread = NULL;
> + m_bActive = FALSE;
> }
>
> QxlDevice::~QxlDevice(void)
> @@ -3490,14 +3491,19 @@ NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION*
> pDispInfo)
> Status = AcquireDisplayInfo(*(pDispInfo));
> if (NT_SUCCESS(Status))
> {
> + m_bActive = TRUE;
> Status = StartPresentThread();
> }
> + if (!NT_SUCCESS(Status)) {
> + m_bActive = FALSE;
> + }
> return Status;
> }
>
> void QxlDevice::QxlClose()
> {
> PAGED_CODE();
> + m_bActive = FALSE;
> StopPresentThread();
> DestroyMemSlots();
> }
> @@ -3945,14 +3951,18 @@ void QxlDevice::WaitForReleaseRing(void)
> {
> PAGED_CODE();
> int wait;
> + BOOLEAN locked;
>
> DbgPrint(TRACE_LEVEL_VERBOSE, ("--->%s\n", __FUNCTION__));
>
> + locked = WaitForObject(&m_MemLock, NULL);
> for (;;) {
> LARGE_INTEGER timeout;
>
> if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
> + ReleaseMutex(&m_MemLock, locked);
> QXL_SLEEP(10);
> + locked = WaitForObject(&m_MemLock, NULL);
> if (!SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
> break;
> }
> @@ -3960,17 +3970,20 @@ void QxlDevice::WaitForReleaseRing(void)
> }
> SPICE_RING_CONS_WAIT(m_ReleaseRing, wait);
>
> - if (!wait) {
> + if (!wait || !m_bActive) {
> break;
> }
>
> + ReleaseMutex(&m_MemLock, locked);
> timeout.QuadPart = -30 * 1000 * 10; //30ms
> WaitForObject(&m_DisplayEvent, &timeout);
> + locked = WaitForObject(&m_MemLock, NULL);
>
> if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
> SyncIo(QXL_IO_NOTIFY_OOM, 0);
> }
> }
> + ReleaseMutex(&m_MemLock, locked);
> DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: <---\n", __FUNCTION__));
> }
>
> @@ -4052,7 +4065,16 @@ void *QxlDevice::AllocMem(UINT32 mspace_type, size_t
> size, BOOL force)
> mspace_malloc_stats(m_MSInfo[mspace_type]._mspace);
> #endif
>
> - locked = WaitForObject(&m_MemLock, NULL);
> + if (force)
> + locked = WaitForObject(&m_MemLock, NULL);
> + else {
> + LARGE_INTEGER doNotWait;
> + doNotWait.QuadPart = 0;
> + locked = WaitForObject(&m_MemLock, &doNotWait);
> + if (!locked) {
> + return NULL;
> + }
> + }
>
> while (1) {
> /* Release lots of queued resources, before allocating, as we
> @@ -4070,17 +4092,20 @@ void *QxlDevice::AllocMem(UINT32 mspace_type, size_t
> size, BOOL force)
> continue;
> }
>
> - if (force) {
> + if (force && m_bActive) {
> /* Ask spice to free some stuff */
> + ReleaseMutex(&m_MemLock, locked);
> WaitForReleaseRing();
> + locked = WaitForObject(&m_MemLock, NULL);
> } else {
> /* Fail */
> break;
> }
> }
> +
> ReleaseMutex(&m_MemLock, locked);
>
> - ASSERT((!ptr && !force) || (ptr >= m_MSInfo[mspace_type].mspace_start &&
> + ASSERT((!ptr && (!force || !m_bActive)) || (ptr >=
> m_MSInfo[mspace_type].mspace_start &&
> ptr <
> m_MSInfo[mspace_type].mspace_end));
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<---%s: ptr 0x%x\n", __FUNCTION__,
> ptr));
> return ptr;
> @@ -4113,6 +4138,9 @@ QXLDrawable *QxlDevice::GetDrawable()
> QXLOutput *output;
>
> output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM, sizeof(QXLOutput) +
> sizeof(QXLDrawable), TRUE);
> + if (!output) {
> + return NULL;
> + }
> output->num_res = 0;
> RESOURCE_TYPE(output, RESOURCE_TYPE_DRAWABLE);
> ((QXLDrawable *)output->data)->release_info.id = (UINT64)output;
> @@ -4128,6 +4156,9 @@ QXLCursorCmd *QxlDevice::CursorCmd()
>
> DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
> output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM, sizeof(QXLOutput) +
> sizeof(QXLCursorCmd), TRUE);
> + if (!output) {
> + return NULL;
> + }
> output->num_res = 0;
> RESOURCE_TYPE(output, RESOURCE_TYPE_CURSOR);
> cursor_cmd = (QXLCursorCmd *)output->data;
> @@ -4277,6 +4308,9 @@ QXLDrawable *QxlDevice::Drawable(UINT8 type, CONST RECT
> *area, CONST RECT *clip,
> DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
>
> drawable = GetDrawable();
> + if (!drawable) {
> + return NULL;
> + }
> drawable->surface_id = surface_id;
> drawable->type = type;
> drawable->effect = QXL_EFFECT_OPAQUE;
> @@ -4420,6 +4454,11 @@ QXLDrawable *QxlDevice::PrepareBltBits (
> alloc_size = BITMAP_ALLOC_BASE + BITS_BUF_MAX - BITS_BUF_MAX %
> line_size;
> alloc_size = MIN(BITMAP_ALLOC_BASE + height * line_size, alloc_size);
> image_res = (Resource*)AllocMem(MSPACE_TYPE_VRAM, alloc_size, TRUE);
> + if (!image_res) {
> + ReleaseOutput(drawable->release_info.id);
> + DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate bitmap for
> drawable\n"));
> + return NULL;
> + }
>
> image_res->refs = 1;
> image_res->free = FreeBitmapImageEx;
> @@ -4450,8 +4489,12 @@ QXLDrawable *QxlDevice::PrepareBltBits (
> alloc_size = height * line_size;
>
> for (; src != src_end; src -= pSrc->Pitch, alloc_size -= line_size) {
> - PutBytesAlign(&chunk, &dest, &dest_end, src,
> - line_size, alloc_size, line_size);
> + if (!PutBytesAlign(&chunk, &dest, &dest_end, src,
> + line_size, alloc_size, line_size)) {
> + ReleaseOutput(drawable->release_info.id);
> + DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional bitmap
> for drawable\n"));
> + return NULL;
> + }
> }
>
> internal->image.bitmap.palette = 0;
> @@ -4472,7 +4515,7 @@ QXLDrawable *QxlDevice::PrepareBltBits (
> return drawable;
> }
>
> -VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> +BOOLEAN QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> UINT8 **end_ptr, UINT8 *src, int size,
> size_t alloc_size, uint32_t alignment)
> {
> @@ -4490,6 +4533,9 @@ VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr,
> UINT8 **now_ptr,
> aligned_size -= aligned_size % alignment;
>
> void *ptr = AllocMem(MSPACE_TYPE_VRAM, size +
> sizeof(QXLDataChunk), TRUE);
> + if (!ptr) {
> + return FALSE;
> + }
> chunk->next_chunk = PA(ptr, m_SurfaceMemSlot);
> ((QXLDataChunk *)ptr)->prev_chunk = PA(chunk, m_SurfaceMemSlot);
> chunk = (QXLDataChunk *)ptr;
> @@ -4510,6 +4556,7 @@ VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr,
> UINT8 **now_ptr,
> *now_ptr = now;
> *end_ptr = end;
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> + return TRUE;
> }
>
> VOID QxlDevice::BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod)
> @@ -4573,6 +4620,11 @@ NTSTATUS QxlDevice::SetPointerShape(_In_ CONST
> DXGKARG_SETPOINTERSHAPE* pSetPoi
> int num_images = 1;
>
> cursor_cmd = CursorCmd();
> + if (!cursor_cmd) {
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> command\n", __FUNCTION__));
> + return STATUS_INSUFFICIENT_RESOURCES;
> + }
> +
> cursor_cmd->type = QXL_CURSOR_SET;
>
> cursor_cmd->u.set.visible = TRUE;
> @@ -4580,6 +4632,12 @@ NTSTATUS QxlDevice::SetPointerShape(_In_ CONST
> DXGKARG_SETPOINTERSHAPE* pSetPoi
> cursor_cmd->u.set.position.y = 0;
>
> res = (Resource *)AllocMem(MSPACE_TYPE_VRAM, CURSOR_ALLOC_SIZE, TRUE);
> + if (!res) {
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor data\n",
> __FUNCTION__));
> + ReleaseOutput(cursor_cmd->release_info.id);
> + return STATUS_INSUFFICIENT_RESOURCES;
> + }
> +
> res->refs = 1;
> res->free = FreeCursorEx;
> res->ptr = this;
> @@ -4616,8 +4674,13 @@ NTSTATUS QxlDevice::SetPointerShape(_In_ CONST
> DXGKARG_SETPOINTERSHAPE* pSetPoi
> end = (UINT8 *)res + CURSOR_ALLOC_SIZE;
> src_end = src + (pSetPointerShape->Pitch * pSetPointerShape->Height *
> num_images);
> for (; src != src_end; src += pSetPointerShape->Pitch) {
> - PutBytesAlign(&chunk, &now, &end, src, line_size,
> - PAGE_SIZE, 1);
> + if (!PutBytesAlign(&chunk, &now, &end, src, line_size,
> + PAGE_SIZE, 1))
> + {
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> bitmap\n", __FUNCTION__));
> + ReleaseOutput(cursor_cmd->release_info.id);
> + return STATUS_INSUFFICIENT_RESOURCES;
> + }
> }
> CursorCmdAddRes(cursor_cmd, res);
> RELEASE_RES(res);
> @@ -4638,6 +4701,11 @@ NTSTATUS QxlDevice::SetPointerPosition(_In_ CONST
> DXGKARG_SETPOINTERPOSITION* pS
> pSetPointerPosition->X,
> pSetPointerPosition->Y));
> QXLCursorCmd *cursor_cmd = CursorCmd();
> + if (!cursor_cmd) {
> + DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> command\n", __FUNCTION__));
> + return STATUS_INSUFFICIENT_RESOURCES;
> + }
> +
> if (pSetPointerPosition->X < 0 || !pSetPointerPosition->Flags.Visible) {
> cursor_cmd->type = QXL_CURSOR_HIDE;
> } else {
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index a1e9634..cbcd11e 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -601,7 +601,7 @@ private:
> void PushCmd(void);
> void WaitForCursorRing(void);
> void PushCursor(void);
> - void PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> + BOOLEAN PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> UINT8 **end_ptr, UINT8 *src, int size,
> size_t alloc_size, uint32_t alignment);
> void AsyncIo(UCHAR Port, UCHAR Value);
> @@ -671,6 +671,7 @@ private:
>
> QXLPresentOnlyRing m_PresentRing[1];
> HANDLE m_PresentThread;
> + BOOLEAN m_bActive;
> };
>
> class QxlDod {
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel