Gitweb:     
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=1e4597e8f0049dccedb0e011934007309fa2aeab
Commit:     1e4597e8f0049dccedb0e011934007309fa2aeab
Parent:     f057131fb6eb2c45f6023e3da41ccd6e4e71aee9
Author:     Oleg Nesterov <[EMAIL PROTECTED]>
AuthorDate: Mon Jul 2 12:26:20 2007 -0300
Committer:  Mauro Carvalho Chehab <[EMAIL PROTECTED]>
CommitDate: Tue Jul 3 15:11:19 2007 -0300

    V4L/DVB (5818): CinergyT2: fix flush_workqueue() vs work->func() deadlock
    
    Spotted and tested by Thomas Sattler <[EMAIL PROTECTED]>.
    
    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 <[EMAIL PROTECTED]>
    Signed-off-by: Mauro Carvalho Chehab <[EMAIL PROTECTED]>
---
 drivers/media/dvb/cinergyT2/cinergyT2.c |   66 +++++++++++++++++++------------
 1 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/media/dvb/cinergyT2/cinergyT2.c 
b/drivers/media/dvb/cinergyT2/cinergyT2.c
index 6aba5b3..b40af48 100644
--- a/drivers/media/dvb/cinergyT2/cinergyT2.c
+++ b/drivers/media/dvb/cinergyT2/cinergyT2.c
@@ -118,6 +118,7 @@ struct cinergyt2 {
        struct dvb_demux demux;
        struct usb_device *udev;
        struct mutex sem;
+       struct mutex wq_sem;
        struct dvb_adapter adapter;
        struct dvb_device *fedev;
        struct dmxdev dmxdev;
@@ -482,14 +483,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);
@@ -498,8 +499,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)
@@ -519,15 +524,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");
@@ -838,13 +845,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)
@@ -907,6 +914,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);
        INIT_DELAYED_WORK(&cinergyt2->query_work, cinergyt2_query);
 
@@ -974,11 +982,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);
@@ -992,21 +997,21 @@ 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 (1) {
-               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;
 }
 
@@ -1014,9 +1019,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);
@@ -1029,7 +1040,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 = {
-
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to