From 18ecbb4771eb0ecf297e996966b3c42f69cd6c02 Mon Sep 17 00:00:00 2001 From: Mike Isely Date: Wed, 9 Apr 2008 05:14:11 -0300 Subject: [PATCH] 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 Signed-off-by: Mauro Carvalho Chehab --- drivers/media/video/pvrusb2/pvrusb2-context.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/media/video/pvrusb2/pvrusb2-context.c b/drivers/media/video/pvrusb2/pvrusb2-context.c index e7a2ed5..a2ce022 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); } -- 2.7.4