| Message ID | 1484923333-8509-3-git-send-email-chaitra.basappa@broadcom.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Fri, Jan 20, 2017 at 08:12:11PM +0530, Chaitra P B wrote: > Small glitch/degraded performance in Crusader is improved with SAS > drives by removing unnecessary spinlocks while clearing scsi command > in drivers internal lookup table. > > Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com> > Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com> > --- Whats wrong with creating a __scsih_scsi_lookup_get_clear() and calling it with the scsi_lookup_lock held from _scsih_scsi_lookup_get_clear() instead of duplicating the code? It's quite common in the linux kernel to have two functions func and __func, where func is func() { spin_lock(); __func(); spin_unlock(); }
On Fri, Jan 20, 2017 at 8:30 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote: > On Fri, Jan 20, 2017 at 08:12:11PM +0530, Chaitra P B wrote: >> Small glitch/degraded performance in Crusader is improved with SAS >> drives by removing unnecessary spinlocks while clearing scsi command >> in drivers internal lookup table. >> >> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com> >> Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com> >> --- > > Whats wrong with creating a __scsih_scsi_lookup_get_clear() and calling > it with the scsi_lookup_lock held from _scsih_scsi_lookup_get_clear() > instead of duplicating the code? > > It's quite common in the linux kernel to have two functions func and __func, > where func is > > func() > { > spin_lock(); > __func(); > spin_unlock(); > } > Sure, We will resend this path with this change. Thanks, Sreekanth > -- > Johannes Thumshirn Storage > jthumshirn@suse.de +49 911 74053 689 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index f00ef88..722fab9 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -5522,6 +5522,7 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc) goto out_free_resources; ioc->non_operational_loop = 0; + ioc->got_task_abort_from_ioctl = 0; return 0; out_free_resources: diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index dcb33f4..83cfa16 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -1000,6 +1000,7 @@ struct MPT3SAS_ADAPTER { u8 broadcast_aen_busy; u16 broadcast_aen_pending; u8 shost_recovery; + u8 got_task_abort_from_ioctl; struct mutex reset_in_progress_mutex; spinlock_t ioc_reset_in_progress_lock; diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index 95f0f24..02fe1c4 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -826,16 +826,18 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg, "TASK_MGMT: handle(0x%04x), task_type(0x%02x)\n", ioc->name, le16_to_cpu(tm_request->DevHandle), tm_request->TaskType)); - + ioc->got_task_abort_from_ioctl = 1; if (tm_request->TaskType == MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK || tm_request->TaskType == MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK) { if (_ctl_set_task_mid(ioc, &karg, tm_request)) { mpt3sas_base_free_smid(ioc, smid); + ioc->got_task_abort_from_ioctl = 0; goto out; } } + ioc->got_task_abort_from_ioctl = 0; if (test_bit(device_handle, ioc->device_remove_in_progress)) { dtmprintk(ioc, pr_info(MPT3SAS_FMT diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index 0f71f4a..217e0fe 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -1074,6 +1074,26 @@ _scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc, u16 smid) } /** + * _scsih_scsi_lookup_get_clear_without_lock - returns scmd entry without + * holding any lock. + * @ioc: per adapter object + * @smid: system request message index + * + * Returns the smid stored scmd pointer. + * Then will dereference the stored scmd pointer. + */ +static inline struct scsi_cmnd * +_scsih_scsi_lookup_get_clear_without_lock(struct MPT3SAS_ADAPTER *ioc, + u16 smid) +{ + struct scsi_cmnd *scmd = NULL; + + swap(scmd, ioc->scsi_lookup[smid - 1].scmd); + + return scmd; +} + +/** * _scsih_scsi_lookup_get_clear - returns scmd entry * @ioc: per adapter object * @smid: system request message index @@ -1088,8 +1108,7 @@ _scsih_scsi_lookup_get_clear(struct MPT3SAS_ADAPTER *ioc, u16 smid) struct scsi_cmnd *scmd; spin_lock_irqsave(&ioc->scsi_lookup_lock, flags); - scmd = ioc->scsi_lookup[smid - 1].scmd; - ioc->scsi_lookup[smid - 1].scmd = NULL; + scmd = _scsih_scsi_lookup_get_clear_without_lock(ioc, smid); spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags); return scmd; @@ -4659,7 +4678,13 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply) unsigned long flags; mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply); - scmd = _scsih_scsi_lookup_get_clear(ioc, smid); + + if (ioc->broadcast_aen_busy || ioc->pci_error_recovery || + ioc->got_task_abort_from_ioctl) + scmd = _scsih_scsi_lookup_get_clear(ioc, smid); + else + scmd = _scsih_scsi_lookup_get_clear_without_lock(ioc, smid); + if (scmd == NULL) return 1;