Em 20-09-2011 07:32, Doron Cohen escreveu:
> Hi,
> This patch step is a  Bug fix - avoid (rare) dead locks causing the
> driver to hang when module removed.
> Thanks,
> Doron Cohen
> 
> -----------------------
> 
>>From ad75d9ce48d440c6db6c5147530f1e23de2fcb28 Mon Sep 17 00:00:00 2001
> From: Doron Cohen <dor...@siano-ms.com>
> Date: Tue, 20 Sep 2011 08:46:52 +0300
> Subject: [PATCH 19/21] Bug fix - waiting for free buffers might have
> caused dead locks. Mechanism changed so locks are released around each
> wait.
> 
> ---
>  drivers/media/dvb/siano/smscoreapi.c |   53
> +++++++++++++++++++++++++++------
>  1 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/dvb/siano/smscoreapi.c
> b/drivers/media/dvb/siano/smscoreapi.c
> index bb92351..0555a38 100644
> --- a/drivers/media/dvb/siano/smscoreapi.c
> +++ b/drivers/media/dvb/siano/smscoreapi.c
> @@ -1543,26 +1543,59 @@ EXPORT_SYMBOL_GPL(smscore_onresponse);
>   *
>   * @return pointer to descriptor on success, NULL on error.
>   */
> -
> -struct smscore_buffer_t *get_entry(struct smscore_device_t *coredev)
> +struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t
> *coredev)
>  {
>       struct smscore_buffer_t *cb = NULL;
>       unsigned long flags;
>  
> +     DEFINE_WAIT(wait);
> +
> +     spin_lock_irqsave(&coredev->bufferslock, flags);
> +
> +     /* set the current process state to interruptible sleep
> +      * in case schedule() will be called, this process will go to sleep 
> +      * and woken up only when a new buffer is available (see
> smscore_putbuffer)
> +      */
> +     prepare_to_wait(&coredev->buffer_mng_waitq, &wait,
> TASK_INTERRUPTIBLE);
> +
> +     if (list_empty(&coredev->buffers)) {
> +             sms_debug("no avaliable common buffer, need to schedule");
> +
> +             /* 
> +         * before going to sleep, release the lock 
> +         */
> +             spin_unlock_irqrestore(&coredev->bufferslock, flags);
> +
> +             schedule();
> +
> +             sms_debug("wake up after schedule()");
> +
> +             /* 
> +         * acquire the lock again 
> +         */
>       spin_lock_irqsave(&coredev->bufferslock, flags);
> -     if (!list_empty(&coredev->buffers)) {
> -             cb = (struct smscore_buffer_t *) coredev->buffers.next;
> -             list_del(&cb->entry);
>       }

The proper fix is not to call schedule(). It is to use a waitqueue. The
current driver has it already. I think you're likely reverting the fix here.
Please take a look at the git history to see how it was solved.

> +
> +     /* 
> +         * in case that schedule() was skipped, set the process state
> to running
> +      */
> +     finish_wait(&coredev->buffer_mng_waitq, &wait);
> +
> +     /* 
> +         * verify that the list is not empty, since it might have been 
> +      * emptied during the sleep
> +      * comment : this sitation can be avoided using
> spin_unlock_irqrestore_exclusive      
> +      */     
> +     if (list_empty(&coredev->buffers)) {
> +             sms_err("failed to allocate buffer, returning NULL");
>       spin_unlock_irqrestore(&coredev->bufferslock, flags);
> -     return cb;
> +             return NULL;
>  }
>  
> -struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t
> *coredev)
> -{
> -     struct smscore_buffer_t *cb = NULL;
> +     cb = (struct smscore_buffer_t *) coredev->buffers.next;
> +     list_del(&cb->entry);
>  
> -     wait_event(coredev->buffer_mng_waitq, (cb = get_entry(coredev)));
> +     spin_unlock_irqrestore(&coredev->bufferslock, flags);
>  
>       return cb;
>  }

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to