diff mbox series

[03/14] scsi: qla4xxx: qla4_82xx_crb_win_lock(): Remove in_interrupt().

Message ID 20201126132952.2287996-4-bigeasy@linutronix.de (mailing list archive)
State Accepted
Headers show
Series scsi: Remove in_interrupt() usage. | expand

Commit Message

Sebastian Andrzej Siewior Nov. 26, 2020, 1:29 p.m. UTC 
From: "Ahmed S. Darwish" <a.darwish@linutronix.de>

qla4_82xx_crb_win_lock() spins on a certain hardware state until it's
updated. At the end of each spin, if in_interrupt() is true, it does 20
loops of cpu_relax(). Otherwise, it yields the CPU.

The in_interrupt() macro is ill-defined as it does not provide what the
name suggests, and it does not catch the intended use-case here.

qla4_82xx_crb_win_lock() is always invoked with scsi_qla_host::hw_lock
acquired, with disabled interrupts. If the caller is in process context,
as in qla4_82xx_need_reset_handler(), then in_interrupt() will return
false even though it is not allowed to call schedule().

Remove the in_interrupt() check.

Change qla4_82xx_crb_win_lock() specification to a purely atomic
function. Mark it as static, remove its forward declaration, and move it
above its callers. To avoid hammering the PCI bus while spinning, use a
10 micro-second delay instead of cpu_relax().

Fixes: f4f5df23bf72 ("[SCSI] qla4xxx: Added support for ISP82XX")
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Manish Rangankar <mrangankar@marvell.com>
Cc: <GR-QLogic-Storage-Upstream@marvell.com>
---
 drivers/scsi/qla4xxx/ql4_glbl.h |  1 -
 drivers/scsi/qla4xxx/ql4_nx.c   | 63 +++++++++++++++------------------
 2 files changed, 29 insertions(+), 35 deletions(-)

Comments

Daniel Wagner Nov. 30, 2020, 1:54 p.m. UTC | #1 
On Thu, Nov 26, 2020 at 02:29:41PM +0100, Sebastian Andrzej Siewior wrote:
> From: "Ahmed S. Darwish" <a.darwish@linutronix.de>
> 
> qla4_82xx_crb_win_lock() spins on a certain hardware state until it's
> updated. At the end of each spin, if in_interrupt() is true, it does 20
> loops of cpu_relax(). Otherwise, it yields the CPU.
> 
> The in_interrupt() macro is ill-defined as it does not provide what the
> name suggests, and it does not catch the intended use-case here.
> 
> qla4_82xx_crb_win_lock() is always invoked with scsi_qla_host::hw_lock
> acquired, with disabled interrupts. If the caller is in process context,
> as in qla4_82xx_need_reset_handler(), then in_interrupt() will return
> false even though it is not allowed to call schedule().
> 
> Remove the in_interrupt() check.
> 
> Change qla4_82xx_crb_win_lock() specification to a purely atomic
> function. Mark it as static, remove its forward declaration, and move it
> above its callers. To avoid hammering the PCI bus while spinning, use a
> 10 micro-second delay instead of cpu_relax().
> 
> Fixes: f4f5df23bf72 ("[SCSI] qla4xxx: Added support for ISP82XX")
> Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Nilesh Javali <njavali@marvell.com>
> Cc: Manish Rangankar <mrangankar@marvell.com>
> Cc: <GR-QLogic-Storage-Upstream@marvell.com>

Reviewed-by: Daniel Wagner <dwagner@suse.de>
diff mbox series

Patch

diff --git a/drivers/scsi/qla4xxx/ql4_glbl.h b/drivers/scsi/qla4xxx/ql4_glbl.h
index b8f02210aeb03..ea60057b2e20a 100644
--- a/drivers/scsi/qla4xxx/ql4_glbl.h
+++ b/drivers/scsi/qla4xxx/ql4_glbl.h
@@ -114,7 +114,6 @@  irqreturn_t qla4_82xx_intr_handler(int irq, void *dev_id);
 void qla4_82xx_queue_iocb(struct scsi_qla_host *ha);
 void qla4_82xx_complete_iocb(struct scsi_qla_host *ha);
 
-int qla4_82xx_crb_win_lock(struct scsi_qla_host *);
 void qla4_82xx_crb_win_unlock(struct scsi_qla_host *);
 int qla4_82xx_pci_get_crb_addr_2M(struct scsi_qla_host *, ulong *);
 void qla4_82xx_wr_32(struct scsi_qla_host *, ulong, u32);
diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c
index f1767b21076f7..da903a545b2c0 100644
--- a/drivers/scsi/qla4xxx/ql4_nx.c
+++ b/drivers/scsi/qla4xxx/ql4_nx.c
@@ -375,6 +375,35 @@  qla4_82xx_pci_set_crbwindow_2M(struct scsi_qla_host *ha, ulong *off)
 	*off = (*off & MASK(16)) + CRB_INDIRECT_2M + ha->nx_pcibase;
 }
 
+#define CRB_WIN_LOCK_TIMEOUT 100000000
+
+/*
+ * Context: atomic
+ */
+static int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha)
+{
+	int done = 0, timeout = 0;
+
+	while (!done) {
+		/* acquire semaphore3 from PCI HW block */
+		done = qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_LOCK));
+		if (done == 1)
+			break;
+		if (timeout >= CRB_WIN_LOCK_TIMEOUT)
+			return -1;
+
+		timeout++;
+		udelay(10);
+	}
+	qla4_82xx_wr_32(ha, QLA82XX_CRB_WIN_LOCK_ID, ha->func_num);
+	return 0;
+}
+
+void qla4_82xx_crb_win_unlock(struct scsi_qla_host *ha)
+{
+	qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_UNLOCK));
+}
+
 void
 qla4_82xx_wr_32(struct scsi_qla_host *ha, ulong off, u32 data)
 {
@@ -475,40 +504,6 @@  int qla4_82xx_md_wr_32(struct scsi_qla_host *ha, uint32_t off, uint32_t data)
 	return rval;
 }
 
-#define CRB_WIN_LOCK_TIMEOUT 100000000
-
-int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha)
-{
-	int i;
-	int done = 0, timeout = 0;
-
-	while (!done) {
-		/* acquire semaphore3 from PCI HW block */
-		done = qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_LOCK));
-		if (done == 1)
-			break;
-		if (timeout >= CRB_WIN_LOCK_TIMEOUT)
-			return -1;
-
-		timeout++;
-
-		/* Yield CPU */
-		if (!in_interrupt())
-			schedule();
-		else {
-			for (i = 0; i < 20; i++)
-				cpu_relax();    /*This a nop instr on i386*/
-		}
-	}
-	qla4_82xx_wr_32(ha, QLA82XX_CRB_WIN_LOCK_ID, ha->func_num);
-	return 0;
-}
-
-void qla4_82xx_crb_win_unlock(struct scsi_qla_host *ha)
-{
-	qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_UNLOCK));
-}
-
 #define IDC_LOCK_TIMEOUT 100000000
 
 /**