[PATCH 07/14] scsi: qla4xxx: qla4_82xx_idc_lock(): Remove in_interrupt().

Sebastian Andrzej Siewior
2020-11-26T13:30:37Z

From: "Ahmed S. Darwish" <a.darwish@linutronix.de>

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

While in_interrupt() is ill-defined and does not provide what the name
suggests, it is not needed here: qla4_82xx_idc_lock() is always called
from process context. Below is an analysis of its callers:

- ql4_nx.c: qla4_82xx_need_reset_handler(), 1-second msleep() in a
loop.

- ql4_nx.c: qla4_82xx_isp_reset(), calls
qla4_8xxx_device_state_handler(), which has multiple msleep()s.

Beside direct calls, qla4_82xx_idc_lock() is also bound to
isp_operations ->idc_lock() hook. Other functions which are bound to the
same hook, e.g. qla4_83xx_drv_lock(), also have an msleep(). For
completeness, below is an analysis of all callers of that hook:

- ql4_83xx.c: qla4_83xx_need_reset_handler(), has an msleep()

- ql4_83xx.c: qla4_83xx_isp_reset(), calls
qla4_8xxx_device_state_handler(), which has multiple msleep()s.

- ql4_83xx.c: qla4_83xx_disable_pause(), all process context callers:
=> ql4_mbx.c: qla4xxx_mailbox_command(), msleep(), mutex_lock()
=> ql4_os.c: qla4xxx_recover_adapter(), schedule_timeout() in loop
=> ql4_os.c: qla4xxx_do_dpc(), workqueue context

- ql4_attr.c: qla4_8xxx_sysfs_write_fw_dump(), sysfs bin_attribute
->write() hook, process context

- ql4_mbx.c: qla4xxx_mailbox_command(), earlier discussed

- ql4_nx.c: qla4_8xxx_device_bootstrap(), callers:
=> ql4_83xx.c: qla4_83xx_need_reset_handler(), process, msleep()
=> ql4_nx.c: qla4_8xxx_device_state_handler(), earlier discussed

- ql4_nx.c: qla4_8xxx_need_qsnt_handler(), callers:
=> ql4_nx.c: qla4_8xxx_device_state_handler(), multipe msleep()s
=> ql4_os.c: qla4xxx_do_dpc(), workqueue context

- ql4_nx.c: qla4_8xxx_update_idc_reg(), callers:
=> ql4_nx.c: qla4_8xxx_device_state_handler(), earlier discussed
=> ql4_os.c: qla4_8xxx_error_recovery(), only called by
qla4xxx_pci_slot_reset(), which is bound to PCI ->slot_reset()
process-context hook

- ql4_nx.c: qla4_8xxx_device_state_handler(), earlier discussed

- ql4_os.c: qla4xxx_recover_adapter(), earlier discussed

- ql4_os.c: qla4xxx_do_dpc(), earlier discussed

Remove the in_interrupt() check. Mark, qla4_82xx_idc_lock(), and the
->idc_lock() hook itself, with "Context: task, can sleep".

Change qla4_82xx_idc_lock() implementation to sleep 100ms, instead of a
schedule(), for each spin. This is more deterministic, and it matches
other PCI HW locking functions in the driver.

Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Manish Rangankar <mrangankar@marvell.com>
Cc: <GR-QLogic-Storage-Upstream@marvell.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/scsi/qla4xxx/ql4_def.h | 2 +-
drivers/scsi/qla4xxx/ql4_nx.c | 14 +++++---------
2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h
index f5b382ed0a1b2..9210547483886 100644
--- a/drivers/scsi/qla4xxx/ql4_def.h
+++ b/drivers/scsi/qla4xxx/ql4_def.h
@@ -435,7 +435,7 @@ struct isp_operations {
void (*wr_reg_direct) (struct scsi_qla_host *, ulong, uint32_t);
int (*rd_reg_indirect) (struct scsi_qla_host *, uint32_t, uint32_t *);
int (*wr_reg_indirect) (struct scsi_qla_host *, uint32_t, uint32_t);
- int (*idc_lock) (struct scsi_qla_host *);
+ int (*idc_lock) (struct scsi_qla_host *); /* Context: task, can sleep */
void (*idc_unlock) (struct scsi_qla_host *);
void (*rom_lock_recovery) (struct scsi_qla_host *);
void (*queue_mailbox_command) (struct scsi_qla_host *, uint32_t *, int);
diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c
index da903a545b2c0..4362d0ebe0e15 100644
--- a/drivers/scsi/qla4xxx/ql4_nx.c
+++ b/drivers/scsi/qla4xxx/ql4_nx.c
@@ -512,12 +512,15 @@ int qla4_82xx_md_wr_32(struct scsi_qla_host *ha, uint32_t off, uint32_t data)
*
* General purpose lock used to synchronize access to
* CRB_DEV_STATE, CRB_DEV_REF_COUNT, etc.
+ *
+ * Context: task, can sleep
**/
int qla4_82xx_idc_lock(struct scsi_qla_host *ha)
{
- int i;
int done = 0, timeout = 0;

+ might_sleep();
+
while (!done) {
/* acquire semaphore5 from PCI HW block */
done = qla4_82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM5_LOCK));
@@ -527,14 +530,7 @@ int qla4_82xx_idc_lock(struct scsi_qla_host *ha)
return -1;

timeout++;
-
- /* Yield CPU */
- if (!in_interrupt())
- schedule();
- else {
- for (i = 0; i < 20; i++)
- cpu_relax(); /*This a nop instr on i386*/
- }
+ msleep(100);
}
return 0;
}
--
2.29.2