Re: [PATCH 07/10] mpt3sas: use hi-priority queue for TMFs
On 02/16/2017 11:23 AM, Sreekanth Reddy wrote: > On Thu, Feb 16, 2017 at 3:44 PM, Hannes Reinecke wrote: >> On 02/16/2017 11:09 AM, Sreekanth Reddy wrote: >>> On Tue, Jan 31, 2017 at 2:55 PM, Hannes Reinecke wrote: When sending a TMF via the ioctl interface we should be using the hi-priority queue instead of the scsi queue to be consistent with overall TMF usage. Signed-off-by: Hannes Reinecke --- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index 95f0f24..e952175 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -720,7 +720,7 @@ enum block_state { } } else { - smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, NULL); + smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); >>> >>> This else part is not for TM path, It is for IO path. For the TM >>> request we are already using smid from hpr queue, as shown below >>> >>> if (mpi_request->Function == MPI2_FUNCTION_SCSI_TASK_MGMT) { >>> smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); >>> if (!smid) { >>> pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n", >>> ioc->name, __func__); >>> ret = -EAGAIN; >>> goto out; >>> } >>> } else { >>> >>> smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, >>> NULL); >>> >> Yes, I know. >> But the current method of inserting a SCSI command completely goes >> against blk-mq request usage; with blk-mq the tags are managed with the >> block layer, so you need to integrate with that to get a correct tag. >> >> Is this _really_ a crucial functionality? Can't we just drop it and use >> sg/bsg for this? >> It would make my life _so_ much easier; the alternative would be to >> either implement 'real' reserved command handling or rewriting the above >> code-path in terms of 'struct request', packing and unpacking as we go. >> Neither is very appealing. > > I think it is better to take out one request frame from can_queue > count and reserve it for this ioctl scsi io. Since we allow only one > ioctl command at a time. > Ok, that makes things easier. Will be updating the code. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 07/10] mpt3sas: use hi-priority queue for TMFs
On Thu, Feb 16, 2017 at 3:44 PM, Hannes Reinecke wrote: > On 02/16/2017 11:09 AM, Sreekanth Reddy wrote: >> On Tue, Jan 31, 2017 at 2:55 PM, Hannes Reinecke wrote: >>> When sending a TMF via the ioctl interface we should be using >>> the hi-priority queue instead of the scsi queue to be consistent >>> with overall TMF usage. >>> >>> Signed-off-by: Hannes Reinecke >>> --- >>> drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c >>> b/drivers/scsi/mpt3sas/mpt3sas_ctl.c >>> index 95f0f24..e952175 100644 >>> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c >>> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c >>> @@ -720,7 +720,7 @@ enum block_state { >>> } >>> } else { >>> >>> - smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, >>> NULL); >>> + smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); >> >> This else part is not for TM path, It is for IO path. For the TM >> request we are already using smid from hpr queue, as shown below >> >> if (mpi_request->Function == MPI2_FUNCTION_SCSI_TASK_MGMT) { >> smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); >> if (!smid) { >> pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n", >> ioc->name, __func__); >> ret = -EAGAIN; >> goto out; >> } >> } else { >> >> smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, >> NULL); >> > Yes, I know. > But the current method of inserting a SCSI command completely goes > against blk-mq request usage; with blk-mq the tags are managed with the > block layer, so you need to integrate with that to get a correct tag. > > Is this _really_ a crucial functionality? Can't we just drop it and use > sg/bsg for this? > It would make my life _so_ much easier; the alternative would be to > either implement 'real' reserved command handling or rewriting the above > code-path in terms of 'struct request', packing and unpacking as we go. > Neither is very appealing. I think it is better to take out one request frame from can_queue count and reserve it for this ioctl scsi io. Since we allow only one ioctl command at a time. Thanks, Sreekanth > > Cheers, > > Hannes > -- > Dr. Hannes ReineckeTeamlead Storage & Networking > h...@suse.de +49 911 74053 688 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton > HRB 21284 (AG Nürnberg)
Re: [PATCH 07/10] mpt3sas: use hi-priority queue for TMFs
On 02/16/2017 11:09 AM, Sreekanth Reddy wrote: > On Tue, Jan 31, 2017 at 2:55 PM, Hannes Reinecke wrote: >> When sending a TMF via the ioctl interface we should be using >> the hi-priority queue instead of the scsi queue to be consistent >> with overall TMF usage. >> >> Signed-off-by: Hannes Reinecke >> --- >> drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c >> b/drivers/scsi/mpt3sas/mpt3sas_ctl.c >> index 95f0f24..e952175 100644 >> --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c >> +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c >> @@ -720,7 +720,7 @@ enum block_state { >> } >> } else { >> >> - smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, >> NULL); >> + smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); > > This else part is not for TM path, It is for IO path. For the TM > request we are already using smid from hpr queue, as shown below > > if (mpi_request->Function == MPI2_FUNCTION_SCSI_TASK_MGMT) { > smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); > if (!smid) { > pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n", > ioc->name, __func__); > ret = -EAGAIN; > goto out; > } > } else { > > smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, > NULL); > Yes, I know. But the current method of inserting a SCSI command completely goes against blk-mq request usage; with blk-mq the tags are managed with the block layer, so you need to integrate with that to get a correct tag. Is this _really_ a crucial functionality? Can't we just drop it and use sg/bsg for this? It would make my life _so_ much easier; the alternative would be to either implement 'real' reserved command handling or rewriting the above code-path in terms of 'struct request', packing and unpacking as we go. Neither is very appealing. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH 07/10] mpt3sas: use hi-priority queue for TMFs
On Tue, Jan 31, 2017 at 2:55 PM, Hannes Reinecke wrote: > When sending a TMF via the ioctl interface we should be using > the hi-priority queue instead of the scsi queue to be consistent > with overall TMF usage. > > Signed-off-by: Hannes Reinecke > --- > drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c > b/drivers/scsi/mpt3sas/mpt3sas_ctl.c > index 95f0f24..e952175 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c > @@ -720,7 +720,7 @@ enum block_state { > } > } else { > > - smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, > NULL); > + smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); This else part is not for TM path, It is for IO path. For the TM request we are already using smid from hpr queue, as shown below if (mpi_request->Function == MPI2_FUNCTION_SCSI_TASK_MGMT) { smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); if (!smid) { pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n", ioc->name, __func__); ret = -EAGAIN; goto out; } } else { smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, NULL); Thanks, Sreekanth > if (!smid) { > pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n", > ioc->name, __func__); > -- > 1.8.5.6 >
Re: [PATCH 07/10] mpt3sas: use hi-priority queue for TMFs
On Tue, Jan 31, 2017 at 10:25:57AM +0100, Hannes Reinecke wrote: > When sending a TMF via the ioctl interface we should be using > the hi-priority queue instead of the scsi queue to be consistent > with overall TMF usage. Looks fine, Reviewed-by: Christoph Hellwig
[PATCH 07/10] mpt3sas: use hi-priority queue for TMFs
When sending a TMF via the ioctl interface we should be using the hi-priority queue instead of the scsi queue to be consistent with overall TMF usage. Signed-off-by: Hannes Reinecke --- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index 95f0f24..e952175 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -720,7 +720,7 @@ enum block_state { } } else { - smid = mpt3sas_base_get_smid_scsiio(ioc, ioc->ctl_cb_idx, NULL); + smid = mpt3sas_base_get_smid_hpr(ioc, ioc->ctl_cb_idx); if (!smid) { pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n", ioc->name, __func__); -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html