| Message ID | 20190621063708epcms2p309f4173afabe5de28942ba15d13987f7@epcms2p3 (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RESEND,RFC] mpt3sas: support target smid for [abort|query] task | expand |
Gentle ping. :) > -----Original Message----- > From: Minwoo Im <minwoo.im@samsung.com> > Sent: Friday, June 21, 2019 3:37 PM > To: sathya.prakash@broadcom.com; suganath-prabu.subramani@broadcom.com; > jejb@linux.ibm.com; martin.petersen@oracle.com > Cc: Minwoo Im <minwoo.im@samsung.com>; MPT-FusionLinux.pdl@broadcom.com; > linux-kernel@vger.kernel.org; linux-scsi@vger.kernel.org; linux- > block@vger.kernel.org; Euihyeok Kwon <eh81.kwon@samsung.com>; Sarah Cho > <sohyeon.jo@samsung.com>; Sanggwan Lee <sanggwan.lee@samsung.com>; > Gyeongmin Nam <gm.nam@samsung.com> > 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 <sathya.prakash@broadcom.com> > Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> > Cc: James E.J. Bottomley <jejb@linux.ibm.com> > Cc: Martin K. Petersen <martin.petersen@oracle.com> > Cc: MPT-FusionLinux.pdl@broadcom.com > Signed-off-by: Minwoo Im <minwoo.im@samsung.com> > --- > 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
Sreekanth/Suganath, > 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: Please review. Thanks!
On Fri, Jun 21, 2019 at 12:07 PM Minwoo Im <minwoo.im@samsung.com> 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 <sathya.prakash@broadcom.com> > Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> > Cc: James E.J. Bottomley <jejb@linux.ibm.com> > Cc: Martin K. Petersen <martin.petersen@oracle.com> > Cc: MPT-FusionLinux.pdl@broadcom.com > Signed-off-by: Minwoo Im <minwoo.im@samsung.com> > --- > 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
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
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;
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 <sathya.prakash@broadcom.com> Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com> Cc: James E.J. Bottomley <jejb@linux.ibm.com> Cc: Martin K. Petersen <martin.petersen@oracle.com> Cc: MPT-FusionLinux.pdl@broadcom.com Signed-off-by: Minwoo Im <minwoo.im@samsung.com> --- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)