Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[John Garry]

On 29/05/2019 14:28, Hannes Reinecke wrote:
> Hi all,
>
> quite some drivers use internal commands for various purposes, most
> commonly sending TMFs or querying the HBA status.
> While these commands use the same submission mechanism than normal
> I/O commands, they will not be counted as outstanding commands,
> requiring those drivers to implement their own mechanism to figure
> out outstanding commands.
> This patchset enables the use of reserved tags for the SCSI midlayer,
> enabling LLDDs to rely on the block layer for tracking outstanding
> commands.
> More importantly, it allows LLDD to request a valid tag from the block
> layer without having to implement some tracking mechanism within the
> driver. This removes quite some hacks which were required for some
> drivers (eg. fnic or snic).
>
> As usual, comments and reviews are welcome.
>

Hi Hannes,

I was wondering if you have any plans to progress this series?

I don't mind helping out...

Cheers,
John

> Hannes Reinecke (24):
>   block: disable elevator for reserved tags
>   scsi: add scsi_{get,put}_reserved_cmd()
>   scsi: add 'nr_reserved_cmds' field to the SCSI host template
>   csiostor: use reserved command for LUN reset
>   scsi: add scsi_cmd_from_priv()
>   virtio_scsi: use reserved commands for TMF
>   scsi: add host tagset busy iterator
>   fnic: use reserved commands
>   fnic: use scsi_host_tagset_busy_iter() to traverse commands
>   scsi: allocate separate queue for reserved commands
>   scsi: add scsi_host_get_reserved_cmd()
>   hpsa: move hpsa_hba_inquiry after scsi_add_host()
>   hpsa: use reserved commands
>   hpsa: use blk_mq_tagset_busy_iter() to traverse outstanding commands
>   hpsa: drop refcount field from CommandList
>   snic: use reserved commands
>   snic: use tagset iter for traversing commands
>   scsi: Implement scsi_is_reserved_cmd()
>   aacraid: move scsi_add_host()
>   aacraid: use private commands
>   aacraid: replace cmd_list with scsi_host_tagset_busy_iter()
>   aacraid: use scsi_host_tagset_busy_iter() to traverse outstanding
>     commands
>   dpt_i2o: drop cmd_list usage
>   scsi: drop scsi command list
>
>  block/blk-mq.c                    |  22 +-
>  drivers/scsi/aacraid/aachba.c     | 125 +++---
>  drivers/scsi/aacraid/aacraid.h    |   6 +-
>  drivers/scsi/aacraid/comminit.c   |  28 +-
>  drivers/scsi/aacraid/commsup.c    | 134 +++---
>  drivers/scsi/aacraid/linit.c      | 302 +++++++------
>  drivers/scsi/csiostor/csio_init.c |   3 +-
>  drivers/scsi/csiostor/csio_scsi.c |  48 +-
>  drivers/scsi/dpt_i2o.c            |  23 +-
>  drivers/scsi/fnic/fnic_scsi.c     | 916 ++++++++++++++++----------------------
>  drivers/scsi/hosts.c              |  27 ++
>  drivers/scsi/hpsa.c               | 310 ++++++-------
>  drivers/scsi/hpsa.h               |   1 -
>  drivers/scsi/hpsa_cmd.h           |   1 -
>  drivers/scsi/scsi.c               |   1 -
>  drivers/scsi/scsi_lib.c           |  69 +--
>  drivers/scsi/scsi_scan.c          |   1 -
>  drivers/scsi/snic/snic.h          |   2 +-
>  drivers/scsi/snic/snic_main.c     |   3 +
>  drivers/scsi/snic/snic_scsi.c     | 502 ++++++++++-----------
>  drivers/scsi/virtio_scsi.c        | 100 ++---
>  include/linux/blk-mq.h            |   2 +
>  include/scsi/scsi_cmnd.h          |  12 +-
>  include/scsi/scsi_device.h        |   1 -
>  include/scsi/scsi_host.h          |  17 +-
>  include/scsi/scsi_tcq.h           |  30 ++
>  26 files changed, 1311 insertions(+), 1375 deletions(-)
>

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[Hannes Reinecke]

On 8/23/19 3:26 PM, John Garry wrote:
> On 29/05/2019 14:28, Hannes Reinecke wrote:
>> Hi all,
>>
>> quite some drivers use internal commands for various purposes, most
>> commonly sending TMFs or querying the HBA status.
>> While these commands use the same submission mechanism than normal
>> I/O commands, they will not be counted as outstanding commands,
>> requiring those drivers to implement their own mechanism to figure
>> out outstanding commands.
>> This patchset enables the use of reserved tags for the SCSI midlayer,
>> enabling LLDDs to rely on the block layer for tracking outstanding
>> commands.
>> More importantly, it allows LLDD to request a valid tag from the block
>> layer without having to implement some tracking mechanism within the
>> driver. This removes quite some hacks which were required for some
>> drivers (eg. fnic or snic).
>>
>> As usual, comments and reviews are welcome.
>>
> 
> Hi Hannes,
> 
> I was wondering if you have any plans to progress this series?
> 
> I don't mind helping out...
> 
Thanks for the reminder.
I'll need to re-evaluate this where we stand now with shared tags;
this patchset partially relies on them.
Will be sending an updated patchset.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      Teamlead Storage & Networking
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[John Garry]

On 26/08/2019 16:27, Hannes Reinecke wrote:
> On 8/23/19 3:26 PM, John Garry wrote:
>> On 29/05/2019 14:28, Hannes Reinecke wrote:
>>> Hi all,
>>>
>>> quite some drivers use internal commands for various purposes, most
>>> commonly sending TMFs or querying the HBA status.
>>> While these commands use the same submission mechanism than normal
>>> I/O commands, they will not be counted as outstanding commands,
>>> requiring those drivers to implement their own mechanism to figure
>>> out outstanding commands.
>>> This patchset enables the use of reserved tags for the SCSI midlayer,
>>> enabling LLDDs to rely on the block layer for tracking outstanding
>>> commands.
>>> More importantly, it allows LLDD to request a valid tag from the block
>>> layer without having to implement some tracking mechanism within the
>>> driver. This removes quite some hacks which were required for some
>>> drivers (eg. fnic or snic).
>>>
>>> As usual, comments and reviews are welcome.
>>>
>>
>> Hi Hannes,
>>
>> I was wondering if you have any plans to progress this series?
>>
>> I don't mind helping out...
>>
> Thanks for the reminder.

Hi Hannes,

> I'll need to re-evaluate this where we stand now with shared tags;

As Ming Lei mentioned in 
https://www.spinics.net/lists/linux-block/msg43779.html, the future for 
hostwide shared tags doesn't look bright. I would tend to agree.

> this patchset partially relies on them.

In which way? I didn't think/hope it did.

> Will be sending an updated patchset.

For me, the way I see forward is to upstream this series, in addition to 
Ming's, linked above.

As for LLDDs and the unique tag problem, I suggest that they use sbitmap 
to generate the tag internally if they want to expose multiple queues. 
 From our testing, using managed interrupts + sbitmap still far 
outperforms non-managed interrupts.

This approach would mean that we can still revisit hostwide shared tags 
or other some other approach in future.

Thanks,
John

>
> Cheers,
>
> Hannes
>

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[Hannes Reinecke]

On 9/2/19 10:47 AM, John Garry wrote:
> On 26/08/2019 16:27, Hannes Reinecke wrote:
>> On 8/23/19 3:26 PM, John Garry wrote:
>>> On 29/05/2019 14:28, Hannes Reinecke wrote:
>>>> Hi all,
>>>>
>>>> quite some drivers use internal commands for various purposes, most
>>>> commonly sending TMFs or querying the HBA status.
>>>> While these commands use the same submission mechanism than normal
>>>> I/O commands, they will not be counted as outstanding commands,
>>>> requiring those drivers to implement their own mechanism to figure
>>>> out outstanding commands.
>>>> This patchset enables the use of reserved tags for the SCSI midlayer,
>>>> enabling LLDDs to rely on the block layer for tracking outstanding
>>>> commands.
>>>> More importantly, it allows LLDD to request a valid tag from the block
>>>> layer without having to implement some tracking mechanism within the
>>>> driver. This removes quite some hacks which were required for some
>>>> drivers (eg. fnic or snic).
>>>>
>>>> As usual, comments and reviews are welcome.
>>>>
>>>
>>> Hi Hannes,
>>>
>>> I was wondering if you have any plans to progress this series?
>>>
>>> I don't mind helping out...
>>>
>> Thanks for the reminder.
> 
> Hi Hannes,
> 
>> I'll need to re-evaluate this where we stand now with shared tags;
> 
> As Ming Lei mentioned in
> https://www.spinics.net/lists/linux-block/msg43779.html, the future for
> hostwide shared tags doesn't look bright. I would tend to agree.
> 
>> this patchset partially relies on them.
> 
> In which way? I didn't think/hope it did.
> 
>> Will be sending an updated patchset.
> 
> For me, the way I see forward is to upstream this series, in addition to
> Ming's, linked above.
> 
> As for LLDDs and the unique tag problem, I suggest that they use sbitmap
> to generate the tag internally if they want to expose multiple queues.
> From our testing, using managed interrupts + sbitmap still far
> outperforms non-managed interrupts.
> 
> This approach would mean that we can still revisit hostwide shared tags
> or other some other approach in future.
> 
Hmm. True. Will be checking.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      Teamlead Storage & Networking
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[John Garry]

On 23/08/2019 14:26, John Garry wrote:
>>
>> quite some drivers use internal commands for various purposes, most
>> commonly sending TMFs or querying the HBA status.
>> While these commands use the same submission mechanism than normal
>> I/O commands, they will not be counted as outstanding commands,
>> requiring those drivers to implement their own mechanism to figure
>> out outstanding commands.
>> This patchset enables the use of reserved tags for the SCSI midlayer,
>> enabling LLDDs to rely on the block layer for tracking outstanding
>> commands.
>> More importantly, it allows LLDD to request a valid tag from the block
>> layer without having to implement some tracking mechanism within the
>> driver. This removes quite some hacks which were required for some
>> drivers (eg. fnic or snic).
>>
>> As usual, comments and reviews are welcome.
>>
> 
Hi Hannes,

JFYI, I have rebased this series to 5.6-rc4 here:

https://github.com/hisilicon/kernel-dev/tree/private-topic-sas-5.6-resv-commands-v1

I am interested in enabling this for libsas and associated HBAs, so 
there are some patches on top for that.

No review comments have been addressed, apart from removing "block: 
disable elevator for reserved tags"

Please let me know your plan for this series.

Thanks,
John

> 
> I was wondering if you have any plans to progress this series?
> 
> I don't mind helping out...
> 
> Cheers,
> John
> 
>> Hannes Reinecke (24):
>>   block: disable elevator for reserved tags
>>   scsi: add scsi_{get,put}_reserved_cmd()
>>   scsi: add 'nr_reserved_cmds' field to the SCSI host template
>>   csiostor: use reserved command for LUN reset
>>   scsi: add scsi_cmd_from_priv()
>>   virtio_scsi: use reserved commands for TMF
>>   scsi: add host tagset busy iterator
>>   fnic: use reserved commands
>>   fnic: use scsi_host_tagset_busy_iter() to traverse commands
>>   scsi: allocate separate queue for reserved commands
>>   scsi: add scsi_host_get_reserved_cmd()
>>   hpsa: move hpsa_hba_inquiry after scsi_add_host()
>>   hpsa: use reserved commands 

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[Hannes Reinecke]

On 3/9/20 1:48 PM, John Garry wrote:
> On 23/08/2019 14:26, John Garry wrote:
>>>
>>> quite some drivers use internal commands for various purposes, most
>>> commonly sending TMFs or querying the HBA status.
>>> While these commands use the same submission mechanism than normal
>>> I/O commands, they will not be counted as outstanding commands,
>>> requiring those drivers to implement their own mechanism to figure
>>> out outstanding commands.
>>> This patchset enables the use of reserved tags for the SCSI midlayer,
>>> enabling LLDDs to rely on the block layer for tracking outstanding
>>> commands.
>>> More importantly, it allows LLDD to request a valid tag from the block
>>> layer without having to implement some tracking mechanism within the
>>> driver. This removes quite some hacks which were required for some
>>> drivers (eg. fnic or snic).
>>>
>>> As usual, comments and reviews are welcome.
>>>
>>
> Hi Hannes,
> 
> JFYI, I have rebased this series to 5.6-rc4 here:
> 
> https://github.com/hisilicon/kernel-dev/tree/private-topic-sas-5.6-resv-commands-v1
> 
> 
> I am interested in enabling this for libsas and associated HBAs, so
> there are some patches on top for that.
> 
> No review comments have been addressed, apart from removing "block:
> disable elevator for reserved tags"
> 
> Please let me know your plan for this series.
> 
The plan is to wait for the shared tagset patchset to go in first :-)

Without it we'll be running on one tag bitmap per queue, and we risk
duplicate tags when sending down reserved commands :-(

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[John Garry]

On 09/03/2020 13:57, Hannes Reinecke wrote:
> On 3/9/20 1:48 PM, John Garry wrote:
>> On 23/08/2019 14:26, John Garry wrote:
>>>>
>>>> quite some drivers use internal commands for various purposes, most
>>>> commonly sending TMFs or querying the HBA status.
>>>> While these commands use the same submission mechanism than normal
>>>> I/O commands, they will not be counted as outstanding commands,
>>>> requiring those drivers to implement their own mechanism to figure
>>>> out outstanding commands.
>>>> This patchset enables the use of reserved tags for the SCSI midlayer,
>>>> enabling LLDDs to rely on the block layer for tracking outstanding
>>>> commands.
>>>> More importantly, it allows LLDD to request a valid tag from the block
>>>> layer without having to implement some tracking mechanism within the
>>>> driver. This removes quite some hacks which were required for some
>>>> drivers (eg. fnic or snic).
>>>>
>>>> As usual, comments and reviews are welcome.
>>>>
>>>
>> Hi Hannes,
>>
>> JFYI, I have rebased this series to 5.6-rc4 here:
>>
>> https://github.com/hisilicon/kernel-dev/tree/private-topic-sas-5.6-resv-commands-v1
>>
>>
>> I am interested in enabling this for libsas and associated HBAs, so
>> there are some patches on top for that.
>>
>> No review comments have been addressed, apart from removing "block:
>> disable elevator for reserved tags"
>>
>> Please let me know your plan for this series.
>>
> The plan is to wait for the shared tagset patchset to go in first :-)
> 

I think that one should go last. Or, to be more specific, switching the 
drivers to expose multiple queues - which is part of that series - 
should go last.

> Without it we'll be running on one tag bitmap per queue, and we risk
> duplicate tags when sending down reserved commands :-(

Until we expose multiple queues, we still have a single HW queue from 
blk-mq perspective, so will only have a single tagset per HBA, so no 
chance of duplicate tags.

This series should be able to go in without dependencies.

Thanks,
John

Re: [PATCH RFC 00/24] scsi: enable reserved commands for LLDDs

[John Garry]

On 09/03/2020 14:21, John Garry wrote:
> On 09/03/2020 13:57, Hannes Reinecke wrote:
>> On 3/9/20 1:48 PM, John Garry wrote:
>>> On 23/08/2019 14:26, John Garry wrote:
>>>>>
>>>>> quite some drivers use internal commands for various purposes, most
>>>>> commonly sending TMFs or querying the HBA status.
>>>>> While these commands use the same submission mechanism than normal
>>>>> I/O commands, they will not be counted as outstanding commands,
>>>>> requiring those drivers to implement their own mechanism to figure
>>>>> out outstanding commands.
>>>>> This patchset enables the use of reserved tags for the SCSI midlayer,
>>>>> enabling LLDDs to rely on the block layer for tracking outstanding
>>>>> commands.
>>>>> More importantly, it allows LLDD to request a valid tag from the block
>>>>> layer without having to implement some tracking mechanism within the
>>>>> driver. This removes quite some hacks which were required for some
>>>>> drivers (eg. fnic or snic).
>>>>>
>>>>> As usual, comments and reviews are welcome.
>>>>>
>>>>
>>> Hi Hannes,
>>>
>>> JFYI, I have rebased this series to 5.6-rc4 here:
>>>
>>> https://github.com/hisilicon/kernel-dev/tree/private-topic-sas-5.6-resv-commands-v1 
>>>
>>>
>>>
>>> I am interested in enabling this for libsas and associated HBAs, so
>>> there are some patches on top for that.
>>>
>>> No review comments have been addressed, apart from removing "block:
>>> disable elevator for reserved tags"
>>>
>>> Please let me know your plan for this series.
>>>
>> The plan is to wait for the shared tagset patchset to go in first :-)
>>
> 
> I think that one should go last. Or, to be more specific, switching the 
> drivers to expose multiple queues - which is part of that series - 
> should go last.
> 
>> Without it we'll be running on one tag bitmap per queue, and we risk
>> duplicate tags when sending down reserved commands :-(
> 
> Until we expose multiple queues, we still have a single HW queue from 
> blk-mq perspective, so will only have a single tagset per HBA, so no 
> chance of duplicate tags.
> 
> This series should be able to go in without dependencies.

BTW, for the other SCSI HBA drivers which use libsas, it would be better 
if they could stop managing their own tags and switch to use the request 
tag. That would mean that we need to reserve some tags, which is 
currently not done in those drivers (pm8xxx, anyway).

If I were to make this change, are there mvsas and aic9xxx cards around 
to test? I have a pm8xxx card.

Thanks,
John