| Message ID | 20201126132952.2287996-8-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: Remove in_interrupt() usage. | expand |
On Thu, Nov 26, 2020 at 02:29:45PM +0100, Sebastian Andrzej Siewior wrote: > 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> Reviewed-by: Daniel Wagner <dwagner@suse.de>
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; }