| Message ID | 20250303094004.93770-1-chandrakanth.patil@broadcom.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mpi3mr: Task Abort EH Support | expand |
Hi Chandrakanth, Code looks fine … just couple of small nits into messages for better readability. On 3/3/25 01:40, Chandrakanth Patil wrote: > Task Abort support is added to handle SCSI command timeouts, ensuring > recovery and cleanup of timed-out commands. This completes the error > handling framework for mpi3mr driver, which already includes device > reset, target reset, bus reset, and host reset. > > Signed-off-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com> > Signed-off-by: Sathya Prakash <sathya.prakash@broadcom.com> > --- > drivers/scsi/mpi3mr/mpi3mr_os.c | 99 +++++++++++++++++++++++++++++++++ > 1 file changed, 99 insertions(+) > > diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c > index e3547ea42613..6a8f3d3a5668 100644 > --- a/drivers/scsi/mpi3mr/mpi3mr_os.c > +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c > @@ -3839,6 +3839,18 @@ int mpi3mr_issue_tm(struct mpi3mr_ioc *mrioc, u8 tm_type, > tgtdev = mpi3mr_get_tgtdev_by_handle(mrioc, handle); > > if (scmd) { > + if (tm_type == MPI3_SCSITASKMGMT_TASKTYPE_ABORT_TASK) { > + cmd_priv = scsi_cmd_priv(scmd); > + if (!cmd_priv) > + goto out_unlock; > + > + struct op_req_qinfo *op_req_q; > + > + op_req_q = &mrioc->req_qinfo[cmd_priv->req_q_idx]; > + tm_req.task_host_tag = cpu_to_le16(cmd_priv->host_tag); > + tm_req.task_request_queue_id = > + cpu_to_le16(op_req_q->qid); > + } > sdev = scmd->device; > sdev_priv_data = sdev->hostdata; > scsi_tgt_priv_data = ((sdev_priv_data) ? > @@ -4387,6 +4399,92 @@ static int mpi3mr_eh_dev_reset(struct scsi_cmnd *scmd) > return retval; > } > > +/** > + * mpi3mr_eh_abort- Abort error handling callback ^^^ function comment should be reworded as "Callback function for abort error handling" > + * @scmd: SCSI command reference > + * > + * Issue Abort Task Management if the command is in LLD scope > + * and verify if it is aborted successfully and return status > + * accordingly. > + * > + * Return: SUCCESS of successful abort the scmd else FAILED > + */ > +static int mpi3mr_eh_abort(struct scsi_cmnd *scmd) > +{ > + struct mpi3mr_ioc *mrioc = shost_priv(scmd->device->host); > + struct mpi3mr_stgt_priv_data *stgt_priv_data; > + struct mpi3mr_sdev_priv_data *sdev_priv_data; > + struct scmd_priv *cmd_priv; > + u16 dev_handle, timeout = MPI3MR_ABORTTM_TIMEOUT; > + u8 resp_code = 0; > + int retval = FAILED, ret = 0; > + struct request *rq = scsi_cmd_to_rq(scmd); > + unsigned long scmd_age_ms = jiffies_to_msecs(jiffies - scmd->jiffies_at_alloc); > + unsigned long scmd_age_sec = scmd_age_ms / HZ; > + > + sdev_printk(KERN_INFO, scmd->device, > + "%s: attempting abort task for scmd(%p)\n", mrioc->name, scmd); > + > + sdev_printk(KERN_INFO, scmd->device, > + "%s: scmd(0x%p) is outstanding for %lus %lums, timeout %us, retries %d, allowed %d\n", > + mrioc->name, scmd, scmd_age_sec, scmd_age_ms % HZ, rq->timeout / HZ, > + scmd->retries, scmd->allowed); > + > + scsi_print_command(scmd); > + > + sdev_priv_data = scmd->device->hostdata; > + if (!sdev_priv_data || !sdev_priv_data->tgt_priv_data) { > + sdev_printk(KERN_INFO, scmd->device, > + "%s: device is not available, abort task is not issued\n", ^^^ This message can be reworded as "%s: Device not available, Skip issuing abort task\n" > + mrioc->name); > + retval = SUCCESS; > + goto out; > + } > + > + stgt_priv_data = sdev_priv_data->tgt_priv_data; > + dev_handle = stgt_priv_data->dev_handle; > + > + cmd_priv = scsi_cmd_priv(scmd); > + if (!cmd_priv->in_lld_scope || > + cmd_priv->host_tag == MPI3MR_HOSTTAG_INVALID) { > + sdev_printk(KERN_INFO, scmd->device, > + "%s: scmd is not in LLD scope, abort task is not issued\n", Same here, message should be reworded "%s: scmd (0x%p) not in LLD scope, Skip issuing Abort Task.\n" Also add scmd pointer for easy debugging in the future. > + mrioc->name); > + retval = SUCCESS; > + goto out; > + } > + > + if (stgt_priv_data->dev_removed) { > + sdev_printk(KERN_INFO, scmd->device, > + "%s: device(handle = 0x%04x) is removed, abort task is not issued\n", This message should be reworded as "%s: Device (handle = 0x%04x) removed, Skip issuing Abort Task.\n" > + mrioc->name, dev_handle); > + retval = FAILED; > + goto out; > + } > + > + ret = mpi3mr_issue_tm(mrioc, MPI3_SCSITASKMGMT_TASKTYPE_ABORT_TASK, > + dev_handle, sdev_priv_data->lun_id, MPI3MR_HOSTTAG_BLK_TMS, > + timeout, &mrioc->host_tm_cmds, &resp_code, scmd); > + > + if (ret) > + goto out; > + > + if (cmd_priv->in_lld_scope) { > + sdev_printk(KERN_INFO, scmd->device, > + "%s: scmd was not terminated, abort task is failed\n", This message could be reworded as "%s: Abort task failed. scmd (0x%p) was not terminated.\n" > + mrioc->name); > + goto out; > + } > + > + retval = SUCCESS; > +out: > + sdev_printk(KERN_INFO, scmd->device, > + "%s: abort task is %s for scmd(%p)\n", mrioc->name, > + ((retval == SUCCESS) ? "SUCCESS" : "FAILED"), scmd); > + Message printed for Successful case should be "SUCCEEDED" for better readability. Something like "%s: Abort Task %s for scmd (0x%p)\n" > + return retval; > +} > + > /** > * mpi3mr_scan_start - Scan start callback handler > * @shost: SCSI host reference > @@ -5069,6 +5167,7 @@ static const struct scsi_host_template mpi3mr_driver_template = { > .scan_finished = mpi3mr_scan_finished, > .scan_start = mpi3mr_scan_start, > .change_queue_depth = mpi3mr_change_queue_depth, > + .eh_abort_handler = mpi3mr_eh_abort, > .eh_device_reset_handler = mpi3mr_eh_dev_reset, > .eh_target_reset_handler = mpi3mr_eh_target_reset, > .eh_bus_reset_handler = mpi3mr_eh_bus_reset, once you fix these messages feel free to add Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index e3547ea42613..6a8f3d3a5668 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -3839,6 +3839,18 @@ int mpi3mr_issue_tm(struct mpi3mr_ioc *mrioc, u8 tm_type, tgtdev = mpi3mr_get_tgtdev_by_handle(mrioc, handle); if (scmd) { + if (tm_type == MPI3_SCSITASKMGMT_TASKTYPE_ABORT_TASK) { + cmd_priv = scsi_cmd_priv(scmd); + if (!cmd_priv) + goto out_unlock; + + struct op_req_qinfo *op_req_q; + + op_req_q = &mrioc->req_qinfo[cmd_priv->req_q_idx]; + tm_req.task_host_tag = cpu_to_le16(cmd_priv->host_tag); + tm_req.task_request_queue_id = + cpu_to_le16(op_req_q->qid); + } sdev = scmd->device; sdev_priv_data = sdev->hostdata; scsi_tgt_priv_data = ((sdev_priv_data) ? @@ -4387,6 +4399,92 @@ static int mpi3mr_eh_dev_reset(struct scsi_cmnd *scmd) return retval; } +/** + * mpi3mr_eh_abort- Abort error handling callback + * @scmd: SCSI command reference + * + * Issue Abort Task Management if the command is in LLD scope + * and verify if it is aborted successfully and return status + * accordingly. + * + * Return: SUCCESS of successful abort the scmd else FAILED + */ +static int mpi3mr_eh_abort(struct scsi_cmnd *scmd) +{ + struct mpi3mr_ioc *mrioc = shost_priv(scmd->device->host); + struct mpi3mr_stgt_priv_data *stgt_priv_data; + struct mpi3mr_sdev_priv_data *sdev_priv_data; + struct scmd_priv *cmd_priv; + u16 dev_handle, timeout = MPI3MR_ABORTTM_TIMEOUT; + u8 resp_code = 0; + int retval = FAILED, ret = 0; + struct request *rq = scsi_cmd_to_rq(scmd); + unsigned long scmd_age_ms = jiffies_to_msecs(jiffies - scmd->jiffies_at_alloc); + unsigned long scmd_age_sec = scmd_age_ms / HZ; + + sdev_printk(KERN_INFO, scmd->device, + "%s: attempting abort task for scmd(%p)\n", mrioc->name, scmd); + + sdev_printk(KERN_INFO, scmd->device, + "%s: scmd(0x%p) is outstanding for %lus %lums, timeout %us, retries %d, allowed %d\n", + mrioc->name, scmd, scmd_age_sec, scmd_age_ms % HZ, rq->timeout / HZ, + scmd->retries, scmd->allowed); + + scsi_print_command(scmd); + + sdev_priv_data = scmd->device->hostdata; + if (!sdev_priv_data || !sdev_priv_data->tgt_priv_data) { + sdev_printk(KERN_INFO, scmd->device, + "%s: device is not available, abort task is not issued\n", + mrioc->name); + retval = SUCCESS; + goto out; + } + + stgt_priv_data = sdev_priv_data->tgt_priv_data; + dev_handle = stgt_priv_data->dev_handle; + + cmd_priv = scsi_cmd_priv(scmd); + if (!cmd_priv->in_lld_scope || + cmd_priv->host_tag == MPI3MR_HOSTTAG_INVALID) { + sdev_printk(KERN_INFO, scmd->device, + "%s: scmd is not in LLD scope, abort task is not issued\n", + mrioc->name); + retval = SUCCESS; + goto out; + } + + if (stgt_priv_data->dev_removed) { + sdev_printk(KERN_INFO, scmd->device, + "%s: device(handle = 0x%04x) is removed, abort task is not issued\n", + mrioc->name, dev_handle); + retval = FAILED; + goto out; + } + + ret = mpi3mr_issue_tm(mrioc, MPI3_SCSITASKMGMT_TASKTYPE_ABORT_TASK, + dev_handle, sdev_priv_data->lun_id, MPI3MR_HOSTTAG_BLK_TMS, + timeout, &mrioc->host_tm_cmds, &resp_code, scmd); + + if (ret) + goto out; + + if (cmd_priv->in_lld_scope) { + sdev_printk(KERN_INFO, scmd->device, + "%s: scmd was not terminated, abort task is failed\n", + mrioc->name); + goto out; + } + + retval = SUCCESS; +out: + sdev_printk(KERN_INFO, scmd->device, + "%s: abort task is %s for scmd(%p)\n", mrioc->name, + ((retval == SUCCESS) ? "SUCCESS" : "FAILED"), scmd); + + return retval; +} + /** * mpi3mr_scan_start - Scan start callback handler * @shost: SCSI host reference @@ -5069,6 +5167,7 @@ static const struct scsi_host_template mpi3mr_driver_template = { .scan_finished = mpi3mr_scan_finished, .scan_start = mpi3mr_scan_start, .change_queue_depth = mpi3mr_change_queue_depth, + .eh_abort_handler = mpi3mr_eh_abort, .eh_device_reset_handler = mpi3mr_eh_dev_reset, .eh_target_reset_handler = mpi3mr_eh_target_reset, .eh_bus_reset_handler = mpi3mr_eh_bus_reset,