Re: [PATCH] st: unblock concurrent access in st_open()

[Kai Makisara <Kai.Makisara@xxxxxxxxxxx>]

On Wed, 15 Aug 2012, Hannes Reinecke wrote:

> st_open() is taking a global mutex 'st_mutex' to protect
> against concurrent calls to st_open(). However, as
> st_open() might take quite some time before returning,
> _any_ open to _any_ st device will be blocked during
> that time.
> 
> Seeing that all critical sections are already protected
> by individual spinlocks 'st_mutex' doesn't serve any
> purpose and can be removed, unblocking concurrent
> accesses.
> 
> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> Cc: James Bottomley <jbottomley@xxxxxxxxxxxxx>
> Cc: Kai Mäkisara <Kai.Makisara@xxxxxxxxxxx>
> Cc: Lee Duncan <lduncan@xxxxxxxx>
> ---
>  drivers/scsi/st.c |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index e41998c..c6c172e 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
...
> @@ -1262,16 +1258,16 @@ static int st_open(struct inode *inode, struct file *filp)
>  			retval = (-EIO);
>  		goto err_out;
>  	}
> -	mutex_unlock(&st_mutex);
>  	return 0;
>  
>   err_out:
>  	normalize_buffer(STp->buffer);
> +	write_lock(&st_dev_arr_lock);
>  	STp->in_use = 0;
> +	write_unlock(&st_dev_arr_lock);

I am not sure locking here is necessary. No-one should be able to remove 
the tape struct when in_use is non-zero. But I don't object to this extra 
protection.

Acked-by: Kai Mäkisara <Kai.Makisara@xxxxxxxxxxx>

Thanks,
Kai