Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e5be15c6 authored by Mike Isely's avatar Mike Isely Committed by Mauro Carvalho Chehab
Browse files

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: default avatarMike Isely <isely@pobox.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@infradead.org>
parent d913d630

drivers/media/video/pvrusb2/pvrusb2-context.c

+155 −49
Original line number Diff line number Diff line
@@ -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);



	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 (initialize)", mp);

		/* Finish hardware initialization */

		if (pvr2_hdw_initialize(mp->hdw,

				(void (*)(void *))pvr2_context_notify,mp)) {

					(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. */

			/* 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 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_trace(PVR2_TRACE_CTXT,

				   "pvr2_context %p (thread notify)",mp);

				   "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). */

		}

	}



	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;

	}

		wait_event_interruptible(mp->wait_data, mp->notify_flag);

}

	pvr2_trace(PVR2_TRACE_CTXT,"pvr2_context %p (thread end)",mp);

	pvr2_context_destroy(mp);





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;

}

drivers/media/video/pvrusb2/pvrusb2-context.h

+7 −2
Original line number Diff line number Diff line
@@ -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 */

/*

drivers/media/video/pvrusb2/pvrusb2-main.c

+12 −0
Original line number Diff line number Diff line
@@ -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);