lkml.org 
[lkml]   [2007]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: 2.6.22-rc6 spurious hangs
    On 06/29, Markus Rechberger wrote:
    >
    > On 6/29/07, Mauro Carvalho Chehab <mchehab@infradead.org> wrote:
    > >> Still we can't do this under cinergyt2->sem, because cinergyt2_query()
    > >> takes it too. This all looks very wrong to me, I hope maintaners can
    > >> explain.
    > >
    > >AFAIK, the driver authors are not working anymore with CinergyT2. The
    > >last patch we have on development tree from Holger is dated as Dec, 3
    > >2004. Since them, only internal kernel API changes and a few sparse
    > >fixes from other contributors were applied.
    > >
    > >Also, none of the current DVB maintainers seem to have any hardware for
    > >testing.
    > >
    >
    > I received a Mail a while ago that this driver is open to the
    > community, it duplicates some code because the developers wanted to
    > use this driver for testing another DVB API which never took off.
    > Best would be to remove the duplicated code in that driver and make it
    > look like all other DVB drivers.

    OK.

    Thomas, any chance you could try the patch below? It is very, very stupid,
    it was done without any understanding of this code, and of course it is
    completely untested. I doubt very much it is correct, and even if it is
    correct it is definitely not good. It would be great if Dmitry can take a look.

    This patch shifts cancel_rearming_delayed_work() out of ->sem. Another mutex,
    ->wq_sem, was added to protect against the concurrent open/resume.

    Oleg.

    --- t/drivers/media/dvb/cinergyT2/cinergyT2.c~cinergy 2007-06-19 17:09:15.000000000 +0400
    +++ t/drivers/media/dvb/cinergyT2/cinergyT2.c 2007-06-30 18:01:43.000000000 +0400
    @@ -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
    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

    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 ino
    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

    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_i
    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
    {
    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,22 @@ static int cinergyt2_suspend (struct usb
    {
    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 +1020,15 @@ static int cinergyt2_resume (struct usb_
    {
    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 +1041,10 @@ static int cinergyt2_resume (struct usb_
    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 linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-06-30 16:21    [W:0.031 / U:59.452 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site