From fc035425e7b3d9c08a27b6860bf1b28abb1454cf Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Mon, 2 Jul 2007 12:26:20 -0300 Subject: cinergyT2: fix flush_workqueue() vs work->func() deadlock From: Oleg Nesterov Spotted and tested by Thomas Sattler . cinergyT2.c does cancel_delayed_work() + flush_scheduled_work() while holding cinergyt2->sem. This leads to deadlock because work->func() needs the same mutex to complete. Another bug is that this code in fact can't reliably stop the re-arming delayed_work. Convert this code to use cancel_rearming_delayed_work() and move it out of ->sem. Another mutex, ->wq_sem, was added to protect against the concurrent open/resume. This patch is a horrible hack to fix the lockup which happens in practice. As Dmitry Torokhov pointed out this driver has other problems and needs further changes. Signed-off-by: Oleg Nesterov Signed-off-by: Mauro Carvalho Chehab --- linux/drivers/media/dvb/cinergyT2/cinergyT2.c | 67 ++++++++++++++++----------- 1 file changed, 41 insertions(+), 26 deletions(-) (limited to 'linux/drivers/media/dvb') diff --git a/linux/drivers/media/dvb/cinergyT2/cinergyT2.c b/linux/drivers/media/dvb/cinergyT2/cinergyT2.c index f495251d6..7d4548069 100644 --- a/linux/drivers/media/dvb/cinergyT2/cinergyT2.c +++ b/linux/drivers/media/dvb/cinergyT2/cinergyT2.c @@ -132,8 +132,10 @@ struct cinergyt2 { struct usb_device *udev; #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,15) struct mutex sem; + struct mutex wq_sem; #else struct semaphore sem; + struct semaphore wq_sem; #endif struct dvb_adapter adapter; struct dvb_device *fedev; @@ -515,14 +517,14 @@ static int cinergyt2_open (struct inode *inode, struct file *file) struct cinergyt2 *cinergyt2 = dvbdev->priv; int err = -ERESTARTSYS; - if (cinergyt2->disconnect_pending || mutex_lock_interruptible(&cinergyt2->sem)) - return -ERESTARTSYS; + if (cinergyt2->disconnect_pending || mutex_lock_interruptible(&cinergyt2->wq_sem)) + goto out; - if ((err = dvb_generic_open(inode, file))) { - mutex_unlock(&cinergyt2->sem); - return err; - } + if (mutex_lock_interruptible(&cinergyt2->sem)) + goto out_unlock1; + if ((err = dvb_generic_open(inode, file))) + goto out_unlock2; if ((file->f_flags & O_ACCMODE) != O_RDONLY) { cinergyt2_sleep(cinergyt2, 0); @@ -531,8 +533,12 @@ static int cinergyt2_open (struct inode *inode, struct file *file) atomic_inc(&cinergyt2->inuse); +out_unlock2: mutex_unlock(&cinergyt2->sem); - return 0; +out_unlock1: + mutex_unlock(&cinergyt2->wq_sem); +out: + return err; } static void cinergyt2_unregister(struct cinergyt2 *cinergyt2) @@ -552,15 +558,17 @@ static int cinergyt2_release (struct inode *inode, struct file *file) struct dvb_device *dvbdev = file->private_data; struct cinergyt2 *cinergyt2 = dvbdev->priv; - mutex_lock(&cinergyt2->sem); + mutex_lock(&cinergyt2->wq_sem); if (!cinergyt2->disconnect_pending && (file->f_flags & O_ACCMODE) != O_RDONLY) { - cancel_delayed_work(&cinergyt2->query_work); - flush_scheduled_work(); + cancel_rearming_delayed_work(&cinergyt2->query_work); + + mutex_lock(&cinergyt2->sem); cinergyt2_sleep(cinergyt2, 1); + mutex_unlock(&cinergyt2->sem); } - mutex_unlock(&cinergyt2->sem); + mutex_unlock(&cinergyt2->wq_sem); if (atomic_dec_and_test(&cinergyt2->inuse) && cinergyt2->disconnect_pending) { warn("delayed unregister in release"); @@ -891,13 +899,13 @@ static int cinergyt2_register_rc(struct cinergyt2 *cinergyt2) static void cinergyt2_unregister_rc(struct cinergyt2 *cinergyt2) { - cancel_delayed_work(&cinergyt2->rc_query_work); + cancel_rearming_delayed_work(&cinergyt2->rc_query_work); input_unregister_device(cinergyt2->rc_input_dev); } static inline void cinergyt2_suspend_rc(struct cinergyt2 *cinergyt2) { - cancel_delayed_work(&cinergyt2->rc_query_work); + cancel_rearming_delayed_work(&cinergyt2->rc_query_work); } static inline void cinergyt2_resume_rc(struct cinergyt2 *cinergyt2) @@ -968,6 +976,7 @@ static int cinergyt2_probe (struct usb_interface *intf, usb_set_intfdata (intf, (void *) cinergyt2); mutex_init(&cinergyt2->sem); + mutex_init(&cinergyt2->wq_sem); init_waitqueue_head (&cinergyt2->poll_wq); #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) INIT_WORK(&cinergyt2->query_work, cinergyt2_query, cinergyt2); @@ -1039,11 +1048,8 @@ static void cinergyt2_disconnect (struct usb_interface *intf) { struct cinergyt2 *cinergyt2 = usb_get_intfdata (intf); - flush_scheduled_work(); - cinergyt2_unregister_rc(cinergyt2); - - cancel_delayed_work(&cinergyt2->query_work); + cancel_rearming_delayed_work(&cinergyt2->query_work); wake_up_interruptible(&cinergyt2->poll_wq); cinergyt2->demux.dmx.close(&cinergyt2->demux.dmx); @@ -1057,7 +1063,7 @@ static int cinergyt2_suspend (struct usb_interface *intf, pm_message_t state) { struct cinergyt2 *cinergyt2 = usb_get_intfdata (intf); - if (cinergyt2->disconnect_pending || mutex_lock_interruptible(&cinergyt2->sem)) + if (cinergyt2->disconnect_pending || mutex_lock_interruptible(&cinergyt2->wq_sem)) return -ERESTARTSYS; #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,14) @@ -1065,17 +1071,17 @@ static int cinergyt2_suspend (struct usb_interface *intf, pm_message_t state) #else if (1) { #endif - struct cinergyt2 *cinergyt2 = usb_get_intfdata (intf); - cinergyt2_suspend_rc(cinergyt2); - cancel_delayed_work(&cinergyt2->query_work); + cancel_rearming_delayed_work(&cinergyt2->query_work); + + mutex_lock(&cinergyt2->sem); if (cinergyt2->streaming) cinergyt2_stop_stream_xfer(cinergyt2); - flush_scheduled_work(); cinergyt2_sleep(cinergyt2, 1); + mutex_unlock(&cinergyt2->sem); } - mutex_unlock(&cinergyt2->sem); + mutex_unlock(&cinergyt2->wq_sem); return 0; } @@ -1083,9 +1089,15 @@ static int cinergyt2_resume (struct usb_interface *intf) { struct cinergyt2 *cinergyt2 = usb_get_intfdata (intf); struct dvbt_set_parameters_msg *param = &cinergyt2->param; + int err = -ERESTARTSYS; - if (cinergyt2->disconnect_pending || mutex_lock_interruptible(&cinergyt2->sem)) - return -ERESTARTSYS; + if (cinergyt2->disconnect_pending || mutex_lock_interruptible(&cinergyt2->wq_sem)) + goto out; + + if (mutex_lock_interruptible(&cinergyt2->sem)) + goto out_unlock1; + + err = 0; if (!cinergyt2->sleeping) { cinergyt2_sleep(cinergyt2, 0); @@ -1098,7 +1110,10 @@ static int cinergyt2_resume (struct usb_interface *intf) cinergyt2_resume_rc(cinergyt2); mutex_unlock(&cinergyt2->sem); - return 0; +out_unlock1: + mutex_unlock(&cinergyt2->wq_sem); +out: + return err; } static const struct usb_device_id cinergyt2_table [] __devinitdata = { -- cgit v1.2.3 From 56dee54783bcafee32bb681a221d4c9a2b517ca3 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Mon, 2 Jul 2007 15:39:57 -0300 Subject: Fix v4l-dvb backward compatibility From: Mauro Carvalho Chehab Due to several internal API changes on kernel, kernel backward compatibility were lost. Basically, compat.h should be the last include for it to work properly. This patch basically reorders kernel headers to allow backward compat to work fine. Also: Some includes were added after some non-include macros, on old drivers. Better to keep all includes at the beginning of the files. Signed-off-by: Mauro Carvalho Chehab --- linux/drivers/media/dvb/dvb-core/dvb_frontend.c | 2 +- linux/drivers/media/dvb/dvb-core/dvbdev.h | 1 - linux/drivers/media/dvb/frontends/tda10086.c | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) (limited to 'linux/drivers/media/dvb') diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c index 2cad44fb2..90093c965 100644 --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c +++ b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c @@ -34,7 +34,6 @@ #include #include #include -#include "compat.h" #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) #include #else @@ -46,6 +45,7 @@ #include "dvb_frontend.h" #include "dvbdev.h" +#include "compat.h" static int dvb_frontend_debug; static int dvb_shutdown_timeout = 5; diff --git a/linux/drivers/media/dvb/dvb-core/dvbdev.h b/linux/drivers/media/dvb/dvb-core/dvbdev.h index cb76869bd..6dff10ebf 100644 --- a/linux/drivers/media/dvb/dvb-core/dvbdev.h +++ b/linux/drivers/media/dvb/dvb-core/dvbdev.h @@ -28,7 +28,6 @@ #include #include #include -#include "compat.h" #define DVB_MAJOR 212 diff --git a/linux/drivers/media/dvb/frontends/tda10086.c b/linux/drivers/media/dvb/frontends/tda10086.c index 0f2d4b415..d5838df74 100644 --- a/linux/drivers/media/dvb/frontends/tda10086.c +++ b/linux/drivers/media/dvb/frontends/tda10086.c @@ -30,6 +30,7 @@ #include "dvb_frontend.h" #include "tda10086.h" +#include "compat.h" #define SACLK 96000000 -- cgit v1.2.3 From ff3f4061e1e6854e0d2c8c52c2363f3d67302651 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Mon, 2 Jul 2007 15:48:40 -0300 Subject: Cleanup on cinergyT2: Remove unneeded if(1) From: Mauro Carvalho Chehab Before kernel 2.6.14, the driver checked for status before stopping the thread. So, a compatibility test did exist. After 2.6.14, the if (state) were replaced by: if (1) However, it makes no sense to keep the if(1). Signed-off-by: Mauro Carvalho Chehab --- linux/drivers/media/dvb/cinergyT2/cinergyT2.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) (limited to 'linux/drivers/media/dvb') diff --git a/linux/drivers/media/dvb/cinergyT2/cinergyT2.c b/linux/drivers/media/dvb/cinergyT2/cinergyT2.c index 7d4548069..cca6b7985 100644 --- a/linux/drivers/media/dvb/cinergyT2/cinergyT2.c +++ b/linux/drivers/media/dvb/cinergyT2/cinergyT2.c @@ -1067,21 +1067,20 @@ static int cinergyt2_suspend (struct usb_interface *intf, pm_message_t state) return -ERESTARTSYS; #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,14) - if (state > 0) { -#else - if (1) { + if (state <= 0) { + mutex_unlock(&cinergyt2->wq_sem); + return 0; + } #endif - cinergyt2_suspend_rc(cinergyt2); - cancel_rearming_delayed_work(&cinergyt2->query_work); + cinergyt2_suspend_rc(cinergyt2); + cancel_rearming_delayed_work(&cinergyt2->query_work); - mutex_lock(&cinergyt2->sem); - if (cinergyt2->streaming) - cinergyt2_stop_stream_xfer(cinergyt2); - cinergyt2_sleep(cinergyt2, 1); - mutex_unlock(&cinergyt2->sem); - } + mutex_lock(&cinergyt2->sem); + if (cinergyt2->streaming) + cinergyt2_stop_stream_xfer(cinergyt2); + cinergyt2_sleep(cinergyt2, 1); + mutex_unlock(&cinergyt2->sem); - mutex_unlock(&cinergyt2->wq_sem); return 0; } -- cgit v1.2.3