scsi: qla4xxx: Remove in_interrupt() from qla4_82xx_idc_lock()

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(), multiple 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.

Link: https://lore.kernel.org/r/20201126132952.2287996-8-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>
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h
index f5b382e..9210547 100644 (file)
--- 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 da903a5..4362d0e 100644 (file)
--- 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;
 }