summaryrefslogtreecommitdiff
path: root/drivers/media
diff options
context:
space:
mode:
authorMike Isely <isely@pobox.com>2008-04-07 09:22:04 +0400
committerMauro Carvalho Chehab <mchehab@infradead.org>2008-04-24 21:09:48 +0400
commite5be15c63804e05b5a94197524023702a259e308 (patch)
tree4223cab8016088a06d0423e1e41eacb646867c46 /drivers/media
parentd913d6303072ca194919d851e6743ad8c3a7563d (diff)
downloadlinux-e5be15c63804e05b5a94197524023702a259e308.tar.xz
V4L/DVB (7711): pvrusb2: Fix race on module unload
The pvrusb2 driver - for basically forever - was not enforcing a proper module tear-down. Kernel threads are used inside the driver and all must be gone before the module can be safely removed. This changeset reimplements a chunk of pvrusb2-context.c to enforce this correctly. Unfortunately this is not a simple fix. The new implementation also cuts back on kernel thread usage; instead of there being 1 control thread per instance now it's just 1 control thread shared by all instances. (By dropping to a single thread then the module exit function can block on its shutdown and the thread itself can monitor and cleanly shut down all of the other instances first.) Signed-off-by: Mike Isely <isely@pobox.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
Diffstat (limited to 'drivers/media')
-rw-r--r--drivers/media/video/pvrusb2/pvrusb2-context.c204
-rw-r--r--drivers/media/video/pvrusb2/pvrusb2-context.h9
-rw-r--r--drivers/media/video/pvrusb2/pvrusb2-main.c12
3 files changed, 174 insertions, 51 deletions
diff --git a/drivers/media/video/pvrusb2/pvrusb2-context.c b/drivers/media/video/pvrusb2/pvrusb2-context.c
index 22bd8302c4aa..e7a2ed58bde2 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-context.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-context.c
@@ -23,100 +23,206 @@
#include "pvrusb2-ioread.h"
#include "pvrusb2-hdw.h"
#include "pvrusb2-debug.h"
+#include <linux/wait.h>
#include <linux/kthread.h>
#include <linux/errno.h>
#include <linux/string.h>
#include <linux/slab.h>
+static struct pvr2_context *pvr2_context_exist_first;
+static struct pvr2_context *pvr2_context_exist_last;
+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 struct task_struct *pvr2_context_thread_ptr;
+
+
+static void pvr2_context_set_notify(struct pvr2_context *mp, int fl)
+{
+ int signal_flag = 0;
+ mutex_lock(&pvr2_context_mutex);
+ if (fl) {
+ if (!mp->notify_flag) {
+ signal_flag = (pvr2_context_notify_first == NULL);
+ mp->notify_prev = pvr2_context_notify_last;
+ mp->notify_next = NULL;
+ pvr2_context_notify_last = mp;
+ if (mp->notify_prev) {
+ mp->notify_prev->notify_next = mp;
+ } else {
+ pvr2_context_notify_first = mp;
+ }
+ mp->notify_flag = !0;
+ }
+ } else {
+ if (mp->notify_flag) {
+ mp->notify_flag = 0;
+ if (mp->notify_next) {
+ mp->notify_next->notify_prev = mp->notify_prev;
+ } else {
+ pvr2_context_notify_last = mp->notify_prev;
+ }
+ if (mp->notify_prev) {
+ mp->notify_prev->notify_next = mp->notify_next;
+ } else {
+ pvr2_context_notify_first = mp->notify_next;
+ }
+ }
+ }
+ mutex_unlock(&pvr2_context_mutex);
+ if (signal_flag) wake_up(&pvr2_context_sync_data);
+}
+
static void pvr2_context_destroy(struct pvr2_context *mp)
{
pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (destroy)",mp);
if (mp->hdw) pvr2_hdw_destroy(mp->hdw);
+ pvr2_context_set_notify(mp, 0);
+ mutex_lock(&pvr2_context_mutex);
+ if (mp->exist_next) {
+ mp->exist_next->exist_prev = mp->exist_prev;
+ } else {
+ pvr2_context_exist_last = mp->exist_prev;
+ }
+ if (mp->exist_prev) {
+ mp->exist_prev->exist_next = mp->exist_next;
+ } else {
+ pvr2_context_exist_first = mp->exist_next;
+ }
+ if (!pvr2_context_exist_first) {
+ /* Trigger wakeup on control thread in case it is waiting
+ for an exit condition. */
+ wake_up(&pvr2_context_sync_data);
+ }
+ mutex_unlock(&pvr2_context_mutex);
kfree(mp);
}
static void pvr2_context_notify(struct pvr2_context *mp)
{
- mp->notify_flag = !0;
- wake_up(&mp->wait_data);
+ pvr2_context_set_notify(mp,!0);
}
-static int pvr2_context_thread(void *_mp)
+static void pvr2_context_check(struct pvr2_context *mp)
{
- struct pvr2_channel *ch1,*ch2;
- struct pvr2_context *mp = _mp;
- pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (thread start)",mp);
-
- /* Finish hardware initialization */
- if (pvr2_hdw_initialize(mp->hdw,
- (void (*)(void *))pvr2_context_notify,mp)) {
- mp->video_stream.stream =
- pvr2_hdw_get_video_stream(mp->hdw);
- /* Trigger interface initialization. By doing this here
- initialization runs in our own safe and cozy thread
- context. */
- if (mp->setup_func) mp->setup_func(mp);
- } else {
+ struct pvr2_channel *ch1, *ch2;
+ pvr2_trace(PVR2_TRACE_CTXT,
+ "pvr2_context %p (notify)", mp);
+ if (!mp->initialized_flag && !mp->disconnect_flag) {
+ mp->initialized_flag = !0;
pvr2_trace(PVR2_TRACE_CTXT,
- "pvr2_context %p (thread skipping setup)",mp);
- /* Even though initialization did not succeed, we're still
- going to enter the wait loop anyway. We need to do this
- in order to await the expected disconnect (which we will
- detect in the normal course of operation). */
- }
-
- /* Now just issue callbacks whenever hardware state changes or if
- there is a disconnect. If there is a disconnect and there are
- no channels left, then there's no reason to stick around anymore
- so we'll self-destruct - tearing down the rest of this driver
- instance along the way. */
- pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (thread enter loop)",mp);
- while (!mp->disconnect_flag || mp->mc_first) {
- if (mp->notify_flag) {
- mp->notify_flag = 0;
+ "pvr2_context %p (initialize)", mp);
+ /* Finish hardware initialization */
+ if (pvr2_hdw_initialize(mp->hdw,
+ (void (*)(void *))pvr2_context_notify,
+ mp)) {
+ mp->video_stream.stream =
+ pvr2_hdw_get_video_stream(mp->hdw);
+ /* Trigger interface initialization. By doing this
+ here initialization runs in our own safe and
+ cozy thread context. */
+ if (mp->setup_func) mp->setup_func(mp);
+ } else {
pvr2_trace(PVR2_TRACE_CTXT,
- "pvr2_context %p (thread notify)",mp);
- for (ch1 = mp->mc_first; ch1; ch1 = ch2) {
- ch2 = ch1->mc_next;
- if (ch1->check_func) ch1->check_func(ch1);
- }
+ "pvr2_context %p (thread skipping setup)",
+ mp);
+ /* Even though initialization did not succeed,
+ we're still going to continue anyway. We need
+ to do this in order to await the expected
+ disconnect (which we will detect in the normal
+ course of operation). */
}
- wait_event_interruptible(mp->wait_data, mp->notify_flag);
}
- pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (thread end)",mp);
- pvr2_context_destroy(mp);
+
+ for (ch1 = mp->mc_first; ch1; ch1 = ch2) {
+ ch2 = ch1->mc_next;
+ if (ch1->check_func) ch1->check_func(ch1);
+ }
+
+ if (mp->disconnect_flag && !mp->mc_first) {
+ /* Go away... */
+ pvr2_context_destroy(mp);
+ return;
+ }
+}
+
+
+static int pvr2_context_shutok(void)
+{
+ return kthread_should_stop() && (pvr2_context_exist_first == NULL);
+}
+
+
+static int pvr2_context_thread_func(void *foo)
+{
+ struct pvr2_context *mp;
+
+ pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context thread start");
+
+ do {
+ while ((mp = pvr2_context_notify_first) != NULL) {
+ pvr2_context_set_notify(mp, 0);
+ pvr2_context_check(mp);
+ }
+ wait_event_interruptible(
+ pvr2_context_sync_data,
+ ((pvr2_context_notify_first != NULL) ||
+ pvr2_context_shutok()));
+ } while (!pvr2_context_shutok());
+
+ pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context thread end");
+
return 0;
}
+int pvr2_context_global_init(void)
+{
+ pvr2_context_thread_ptr = kthread_run(pvr2_context_thread_func,
+ 0,
+ "pvrusb2-context");
+ return (pvr2_context_thread_ptr ? 0 : -ENOMEM);
+}
+
+
+void pvr2_context_global_done(void)
+{
+ kthread_stop(pvr2_context_thread_ptr);
+}
+
+
struct pvr2_context *pvr2_context_create(
struct usb_interface *intf,
const struct usb_device_id *devid,
void (*setup_func)(struct pvr2_context *))
{
- struct task_struct *thread;
struct pvr2_context *mp = NULL;
mp = kzalloc(sizeof(*mp),GFP_KERNEL);
if (!mp) goto done;
pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (create)",mp);
- init_waitqueue_head(&mp->wait_data);
mp->setup_func = setup_func;
mutex_init(&mp->mutex);
+ mutex_lock(&pvr2_context_mutex);
+ mp->exist_prev = pvr2_context_exist_last;
+ mp->exist_next = NULL;
+ pvr2_context_exist_last = mp;
+ if (mp->exist_prev) {
+ mp->exist_prev->exist_next = mp;
+ } else {
+ pvr2_context_exist_first = mp;
+ }
+ mutex_unlock(&pvr2_context_mutex);
mp->hdw = pvr2_hdw_create(intf,devid);
if (!mp->hdw) {
pvr2_context_destroy(mp);
mp = NULL;
goto done;
}
- thread = kthread_run(pvr2_context_thread, mp, "pvrusb2-context");
- if (!thread) {
- pvr2_context_destroy(mp);
- mp = NULL;
- goto done;
- }
+ pvr2_context_set_notify(mp, !0);
done:
return mp;
}
diff --git a/drivers/media/video/pvrusb2/pvrusb2-context.h b/drivers/media/video/pvrusb2/pvrusb2-context.h
index 127ec53e0913..6fb6ab022851 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-context.h
+++ b/drivers/media/video/pvrusb2/pvrusb2-context.h
@@ -40,14 +40,17 @@ struct pvr2_context_stream {
struct pvr2_context {
struct pvr2_channel *mc_first;
struct pvr2_channel *mc_last;
+ struct pvr2_context *exist_next;
+ struct pvr2_context *exist_prev;
+ struct pvr2_context *notify_next;
+ struct pvr2_context *notify_prev;
struct pvr2_hdw *hdw;
struct pvr2_context_stream video_stream;
struct mutex mutex;
int notify_flag;
+ int initialized_flag;
int disconnect_flag;
- wait_queue_head_t wait_data;
-
/* Called after pvr2_context initialization is complete */
void (*setup_func)(struct pvr2_context *);
@@ -74,6 +77,8 @@ int pvr2_channel_claim_stream(struct pvr2_channel *,
struct pvr2_ioread *pvr2_channel_create_mpeg_stream(
struct pvr2_context_stream *);
+int pvr2_context_global_init(void);
+void pvr2_context_global_done(void);
#endif /* __PVRUSB2_CONTEXT_H */
/*
diff --git a/drivers/media/video/pvrusb2/pvrusb2-main.c b/drivers/media/video/pvrusb2/pvrusb2-main.c
index 54d9f168d7ad..332aced8a5a1 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-main.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-main.c
@@ -125,6 +125,12 @@ static int __init pvr_init(void)
pvr2_trace(PVR2_TRACE_INIT,"pvr_init");
+ ret = pvr2_context_global_init();
+ if (ret != 0) {
+ pvr2_trace(PVR2_TRACE_INIT,"pvr_init failure code=%d",ret);
+ return ret;
+ }
+
#ifdef CONFIG_VIDEO_PVRUSB2_SYSFS
class_ptr = pvr2_sysfs_class_create();
#endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
@@ -136,6 +142,8 @@ static int __init pvr_init(void)
if (pvrusb2_debug) info("Debug mask is %d (0x%x)",
pvrusb2_debug,pvrusb2_debug);
+ pvr2_trace(PVR2_TRACE_INIT,"pvr_init complete");
+
return ret;
}
@@ -148,6 +156,10 @@ static void __exit pvr_exit(void)
#ifdef CONFIG_VIDEO_PVRUSB2_SYSFS
pvr2_sysfs_class_destroy(class_ptr);
#endif /* CONFIG_VIDEO_PVRUSB2_SYSFS */
+
+ pvr2_context_global_done();
+
+ pvr2_trace(PVR2_TRACE_INIT,"pvr_exit complete");
}
module_init(pvr_init);