Re: [PATCH 07/10] mpt3sas: use hi-priority queue for TMFs

2017-02-16 Thread Hannes Reinecke
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

2017-02-16 Thread Sreekanth Reddy
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

2017-02-16 Thread Hannes Reinecke
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

2017-02-16 Thread Sreekanth Reddy
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

2017-02-07 Thread Christoph Hellwig
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

2017-01-31 Thread Hannes Reinecke
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