From 147fca6daf85ca62b926829a048dfa2724c0ea94 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Thu, 29 Apr 2010 14:28:37 +0300 Subject: [PATCH] gpu: pvr: fix lockdep problem On the CLK_PRE_RATE_CHANGE: 1. lock A <- __blocking_notifier_call_chain:nh->rwsem 2. lock B <- vdd2_pre_post_func:gPVRSRVLock 3. unlock A On the CLK_POST_RATE_CHANGE/CLK_ABORT_RATE_CHANGE: 4. lock A 5. unlock B 6. unlock A The above has an ABA lock pattern which triggers the warning. This can't lead to a dead lock though since at 3. we always release A, before it's again acquired at 4. To avoid the warning use a wait queue based approach so that we can unlock B before 3. Fixes: NB#166168 - PVR: possible circular locking dependency detected Signed-off-by: Imre Deak --- pvr/pvr_bridge_k.c | 74 ++++++++++++++++++++++++++++++++++++++++++++- pvr/pvr_bridge_km.h | 27 ++++------------- pvr/sysutils.c | 6 ++-- 3 files changed, 82 insertions(+), 25 deletions(-) diff --git a/pvr/pvr_bridge_k.c b/pvr/pvr_bridge_k.c index eade118..82b2d19 100644 --- a/pvr/pvr_bridge_k.c +++ b/pvr/pvr_bridge_k.c @@ -24,6 +24,8 @@ * ******************************************************************************/ +#include + #include "img_defs.h" #include "services.h" #include "pvr_bridge.h" @@ -39,7 +41,77 @@ #include "bridged_pvr_bridge.h" /* Global driver lock protecting all HW and SW state tracking objects. */ -struct mutex gPVRSRVLock; +static struct mutex gPVRSRVLock; +static int pvr_dvfs_active; +static DECLARE_WAIT_QUEUE_HEAD(pvr_dvfs_wq); + +/* + * The pvr_dvfs_* interface is needed to suppress a lockdep warning in + * the following situation during a clock rate change: + * On the CLK_PRE_RATE_CHANGE: + * 1. lock A <- __blocking_notifier_call_chain:nh->rwsem + * 2. lock B <- vdd2_pre_post_func:gPVRSRVLock + * 3. unlock A + * + * On the CLK_POST_RATE_CHANGE/CLK_ABORT_RATE_CHANGE: + * 4. lock A + * 5. unlock B + * 6. unlock A + * + * The above has an ABA lock pattern which triggers the warning. This can't + * lead to a dead lock though since at 3. we always release A, before it's + * again acquired at 4. To avoid the warning use a wait queue based approach + * so that we can unlock B before 3. + */ +void pvr_dvfs_lock(void) +{ + mutex_lock(&gPVRSRVLock); + pvr_dvfs_active = 1; + mutex_unlock(&gPVRSRVLock); +} + +void pvr_dvfs_unlock(void) +{ + mutex_lock(&gPVRSRVLock); + pvr_dvfs_active = 0; + wake_up(&pvr_dvfs_wq); + mutex_unlock(&gPVRSRVLock); +} + +static void pvr_dvfs_wait_active(void) +{ + while (pvr_dvfs_active) { + DEFINE_WAIT(pvr_dvfs_wait); + prepare_to_wait(&pvr_dvfs_wq, &pvr_dvfs_wait, + TASK_UNINTERRUPTIBLE); + mutex_unlock(&gPVRSRVLock); + schedule(); + mutex_lock(&gPVRSRVLock); + finish_wait(&pvr_dvfs_wq, &pvr_dvfs_wait); + } +} + +void pvr_lock(void) +{ + mutex_lock(&gPVRSRVLock); + pvr_dvfs_wait_active(); +} + +void pvr_unlock(void) +{ + mutex_unlock(&gPVRSRVLock); +} + +int pvr_is_locked(void) +{ + return mutex_is_locked(&gPVRSRVLock); +} + +void pvr_init_lock(void) +{ + mutex_init(&gPVRSRVLock); +} + #if defined(DEBUG_BRIDGE_KM) static off_t printLinuxBridgeStats(char *buffer, size_t size, off_t off); diff --git a/pvr/pvr_bridge_km.h b/pvr/pvr_bridge_km.h index ea3330f..b075628 100644 --- a/pvr/pvr_bridge_km.h +++ b/pvr/pvr_bridge_km.h @@ -32,27 +32,12 @@ #include "pvr_bridge.h" #include "perproc.h" -extern struct mutex gPVRSRVLock; - -static inline void pvr_lock(void) -{ - mutex_lock(&gPVRSRVLock); -} - -static inline void pvr_unlock(void) -{ - mutex_unlock(&gPVRSRVLock); -} - -static inline int pvr_is_locked(void) -{ - return mutex_is_locked(&gPVRSRVLock); -} - -static inline void pvr_init_lock(void) -{ - mutex_init(&gPVRSRVLock); -} +void pvr_lock(void); +void pvr_unlock(void); +int pvr_is_locked(void); +void pvr_init_lock(void); +void pvr_dvfs_lock(void); +void pvr_dvfs_unlock(void); enum PVRSRV_ERROR LinuxBridgeInit(void); void LinuxBridgeDeInit(void); diff --git a/pvr/sysutils.c b/pvr/sysutils.c index 3f51465..044a8ee 100644 --- a/pvr/sysutils.c +++ b/pvr/sysutils.c @@ -144,16 +144,16 @@ static int vdd2_pre_post_func(struct notifier_block *n, unsigned long event, cnd->rate); if (CLK_PRE_RATE_CHANGE == event) { - pvr_lock(); + pvr_dvfs_lock(); PVR_TRACE("vdd2_pre_post_func: CLK_PRE_RATE_CHANGE event"); vdd2_pre_func(n, event, ptr); } else if (CLK_POST_RATE_CHANGE == event) { PVR_TRACE("vdd2_pre_post_func: CLK_POST_RATE_CHANGE event"); vdd2_post_func(n, event, ptr); - pvr_unlock(); + pvr_dvfs_unlock(); } else if (CLK_ABORT_RATE_CHANGE == event) { PVR_TRACE("vdd2_pre_post_func: CLK_ABORT_RATE_CHANGE event"); - pvr_unlock(); + pvr_dvfs_unlock(); } else { printk(KERN_ERR "vdd2_pre_post_func: unexpected event (%lu)\n", event); -- 2.39.5