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 <imre.deak@nokia.com>

diff --git a/pvr/pvr_bridge_k.c b/pvr/pvr_bridge_k.c
index eade118..82b2d19 100644 (file)
--- a/pvr/pvr_bridge_k.c
+++ b/pvr/pvr_bridge_k.c
@@ -24,6 +24,8 @@
  *
  ******************************************************************************/
 
+#include <linux/sched.h>
+
 #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 (file)
--- 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 (file)
--- 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);