2019-06-21 06:37:42

by Minwoo Im

[permalink] [raw]
Subject: [RESEND RFC PATCH] mpt3sas: support target smid for [abort|query] task

We can request task management IOCTL command(MPI2_FUNCTION_SCSI_TASK_MGMT)
to /dev/mpt3ctl. If the given task_type is either abort task or query
task, it may need a field named "Initiator Port Transfer Tag to Manage"
in the IU.

Current code does not support to check target IPTT tag from the
tm_request. This patch introduces to check TaskMID given from the
userspace as a target tag. We have a rule of relationship between
(struct request *req->tag) and smid in mpt3sas_base.c:

3318 u16
3319 mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
3320 struct scsi_cmnd *scmd)
3321 {
3322 struct scsiio_tracker *request = scsi_cmd_priv(scmd);
3323 unsigned int tag = scmd->request->tag;
3324 u16 smid;
3325
3326 smid = tag + 1;

So if we want to abort a request tagged #X, then we can pass (X + 1) to
this IOCTL handler.

Cc: Sathya Prakash <[email protected]>
Cc: Suganath Prabu Subramani <[email protected]>
Cc: James E.J. Bottomley <[email protected]>
Cc: Martin K. Petersen <[email protected]>
Cc: [email protected]
Signed-off-by: Minwoo Im <[email protected]>
---
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index b2bb47c14d35..5c7539dae713 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -596,15 +596,17 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command *karg,
if (priv_data->sas_target->handle != handle)
continue;
st = scsi_cmd_priv(scmd);
- tm_request->TaskMID = cpu_to_le16(st->smid);
- found = 1;
+ if (tm_request->TaskMID == st->smid) {
+ tm_request->TaskMID = cpu_to_le16(st->smid);
+ found = 1;
+ }
}

if (!found) {
dctlprintk(ioc,
- ioc_info(ioc, "%s: handle(0x%04x), lun(%d), no active mid!!\n",
+ ioc_info(ioc, "%s: handle(0x%04x), lun(%d), no matched mid(%d)!!\n",
desc, le16_to_cpu(tm_request->DevHandle),
- lun));
+ lun, tm_request->TaskMID));
tm_reply = ioc->ctl_cmds.reply;
tm_reply->DevHandle = tm_request->DevHandle;
tm_reply->Function = MPI2_FUNCTION_SCSI_TASK_MGMT;
--
2.16.1


2019-06-27 07:40:57

by Minwoo Im

[permalink] [raw]
Subject: Re: [RESEND RFC PATCH] mpt3sas: support target smid for [abort|query] task

Gentle ping. :)

> -----Original Message-----
> From: Minwoo Im <[email protected]>
> Sent: Friday, June 21, 2019 3:37 PM
> To: [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Minwoo Im <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Euihyeok Kwon <[email protected]>; Sarah Cho
> <[email protected]>; Sanggwan Lee <[email protected]>;
> Gyeongmin Nam <[email protected]>
> Subject: [RESEND RFC PATCH] mpt3sas: support target smid for [abort|query]
> task
>
> We can request task management IOCTL command(MPI2_FUNCTION_SCSI_TASK_MGMT)
> to /dev/mpt3ctl. If the given task_type is either abort task or query
> task, it may need a field named "Initiator Port Transfer Tag to Manage"
> in the IU.
>
> Current code does not support to check target IPTT tag from the
> tm_request. This patch introduces to check TaskMID given from the
> userspace as a target tag. We have a rule of relationship between
> (struct request *req->tag) and smid in mpt3sas_base.c:
>
> 3318 u16
> 3319 mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
> 3320 struct scsi_cmnd *scmd)
> 3321 {
> 3322 struct scsiio_tracker *request = scsi_cmd_priv(scmd);
> 3323 unsigned int tag = scmd->request->tag;
> 3324 u16 smid;
> 3325
> 3326 smid = tag + 1;
>
> So if we want to abort a request tagged #X, then we can pass (X + 1) to
> this IOCTL handler.
>
> Cc: Sathya Prakash <[email protected]>
> Cc: Suganath Prabu Subramani <[email protected]>
> Cc: James E.J. Bottomley <[email protected]>
> Cc: Martin K. Petersen <[email protected]>
> Cc: [email protected]
> Signed-off-by: Minwoo Im <[email protected]>
> ---
> drivers/scsi/mpt3sas/mpt3sas_ctl.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> index b2bb47c14d35..5c7539dae713 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> @@ -596,15 +596,17 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc,
> struct mpt3_ioctl_command *karg,
> if (priv_data->sas_target->handle != handle)
> continue;
> st = scsi_cmd_priv(scmd);
> - tm_request->TaskMID = cpu_to_le16(st->smid);
> - found = 1;
> + if (tm_request->TaskMID == st->smid) {
> + tm_request->TaskMID = cpu_to_le16(st->smid);
> + found = 1;
> + }
> }
>
> if (!found) {
> dctlprintk(ioc,
> - ioc_info(ioc, "%s: handle(0x%04x), lun(%d), no
> active mid!!\n",
> + ioc_info(ioc, "%s: handle(0x%04x), lun(%d), no
> matched mid(%d)!!\n",
> desc, le16_to_cpu(tm_request->DevHandle),
> - lun));
> + lun, tm_request->TaskMID));
> tm_reply = ioc->ctl_cmds.reply;
> tm_reply->DevHandle = tm_request->DevHandle;
> tm_reply->Function = MPI2_FUNCTION_SCSI_TASK_MGMT;
> --
> 2.16.1

2019-07-12 08:28:05

by Sreekanth Reddy

[permalink] [raw]
Subject: Re: [RESEND RFC PATCH] mpt3sas: support target smid for [abort|query] task

On Fri, Jun 21, 2019 at 12:07 PM Minwoo Im <[email protected]> wrote:
>
> We can request task management IOCTL command(MPI2_FUNCTION_SCSI_TASK_MGMT)
> to /dev/mpt3ctl. If the given task_type is either abort task or query
> task, it may need a field named "Initiator Port Transfer Tag to Manage"
> in the IU.
>
> Current code does not support to check target IPTT tag from the
> tm_request. This patch introduces to check TaskMID given from the
> userspace as a target tag. We have a rule of relationship between
> (struct request *req->tag) and smid in mpt3sas_base.c:
>
> 3318 u16
> 3319 mpt3sas_base_get_smid_scsiio(struct MPT3SAS_ADAPTER *ioc, u8 cb_idx,
> 3320 struct scsi_cmnd *scmd)
> 3321 {
> 3322 struct scsiio_tracker *request = scsi_cmd_priv(scmd);
> 3323 unsigned int tag = scmd->request->tag;
> 3324 u16 smid;
> 3325
> 3326 smid = tag + 1;
>
> So if we want to abort a request tagged #X, then we can pass (X + 1) to
> this IOCTL handler.
>
> Cc: Sathya Prakash <[email protected]>
> Cc: Suganath Prabu Subramani <[email protected]>
> Cc: James E.J. Bottomley <[email protected]>
> Cc: Martin K. Petersen <[email protected]>
> Cc: [email protected]
> Signed-off-by: Minwoo Im <[email protected]>
> ---
> drivers/scsi/mpt3sas/mpt3sas_ctl.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> index b2bb47c14d35..5c7539dae713 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> @@ -596,15 +596,17 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command *karg,
> if (priv_data->sas_target->handle != handle)
> continue;
> st = scsi_cmd_priv(scmd);
> - tm_request->TaskMID = cpu_to_le16(st->smid);
> - found = 1;
> + if (tm_request->TaskMID == st->smid) {

I think it will difficult for the user to find the smid that he want
to abort. For this user has to enable the scsi logging level and get
the tag and pass the ioctl with tag +1 value in TaskMID field. And
hence currently driver will loop over all the smid's and if it fines
any outstanding smid then it will issue task abort or task query TM
for this outstanding smid to the HBA firmware.

May be we can do like below,
* First check whether user provided "TaskMID" is non zero or not. if
user provided TaskMID is non-zero and if this TaskMID is outstanding
then driver will issue TaskAbort/QueryTask TM with this TaskMID value
else driver will loop over all the smid's and if finds any smid is
outstanding then it will issue TaskAbort/QueryTask TM with TaskMID
value set to outstanding smid.

With the above logic still legacy application will be supported
without breaking anything where they provide TaskMID filed as zero.
And it also allows the user to abort the IO which he wants.

Thanks,
Sreekanth

> + tm_request->TaskMID = cpu_to_le16(st->smid);
> + found = 1;
> + }
> }
>
> if (!found) {
> dctlprintk(ioc,
> - ioc_info(ioc, "%s: handle(0x%04x), lun(%d), no active mid!!\n",
> + ioc_info(ioc, "%s: handle(0x%04x), lun(%d), no matched mid(%d)!!\n",
> desc, le16_to_cpu(tm_request->DevHandle),
> - lun));
> + lun, tm_request->TaskMID));
> tm_reply = ioc->ctl_cmds.reply;
> tm_reply->DevHandle = tm_request->DevHandle;
> tm_reply->Function = MPI2_FUNCTION_SCSI_TASK_MGMT;
> --
> 2.16.1

2019-07-14 02:21:03

by Minwoo Im

[permalink] [raw]
Subject: Re: [RESEND RFC PATCH] mpt3sas: support target smid for [abort|query] task

Hi Sreekanth,

> > drivers/scsi/mpt3sas/mpt3sas_ctl.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> > index b2bb47c14d35..5c7539dae713 100644
> > --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> > +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
> > @@ -596,15 +596,17 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc,
> struct mpt3_ioctl_command *karg,
> > if (priv_data->sas_target->handle != handle)
> > continue;
> > st = scsi_cmd_priv(scmd);
> > - tm_request->TaskMID = cpu_to_le16(st->smid);
> > - found = 1;
> > + if (tm_request->TaskMID == st->smid) {
>
> I think it will difficult for the user to find the smid that he want
> to abort. For this user has to enable the scsi logging level and get
> the tag and pass the ioctl with tag +1 value in TaskMID field. And
> hence currently driver will loop over all the smid's and if it fines
> any outstanding smid then it will issue task abort or task query TM
> for this outstanding smid to the HBA firmware.

Sreekanth,

You're exactly right because I have done this kind of abort based on
The scsi logs with logging level configured.

>
> May be we can do like below,
> * First check whether user provided "TaskMID" is non zero or not. if
> user provided TaskMID is non-zero and if this TaskMID is outstanding
> then driver will issue TaskAbort/QueryTask TM with this TaskMID value

Okay. If I get it right, you mean to check the given TaskMID(!=0) is
Outstanding or not is by an iteration first.

> else driver will loop over all the smid's and if finds any smid is
> outstanding then it will issue TaskAbort/QueryTask TM with TaskMID
> value set to outstanding smid.

Got your point here. I'll make a conditional statement for the case
where the TaskMID is zero which is the legacy behaviour.

> With the above logic still legacy application will be supported
> without breaking anything where they provide TaskMID filed as zero.
> And it also allows the user to abort the IO which he wants.

Sure, I'll prepare V2 patch soon.

Thanks Sreekanth,

Minwoo Im