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 <h...@suse.de>
> Cc: James Bottomley <jbottom...@parallels.com>
> Cc: Kai Mäkisara <kai.makis...@kolumbus.fi>
> Cc: Lee Duncan <ldun...@suse.com>
> ---
>  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.makis...@kolumbus.fi>

Thanks,
Kai

Reply via email to