| Message ID | 1647945585-197349-1-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | blk-mq/libata/scsi: SCSI driver tagging improvements | expand |
On 3/22/22 11:39, John Garry wrote: > Currently SCSI low-level drivers are required to manage tags for commands > which do not come via the block layer - libata internal commands would be > an example of one of these. > > There was some work to provide "reserved commands" support in such series > as https://lore.kernel.org/linux-scsi/20211125151048.103910-1-hare@suse.de/ > > This was based on allocating a request for the lifetime of the "internal" > command. > > This series tries to solve that problem by not just allocating the request > but also sending it through the block layer, that being the normal flow > for a request. We need to do this as we may only poll completion of > requests through the block layer, so would need to do this for poll queue > support. > > There is still scope to allocate commands just to get a tag as token as > that may suit some other scenarios, but it's not what we do here. > > This series extends blk-mq to support a request queue having a custom set > of ops. In addition SCSI core code adds support for these type of requests. > > This series does not include SCSI core handling for enabling reserved > tags per tagset, but that would be easy to add. > > Based on mkp-scsi 5.18/scsi-staging @ 66daf3e6b993 > > Please consider as an RFC for now. I think that the libata change has the > largest scope for improvement... > Grand seeing that someone is taking it up. Thanks for doing this! But: Allocating a queue for every request (eg in patch 3) is overkill. If we want to go that route we should be allocating the queue upfront (eg when creating the device itself), and then just referencing it. However, can't say I like this approach. I've been playing around with supporting internal commands, and really found two constraints really annoying: - The tagset supports only _one_ set of payload via blk_mq_rq_(to,from)_pdu(). This requires each request to be of the same type, and with that making it really hard for re-purposing the request for internal usage. In the end I settled by just keeping it and skipping the SCSI command field. If we could have a distinct PDU type for internal commands I guess things would be easier. - The number of reserved commands is static. With that it's getting really hard using reserved commands with low-queue depth devices like ATA; we only have 31 commands to work with, and setting one or two aside for TMF is really making a difference performance wise. It would be _awesome_ if we could allocate reserved commands dynamically (ie just marking a command as 'reserved' once allocated). Sure, it won't have the same guarantees as 'real' reserved commands, but in most cases we don't actually need that. Maybe these are some lines we could investigate? Hmm? Cheers, Hannes
On 22/03/2022 11:30, Hannes Reinecke wrote: > On 3/22/22 11:39, John Garry wrote: >> Currently SCSI low-level drivers are required to manage tags for commands >> which do not come via the block layer - libata internal commands would be >> an example of one of these. >> >> There was some work to provide "reserved commands" support in such series >> as >> https://lore.kernel.org/linux-scsi/20211125151048.103910-1-hare@suse.de/ >> >> This was based on allocating a request for the lifetime of the "internal" >> command. >> >> This series tries to solve that problem by not just allocating the >> request >> but also sending it through the block layer, that being the normal flow >> for a request. We need to do this as we may only poll completion of >> requests through the block layer, so would need to do this for poll queue >> support. >> >> There is still scope to allocate commands just to get a tag as token as >> that may suit some other scenarios, but it's not what we do here. >> >> This series extends blk-mq to support a request queue having a custom set >> of ops. In addition SCSI core code adds support for these type of >> requests. >> >> This series does not include SCSI core handling for enabling reserved >> tags per tagset, but that would be easy to add. >> >> Based on mkp-scsi 5.18/scsi-staging @ 66daf3e6b993 >> >> Please consider as an RFC for now. I think that the libata change has the >> largest scope for improvement... >> > > Grand seeing that someone is taking it up. Thanks for doing this! > > But: > Allocating a queue for every request (eg in patch 3) is overkill. If we > want to go that route we should be allocating the queue upfront (eg when > creating the device itself), and then just referencing it. For patch #3 I needed to allocate a separate request queue as the scsi device is not created by that stage. And then for other scenarios in which we allocate the separate request queue, since the scheme here is to allocate a request queue with different ops, we can't use the same scsi_device request queue. note: As for allocating request queues for the lifetime of the host, we need to remember that blk-mq fairly reserves a tag budget per request queue, and it would be a waste to keep a budget just for these internal commands. So that is why I only keep the request queues temporarily. > > However, can't say I like this approach. I've been playing around with > supporting internal commands, and really found two constraints really > annoying: > > - The tagset supports only _one_ set of payload via > blk_mq_rq_(to,from)_pdu(). > This requires each request to be of the same type, and with that making > it really hard for re-purposing the request for internal usage. In the > end I settled by just keeping it and skipping the SCSI command field. That sounds reasonable. For this series I am just fixing up libsas, and libsas has a sas_task per command already, so we can just allocate the sas_task separately and use the host_scribble to point to the sas_task. > If we could have a distinct PDU type for internal commands I guess > things would be easier. Other drivers can do something similar to above or use the scsi priv data. > > - The number of reserved commands is static. > With that it's getting really hard using reserved commands with > low-queue depth devices like ATA; we only have 31 commands to work with, > and setting one or two aside for TMF is really making a difference > performance wise. It would be _awesome_ if we could allocate reserved > commands dynamically (ie just marking a command as 'reserved' once > allocated). I see. So you want to allocate a request, mark as "internal", and then we have a flag which can be used to decide which path we need to send it on. eh, maybe scsi_cmnd.submitter could be used > Sure, it won't have the same guarantees as 'real' reserved commands, but > in most cases we don't actually need that. > > Maybe these are some lines we could investigate? > Hmm? > Thanks, John
Currently SCSI low-level drivers are required to manage tags for commands which do not come via the block layer - libata internal commands would be an example of one of these. There was some work to provide "reserved commands" support in such series as https://lore.kernel.org/linux-scsi/20211125151048.103910-1-hare@suse.de/ This was based on allocating a request for the lifetime of the "internal" command. This series tries to solve that problem by not just allocating the request but also sending it through the block layer, that being the normal flow for a request. We need to do this as we may only poll completion of requests through the block layer, so would need to do this for poll queue support. There is still scope to allocate commands just to get a tag as token as that may suit some other scenarios, but it's not what we do here. This series extends blk-mq to support a request queue having a custom set of ops. In addition SCSI core code adds support for these type of requests. This series does not include SCSI core handling for enabling reserved tags per tagset, but that would be easy to add. Based on mkp-scsi 5.18/scsi-staging @ 66daf3e6b993 Please consider as an RFC for now. I think that the libata change has the largest scope for improvement... John Garry (11): blk-mq: Add blk_mq_init_queue_ops() scsi: core: Add SUBMITTED_BY_SCSI_CUSTOM_OPS libata: Send internal commands through the block layer scsi: libsas: Send SMP commands through the block layer scsi: libsas: Send TMF commands through the block layer scsi: core: Add scsi_alloc_request_hwq() scsi: libsas: Send internal abort commands through the block layer scsi: libsas: Change ATA support to deal with each qc having a SCSI command scsi: libsas: Add sas_task_to_unique_tag() scsi: libsas: Add sas_task_to_hwq() scsi: hisi_sas: Remove private tag management block/blk-mq.c | 23 +++- drivers/ata/libata-core.c | 121 +++++++++++++------ drivers/md/dm-rq.c | 2 +- drivers/scsi/hisi_sas/hisi_sas_main.c | 66 +---------- drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 3 +- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +- drivers/scsi/libsas/sas_ata.c | 11 +- drivers/scsi/libsas/sas_expander.c | 38 ++++-- drivers/scsi/libsas/sas_internal.h | 1 + drivers/scsi/libsas/sas_scsi_host.c | 153 ++++++++++++++++++++----- drivers/scsi/scsi_lib.c | 14 +++ include/linux/blk-mq.h | 5 +- include/scsi/libsas.h | 4 +- include/scsi/scsi_cmnd.h | 4 + 14 files changed, 298 insertions(+), 150 deletions(-)