From d12146f9e48aefc69c62d878c965c868fba3d526 Mon Sep 17 00:00:00 2001 From: Memphiz Date: Thu, 15 Jan 2015 09:41:17 +0100 Subject: [PATCH] [Wetek.Play/amlkernel] - fix race condition in ge2d_wq. 1. thread 1 is here https://github.com/codesnake/linux-amlogic/blob/master/drivers/amlogic/display/ge2d/ge2d_wq.c#L147 2. thread 2 is here https://github.com/codesnake/linux-amlogic/blob/master/drivers/amlogic/display/ge2d/ge2d_wq.c#L855 3. thread 2 is running and enters the if condition because state is GE2D_STATE_RUNNING 4. thread 2 gets interrupted by thread 1 thread 1 jumps over the if and sets GE2D_STATE_IDLE and is done 5. back to thread 2 which now sets state GE2D_STATE_REMOVING_WQ and calls wait_for_completion 6. thread2 will never return because thread 1 is done already and won't signal the event destroy_ge2d_work_queue is called from https://github.com/codesnake/linux-amlogic/blob/master/drivers/amlogic/amports/amvideocap.c#L320 in my use case and will block there forever - and basically blocks down to the read in the userspace which tries to read the current captured frame --- .../linux/80-ge2d-fix-ge2d_state-race.patch | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 projects/WeTek_Play/patches/linux/80-ge2d-fix-ge2d_state-race.patch diff --git a/projects/WeTek_Play/patches/linux/80-ge2d-fix-ge2d_state-race.patch b/projects/WeTek_Play/patches/linux/80-ge2d-fix-ge2d_state-race.patch new file mode 100644 index 0000000000..8e5a48983f --- /dev/null +++ b/projects/WeTek_Play/patches/linux/80-ge2d-fix-ge2d_state-race.patch @@ -0,0 +1,53 @@ +--- a/include/linux/amlogic/ge2d/ge2d_wq.h 2015-01-14 00:46:24.916408987 +0100 ++++ b/include/linux/amlogic/ge2d/ge2d_wq.h 2015-01-14 00:45:24.233715480 +0100 +@@ -81,6 +81,7 @@ + ge2d_event_t event ; + int irq_num; + int ge2d_state; ++ spinlock_t state_lock; //for sync access to ge2d_state + int process_queue_state; + }ge2d_manager_t ; + +--- a/drivers/amlogic/display/ge2d/ge2d_wq.c 2015-01-14 00:59:18.775744127 +0100 ++++ b/drivers/amlogic/display/ge2d/ge2d_wq.c 2015-01-14 00:58:59.440160948 +0100 +@@ -144,9 +144,11 @@ + }while(pos!=head); + ge2d_manager.last_wq=wq; + exit: ++ spin_lock(&ge2d_manager.state_lock); + if(ge2d_manager.ge2d_state==GE2D_STATE_REMOVING_WQ) +- complete(&ge2d_manager.event.process_complete); ++ complete(&ge2d_manager.event.process_complete); + ge2d_manager.ge2d_state=GE2D_STATE_IDLE; ++ spin_unlock(&ge2d_manager.state_lock); + return ret; + } + +@@ -854,8 +856,17 @@ + spin_unlock(&ge2d_manager.event.sem_lock); + if((ge2d_manager.current_wq==ge2d_work_queue)&&(ge2d_manager.ge2d_state== GE2D_STATE_RUNNING)) + { +- ge2d_manager.ge2d_state=GE2D_STATE_REMOVING_WQ; +- wait_for_completion(&ge2d_manager.event.process_complete); ++ // check again with lock ++ int wasRunning = 0; ++ spin_lock(&ge2d_manager.state_lock); ++ if (ge2d_manager.ge2d_state== GE2D_STATE_RUNNING) ++ { ++ ge2d_manager.ge2d_state=GE2D_STATE_REMOVING_WQ; ++ wasRunning = 1; ++ } ++ spin_unlock(&ge2d_manager.state_lock); ++ if (wasRunning) ++ wait_for_completion(&ge2d_manager.event.process_complete); + ge2d_manager.last_wq=NULL; //condition so complex ,simplify it . + }//else we can delete it safely. + +@@ -902,6 +913,7 @@ + //prepare bottom half + + spin_lock_init(&ge2d_manager.event.sem_lock); ++ spin_lock_init(&ge2d_manager.state_lock); + sema_init (&ge2d_manager.event.cmd_in_sem,1); + init_waitqueue_head (&ge2d_manager.event.cmd_complete); + init_completion(&ge2d_manager.event.process_complete);