From 807b40416dadcd925aef081e63524e98f4b274f8 Mon Sep 17 00:00:00 2001 From: Mikhail Aksenov Date: Tue, 30 Apr 2019 09:15:57 +0300 Subject: [PATCH] [Decode] Prevent mt read/write conflict Two concurrent DdiMedia_SyncSurface function calls read and write the same data when the second frame uses the first frame as a reference frame. Enqueue frames order: Frame #0 Frame #1 (uses #0 as a reference) Threads release order: Thread #1 (waits for Frame #1) Thread #0 (waits for Frame #0) Resolution is lock with the mutex the problem source code (read and update StatusReport), so only one thread writes the data, the second thread is waiting. --- media_driver/linux/common/ddi/media_libva.cpp | 5 ++--- .../linux/common/ddi/media_libva_util.h | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/media_driver/linux/common/ddi/media_libva.cpp b/media_driver/linux/common/ddi/media_libva.cpp index 854485dd5cf..292242a184a 100755 --- a/media_driver/linux/common/ddi/media_libva.cpp +++ b/media_driver/linux/common/ddi/media_libva.cpp @@ -3289,6 +3289,8 @@ static VAStatus DdiMedia_SyncSurface ( PDDI_DECODE_CONTEXT decCtx = (PDDI_DECODE_CONTEXT)surface->pDecCtx; if (decCtx && surface->curCtxType == DDI_MEDIA_CONTEXT_TYPE_DECODER) { + DdiMediaUtil_LockGuard guard(&mediaCtx->SurfaceMutex); + Codechal *codecHal = decCtx->pCodecHal; DDI_CHK_NULL(codecHal, "nullptr decCtx->pCodecHal", VA_STATUS_ERROR_INVALID_CONTEXT); @@ -3335,7 +3337,6 @@ static VAStatus DdiMedia_SyncSurface ( if ((tempNewReport.m_codecStatus == CODECHAL_STATUS_SUCCESSFUL) || (tempNewReport.m_codecStatus == CODECHAL_STATUS_ERROR) || (tempNewReport.m_codecStatus == CODECHAL_STATUS_INCOMPLETE)) { - DdiMediaUtil_LockMutex(&mediaCtx->SurfaceMutex); PDDI_MEDIA_SURFACE_HEAP_ELEMENT mediaSurfaceHeapElmt = (PDDI_MEDIA_SURFACE_HEAP_ELEMENT)mediaCtx->pSurfaceHeap->pHeapBase; uint32_t j = 0; @@ -3355,10 +3356,8 @@ static VAStatus DdiMedia_SyncSurface ( if (j == mediaCtx->pSurfaceHeap->uiAllocatedHeapElements) { - DdiMediaUtil_UnLockMutex(&mediaCtx->SurfaceMutex); return VA_STATUS_ERROR_OPERATION_FAILED; } - DdiMediaUtil_UnLockMutex(&mediaCtx->SurfaceMutex); } else { diff --git a/media_driver/linux/common/ddi/media_libva_util.h b/media_driver/linux/common/ddi/media_libva_util.h index 19579a6fe87..c4d1772b4c4 100644 --- a/media_driver/linux/common/ddi/media_libva_util.h +++ b/media_driver/linux/common/ddi/media_libva_util.h @@ -152,6 +152,28 @@ void DdiMediaUtil_LockMutex(PMEDIA_MUTEX_T mutex); //! void DdiMediaUtil_UnLockMutex(PMEDIA_MUTEX_T mutex); +//! +//! \brief Helper inline class intended to simplify mutex lock/unlock +//! operations primarily used as a stack-allocated object. +//! In that case, the compiler guarantees to call the destructor +//! leaving the scope. The class becomes handy in functions +//! where there are several return statements with different +//! exit code value. +//! +class DdiMediaUtil_LockGuard { +private: + PMEDIA_MUTEX_T m_pMutex; +public: + DdiMediaUtil_LockGuard(PMEDIA_MUTEX_T pMutex):m_pMutex(pMutex) + { + DdiMediaUtil_LockMutex(m_pMutex); + } + ~DdiMediaUtil_LockGuard() + { + DdiMediaUtil_UnLockMutex(m_pMutex); + } +}; + //! //! \brief Destroy semaphore //!