From: Mike Isely <isely@pobox.com>
Date: Wed, 9 Apr 2008 08:14:11 +0000 (-0300)
Subject: V4L/DVB (7714): pvrusb2: Fix hang on module removal
X-Git-Tag: v2.6.26-rc1~1084^2~15
X-Git-Url: http://pilppa.com/gitweb/?a=commitdiff_plain;h=18ecbb4771eb0ecf297e996966b3c42f69cd6c02;p=linux-2.6-omap-h63xx.git

V4L/DVB (7714): pvrusb2: Fix hang on module removal

The pvrusb2 driver was getting had by this scenario:

1. Task A calls kthread_stop() for task B.
2. Before exiting, then Task B calls kthread_stop() for task C.

The problem is, kthread_stop() wants to allocate an internal resource
to itself (i.e. acquire a lock), which won't be released until
kthread_stop() returns.  But kthread_stop() won't return until task B
is dead.  But task B won't die until it finishes its call to
kthread_stop() for task C, and that will block waiting on the resource
already allocated inside task A.  Deadlock.

With the pvrusb2 driver, task A is the caller to pvr_exit(), task B is
the control thread run inside of pvrusb2-context.c, and task C is any
worker thread run inside of pvrusb2-hdw.c.

This problem got introduced by the previous threading setup change,
which was itself an attempt to fix a module tear-down race (which it
actually did fix).  The lesson here is that a task being waited on as
part of a kthread_stop() simply cannot be allow to also issue a
kthread_stop() - or we make sure not to issue the enclosing
kthread_stop() until we know that the inner kthread_stop() has
completed first.  The solution for the pvrusb2 driver is some hackish
code which changes the main control thread tear down into a two step
process.  This then makes it possible to delay issuing the
kthread_stop() on the control thread until after we know that
everything has been torn down first.  (And yes, we really need that
kthread_stop() because it's the only way to safely guarantee that a
module-referencing kernel thread has safely returned back out of the
module before we finally remove the module.)

Signed-off-by: Mike Isely <isely@pobox.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
---

diff --git a/drivers/media/video/pvrusb2/pvrusb2-context.c b/drivers/media/video/pvrusb2/pvrusb2-context.c
index e7a2ed58bde..a2ce022c515 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-context.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-context.c
@@ -35,6 +35,9 @@ static struct pvr2_context *pvr2_context_notify_first;
 static struct pvr2_context *pvr2_context_notify_last;
 static DEFINE_MUTEX(pvr2_context_mutex);
 static DECLARE_WAIT_QUEUE_HEAD(pvr2_context_sync_data);
+static DECLARE_WAIT_QUEUE_HEAD(pvr2_context_cleanup_data);
+static int pvr2_context_cleanup_flag;
+static int pvr2_context_cleaned_flag;
 static struct task_struct *pvr2_context_thread_ptr;
 
 
@@ -153,7 +156,7 @@ static void pvr2_context_check(struct pvr2_context *mp)
 
 static int pvr2_context_shutok(void)
 {
-	return kthread_should_stop() && (pvr2_context_exist_first == NULL);
+	return pvr2_context_cleanup_flag && (pvr2_context_exist_first == NULL);
 }
 
 
@@ -174,6 +177,15 @@ static int pvr2_context_thread_func(void *foo)
 			 pvr2_context_shutok()));
 	} while (!pvr2_context_shutok());
 
+	pvr2_context_cleaned_flag = !0;
+	wake_up(&pvr2_context_cleanup_data);
+
+	pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context thread cleaned up");
+
+	wait_event_interruptible(
+		pvr2_context_sync_data,
+		kthread_should_stop());
+
 	pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context thread end");
 
 	return 0;
@@ -191,6 +203,11 @@ int pvr2_context_global_init(void)
 
 void pvr2_context_global_done(void)
 {
+	pvr2_context_cleanup_flag = !0;
+	wake_up(&pvr2_context_sync_data);
+	wait_event_interruptible(
+		pvr2_context_cleanup_data,
+		pvr2_context_cleaned_flag);
 	kthread_stop(pvr2_context_thread_ptr);
 }