mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: santosh nayak <santoshprasadnayak@gmail.com>
To: jack_wang@usish.com
Cc: lindar_liu@usish.com, JBottomley@parallels.com,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org,
	Santosh Nayak <santoshprasadnayak@gmail.com>
Subject: [PATCH 1/2] [SCSI] pm8001: Fix bogus interrupt state flag issue.
Date: Sun, 26 Feb 2012 19:03:30 +0530	[thread overview]
Message-ID: <1330263210-25134-1-git-send-email-santoshprasadnayak@gmail.com> (raw)

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()'.

Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
---
 drivers/scsi/pm8001/pm8001_hwi.c |   58 ++++++++++++++++++-------------------
 1 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 833e720..838e3e2 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1877,7 +1877,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 {
 	struct sas_task *t;
 	struct pm8001_ccb_info *ccb;
-	unsigned long flags = 0;
 	u32 param;
 	u32 status;
 	u32 tag;
@@ -2016,9 +2015,9 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 			ts->stat = SAS_QUEUE_FULL;
 			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 			mb();/*in order to force CPU ordering*/
-			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			spin_unlock_irq(&pm8001_ha->lock);
 			t->task_done(t);
-			spin_lock_irqsave(&pm8001_ha->lock, flags);
+			spin_lock_irq(&pm8001_ha->lock);
 			return;
 		}
 		break;
@@ -2036,9 +2035,9 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 			ts->stat = SAS_QUEUE_FULL;
 			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 			mb();/*ditto*/
-			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			spin_unlock_irq(&pm8001_ha->lock);
 			t->task_done(t);
-			spin_lock_irqsave(&pm8001_ha->lock, flags);
+			spin_lock_irq(&pm8001_ha->lock);
 			return;
 		}
 		break;
@@ -2064,9 +2063,9 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 			ts->stat = SAS_QUEUE_FULL;
 			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 			mb();/* ditto*/
-			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			spin_unlock_irq(&pm8001_ha->lock);
 			t->task_done(t);
-			spin_lock_irqsave(&pm8001_ha->lock, flags);
+			spin_lock_irq(&pm8001_ha->lock);
 			return;
 		}
 		break;
@@ -2131,9 +2130,9 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 			ts->stat = SAS_QUEUE_FULL;
 			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 			mb();/*ditto*/
-			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			spin_unlock_irq(&pm8001_ha->lock);
 			t->task_done(t);
-			spin_lock_irqsave(&pm8001_ha->lock, flags);
+			spin_lock_irq(&pm8001_ha->lock);
 			return;
 		}
 		break;
@@ -2155,9 +2154,9 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 			ts->stat = SAS_QUEUE_FULL;
 			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 			mb();/*ditto*/
-			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			spin_unlock_irq(&pm8001_ha->lock);
 			t->task_done(t);
-			spin_lock_irqsave(&pm8001_ha->lock, flags);
+			spin_lock_irq(&pm8001_ha->lock);
 			return;
 		}
 		break;
@@ -2175,31 +2174,31 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		ts->stat = SAS_DEV_NO_RESPONSE;
 		break;
 	}
-	spin_lock_irqsave(&t->task_state_lock, flags);
+	spin_lock_irq(&t->task_state_lock);
 	t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
 	t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
 	t->task_state_flags |= SAS_TASK_STATE_DONE;
 	if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
-		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irq(&t->task_state_lock);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("task 0x%p done with io_status 0x%x"
 			" resp 0x%x stat 0x%x but aborted by upper layer!\n",
 			t, status, ts->resp, ts->stat));
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 	} else if (t->uldd_task) {
-		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irq(&t->task_state_lock);
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 		mb();/* ditto */
-		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+		spin_unlock_irq(&pm8001_ha->lock);
 		t->task_done(t);
-		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		spin_lock_irq(&pm8001_ha->lock);
 	} else if (!t->uldd_task) {
-		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irq(&t->task_state_lock);
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 		mb();/*ditto*/
-		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+		spin_unlock_irq(&pm8001_ha->lock);
 		t->task_done(t);
-		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		spin_lock_irq(&pm8001_ha->lock);
 	}
 }
 
@@ -2207,7 +2206,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
 static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha , void *piomb)
 {
 	struct sas_task *t;
-	unsigned long flags = 0;
 	struct task_status_struct *ts;
 	struct pm8001_ccb_info *ccb;
 	struct pm8001_device *pm8001_dev;
@@ -2287,9 +2285,9 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha , void *piomb)
 			ts->stat = SAS_QUEUE_FULL;
 			pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 			mb();/*ditto*/
-			spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+			spin_unlock_irq(&pm8001_ha->lock);
 			t->task_done(t);
-			spin_lock_irqsave(&pm8001_ha->lock, flags);
+			spin_lock_irq(&pm8001_ha->lock);
 			return;
 		}
 		break;
@@ -2387,31 +2385,31 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha , void *piomb)
 		ts->stat = SAS_OPEN_TO;
 		break;
 	}
-	spin_lock_irqsave(&t->task_state_lock, flags);
+	spin_lock_irq(&t->task_state_lock);
 	t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
 	t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
 	t->task_state_flags |= SAS_TASK_STATE_DONE;
 	if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
-		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irq(&t->task_state_lock);
 		PM8001_FAIL_DBG(pm8001_ha,
 			pm8001_printk("task 0x%p done with io_status 0x%x"
 			" resp 0x%x stat 0x%x but aborted by upper layer!\n",
 			t, event, ts->resp, ts->stat));
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 	} else if (t->uldd_task) {
-		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irq(&t->task_state_lock);
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 		mb();/* ditto */
-		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+		spin_unlock_irq(&pm8001_ha->lock);
 		t->task_done(t);
-		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		spin_lock_irq(&pm8001_ha->lock);
 	} else if (!t->uldd_task) {
-		spin_unlock_irqrestore(&t->task_state_lock, flags);
+		spin_unlock_irq(&t->task_state_lock);
 		pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
 		mb();/*ditto*/
-		spin_unlock_irqrestore(&pm8001_ha->lock, flags);
+		spin_unlock_irq(&pm8001_ha->lock);
 		t->task_done(t);
-		spin_lock_irqsave(&pm8001_ha->lock, flags);
+		spin_lock_irq(&pm8001_ha->lock);
 	}
 }
 
-- 
1.7.4.4


             reply	other threads:[~2012-02-26 13:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-26 13:33 santosh nayak [this message]
2012-02-26 15:07 ` Dan Carpenter
2012-02-27  4:57   ` santosh prasad nayak
2012-02-27  6:55     ` Dan Carpenter
2012-02-27  8:29     ` Jack Wang
2012-03-08 13:32 ` Mark Salyzyn
2012-03-08 14:15   ` jack_wang
2012-03-08 16:50   ` santosh prasad nayak
2012-03-08 17:11     ` Mark Salyzyn
2012-03-08 20:48       ` santosh prasad nayak
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=1330263210-25134-1-git-send-email-santoshprasadnayak@gmail.com \
    --to=santoshprasadnayak@gmail.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 \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®