All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: santosh nayak <santoshprasadnayak@gmail.com>
Cc: jack_wang@usish.com, lindar_liu@usish.com,
	JBottomley@parallels.com, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 1/2] [SCSI] pm8001: Fix bogus interrupt state flag issue.
Date: Sun, 26 Feb 2012 15:07:20 +0000	[thread overview]
Message-ID: <20120226150720.GB4763@mwanda> (raw)
In-Reply-To: <1330263210-25134-1-git-send-email-santoshprasadnayak@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

On Sun, Feb 26, 2012 at 07:03:30PM +0530, santosh nayak wrote:
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
> 
> 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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: santosh nayak <santoshprasadnayak@gmail.com>
Cc: jack_wang@usish.com, lindar_liu@usish.com,
	JBottomley@parallels.com, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 1/2] [SCSI] pm8001: Fix bogus interrupt state flag issue.
Date: Sun, 26 Feb 2012 18:07:20 +0300	[thread overview]
Message-ID: <20120226150720.GB4763@mwanda> (raw)
In-Reply-To: <1330263210-25134-1-git-send-email-santoshprasadnayak@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

On Sun, Feb 26, 2012 at 07:03:30PM +0530, santosh nayak wrote:
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
> 
> 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


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2012-02-26 15:07 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-26 13:33 [PATCH 1/2] [SCSI] pm8001: Fix bogus interrupt state flag issue santosh nayak
2012-02-26 13:45 ` santosh nayak
2012-02-26 15:07 ` Dan Carpenter [this message]
2012-02-26 15:07   ` Dan Carpenter
2012-02-27  4:57   ` santosh prasad nayak
2012-02-27  4:58     ` santosh prasad nayak
2012-02-27  4:57     ` santosh prasad nayak
2012-02-27  6:55     ` Dan Carpenter
2012-02-27  6:55       ` Dan Carpenter
2012-02-27  8:29     ` Jack Wang
2012-02-27  8:29       ` Jack Wang
2012-02-27  8:29       ` Jack Wang
2012-03-08 13:32 ` Mark Salyzyn
2012-03-08 13:32   ` Mark Salyzyn
2012-03-08 14:15   ` jack_wang
2012-03-08 14:15     ` jack_wang
2012-03-08 14:15     ` jack_wang
2012-03-08 16:50   ` santosh prasad nayak
2012-03-08 16:51     ` santosh prasad nayak
2012-03-08 16:50     ` santosh prasad nayak
2012-03-08 17:11     ` Mark Salyzyn
2012-03-08 17:11       ` Mark Salyzyn
2012-03-08 20:48       ` santosh prasad nayak
2012-03-08 20:49         ` santosh prasad nayak
2012-03-08 21:16         ` Mark Salyzyn
2012-03-08 21:16           ` Mark Salyzyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120226150720.GB4763@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=JBottomley@parallels.com \
    --cc=jack_wang@usish.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=lindar_liu@usish.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=santoshprasadnayak@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.