Re: [patch 1/1] uml:fix ubd deadlock on SMP

[Jens Axboe]

On Wed, Sep 08 2004, blaisorblade_spam@xxxxxxxx wrote:
> diff -puN arch/um/drivers/ubd_kern.c~uml-fix-ubd-deadlock arch/um/drivers/ubd_kern.c
> --- uml-linux-2.6.8.1/arch/um/drivers/ubd_kern.c~uml-fix-ubd-deadlock	2004-09-08 19:04:27.662926344 +0200
> +++ uml-linux-2.6.8.1-paolo/arch/um/drivers/ubd_kern.c	2004-09-08 19:05:36.700431048 +0200
> @@ -54,6 +54,7 @@
>  #include "mem.h"
>  #include "mem_kern.h"
>  
> +/*This is the queue lock. FIXME: make it per-UBD device.*/
>  static spinlock_t ubd_io_lock = SPIN_LOCK_UNLOCKED;
>  static spinlock_t ubd_lock = SPIN_LOCK_UNLOCKED;

probably not worth it to make it per-device. doing so should be a simple
search-replace job, though.

> @@ -396,14 +397,16 @@ int thread_fd = -1;
>   */
>  int intr_count = 0;
>  
> -static void ubd_finish(struct request *req, int error)
> +static inline void __ubd_finish(struct request *req, int error, int lock)
>  {
>  	int nsect;
>  
>  	if(error){
> - 		spin_lock(&ubd_io_lock);
> +		if (lock)
> +			spin_lock(&ubd_io_lock);
>  		end_request(req, 0);
> - 		spin_unlock(&ubd_io_lock);
> +		if (lock)
> +			spin_unlock(&ubd_io_lock);

In general, doing it this way is throwned upon. Either split the
function, or just make the callers acquire the lock if they have to.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/