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
