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@xxxxxxxxxxxx>
Signed-off-by: Sathya Prakash <sathya.prakash@xxxxxxxxxxxx>
---
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@xxxxxxxxxx>
--
Himanshu Madhani Oracle Linux Engineering
