Re: [PATCH] csiostor: Fix backwards locking in the function __csio_unreg_rnode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2016-04-06 at 09:21 -0400, Bastien Philbert wrote:
> 
> On 2016-04-06 03:48 AM, Julian Calaby wrote:
> > Hi Bastien,
> > 
> > On Wed, Apr 6, 2016 at 7:19 AM, Bastien Philbert
> > <bastienphilbert@xxxxxxxxx> wrote:
> > > This fixes backwards locking in the function __csio_unreg_rnode
> > > to
> > > properly lock before the call to the function csio_unreg_rnode
> > > and
> > > not unlock with spin_unlock_irq as this would not allow the
> > > proper
> > > protection for concurrent access on the shared csio_hw structure
> > > pointer hw. In addition switch the locking after the critical
> > > region
> > > function call to properly unlock instead with spin_unlock_irq on
> > > 
> > > Signed-off-by: Bastien Philbert <bastienphilbert@xxxxxxxxx>
> > > ---
> > >  drivers/scsi/csiostor/csio_rnode.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/scsi/csiostor/csio_rnode.c
> > > b/drivers/scsi/csiostor/csio_rnode.c
> > > index e9c3b04..029a09e 100644
> > > --- a/drivers/scsi/csiostor/csio_rnode.c
> > > +++ b/drivers/scsi/csiostor/csio_rnode.c
> > > @@ -580,9 +580,9 @@ __csio_unreg_rnode(struct csio_rnode *rn)
> > >                 ln->last_scan_ntgts--;
> > >         }
> > > 
> > > -       spin_unlock_irq(&hw->lock);
> > > -       csio_unreg_rnode(rn);
> > >         spin_lock_irq(&hw->lock);
> > > +       csio_unreg_rnode(rn);
> > > +       spin_unlock_irq(&hw->lock);
> > 
> > Are you _certain_ this is correct? This construct usually appears
> > when
> > a function has a particular lock held, then needs to unlock it to
> > call
> > some other function. Are you _certain_ that this isn't the case?
> > 
> > Thanks,
> > 
> Yes I am pretty certain this is correct. I checked the paths that
> called this function
> and it was weired that none of them gradded the spinlock before hand.

That's not good enough.  If your theory is correct, lockdep should be
dropping an already unlocked assertion in this codepath ... do you see
this?

James




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



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux