On Fri, Feb 17, 2017 at 09:23:08AM +0100, Hannes Reinecke wrote:
> Enable lockless command submission for scsi-mq by moving the
> command structure into the payload for struct request.
>
> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> ---
> drivers/scsi/mpt3sas/mpt3sas_base.c | 123 ++++-----
> drivers/scsi/mpt3sas/mpt3sas_base.h | 13 +-
> drivers/scsi/mpt3sas/mpt3sas_ctl.c | 85 ++++--
> drivers/scsi/mpt3sas/mpt3sas_scsih.c | 460 ++++++++++++++-----------------
> drivers/scsi/mpt3sas/mpt3sas_warpdrive.c | 20 +-
> 5 files changed, 330 insertions(+), 371 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index dec86c4..e9470a3 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -865,9 +865,17 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
> struct scsiio_tracker *
> mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
> {
> + u32 unique_tag;
> + struct scsi_cmnd *cmd;
> +
> WARN_ON(!smid);
> WARN_ON(smid >= ioc->hi_priority_smid);
> - return &ioc->scsi_lookup[smid - 1];
> + unique_tag = smid - 1;
> + cmd = scsi_host_find_tag(ioc->shost, unique_tag);
> + if (cmd)
> + return scsi_cmd_priv(cmd);
> +
> + return NULL;
> }
>
> /**
> @@ -2343,34 +2351,23 @@ struct scsiio_tracker *
> mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
> struct scsi_cmnd *scmd)
> {
> - unsigned long flags;
> struct scsiio_tracker *request;
> u16 smid;
>
> - spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
> - if (list_empty(&ioc->free_list)) {
> - spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
> - pr_err(MPT3SAS_FMT "%s: smid not available\n",
> - ioc->name, __func__);
> - return 0;
> - }
> -
> if (!scmd) {
> - /* ioctl command, always use the first slot */
> - request = ioc->lookup[0];
> - request->scmd = NULL;
> + smid = 1;
> + request = mpt3sas_get_st_from_smid(ioc, smid);
How is this going to work? mpt3sas_get_st_from_smid fails if we
don't find a block layer tag derived from smid?
> /**
> * mpt3sas_base_free_smid - put smid back on free_list
> * @ioc: per adapter object
> @@ -2428,22 +2441,21 @@ struct scsiio_tracker *
> unsigned long flags;
> int i;
>
> if (smid < ioc->hi_priority_smid) {
> + struct scsiio_tracker *st;
>
> + st = mpt3sas_get_st_from_smid(ioc, smid);
> + if (WARN_ON(!st)) {
> + _base_recovery_check(ioc);
> + return;
> + }
> + mpt3sas_base_clear_st(ioc, st);
> _base_recovery_check(ioc);
st = mpt3sas_get_st_from_smid(ioc, smid);
if (!WARN_ON_ONCE(!st))
mpt3sas_base_clear_st(ioc, st);
_base_recovery_check(ioc);
return;
> /* pending command count */
> - spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
> - for (i = 0; i < ioc->scsiio_depth; i++)
> - if (ioc->scsi_lookup[i].cb_idx != 0xFF)
> - ioc->pending_io_count++;
> - spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
> + blk_mq_tagset_busy_iter(&ioc->shost->tag_set,
> + _count_pending, ioc);
You can't rely on blk-mq being used, and we'd really want to avoid
layering violations like this anyway.
