In 'mpi_sata_completion' the first call for 'spin_unlock_irqrestore()' is with flags=0, which is as good as 'spin_unlock_irq()' ( unconditional interrupt enabling). If intention of the developer is to enable the interrupt during execution of ' mpi_sata_completion' , then the code changes in the patch looks ok. If interrupt should not be enabled during execution of 'mpi_sata_completion' then we can use simple spin_lock and spin_unlock. regards santosh On Sun, Feb 26, 2012 at 8:37 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > On Sun, Feb 26, 2012 at 07:03:30PM +0530, santosh nayak wrote: >> From: Santosh Nayak <santoshprasadnayak@xxxxxxxxx> >> >> Static checker is giving following warning: >> " error: calling 'spin_unlock_irqrestore()' with bogus flags" >> >> The code flow is as shown below: >> process_oq() --> process_one_iomb --> mpi_sata_completion >> >> In 'mpi_sata_completion' >> the first call for 'spin_unlock_irqrestore()' is with flags=0, >> which is as good as 'spin_unlock_irq()' ( unconditional interrupt >> enabling). >> >> So for better performance 'spin_unlock_irqrestore()' can be replaced >> with 'spin_unlock_irq()' and 'spin_lock_irqsave()' can be replaced by >> 'spin_lock_irq()'. >> > > process_oq() is called from the interrupt handler pm8001_chip_isr() > with interrupts disabled. > > drivers/scsi/pm8001/pm8001_hwi.c > 4301 spin_lock_irqsave(&pm8001_ha->lock, flags); > 4302 pm8001_chip_interrupt_disable(pm8001_ha); > 4303 process_oq(pm8001_ha); > 4304 pm8001_chip_interrupt_enable(pm8001_ha); > 4305 spin_unlock_irqrestore(&pm8001_ha->lock, flags); > > Probably we should just be doing a spin_lock() and spin_unlock() > without re-enabling the IRQs. Should we even be doing that in the > irq handler anyway? > > regards, > dan carpenter > -- 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
