On 25/10/2019 10:43, Ming Lei wrote:
On Fri, Oct 25, 2019 at 09:58:16AM +0100, John Garry wrote:
In scsi_host.h, we have for scsi_host_template.can_queue: "It is set to the
maximum number of simultaneous commands a given host adapter will accept.",
so that should be honoured.
Hi Ming,
That words should have been changed to:
"It is set to the maximum number of simultaneous commands a given host adapter's
hw queue will accept."
Hi Ming,
I find this definition misleading. As you know, some MQ SAS HBAs can accept
.can_queue commands on a given hw queue, but can still only accept
.can_queue commands over all hw queues.
I don't know there are such MQ HBA driver in tree,
HiSilicon SAS HBA can accept 4096 commands on any given hw queue but can
also only accept 4096 commands over all queues simultaneously. In fact,
the hw queue depth is configurable.
That's why I think that the definition is misleading.
I think I sound like a broken record now :)
at least that is the
current blk-mq/scsi-mq model: each hw queue has its own independent
tags, so there can't be the limit for MQ HBA, which should allow to
accept (.can_queue * nr_hw_queues) commands. And I did hear people
complains bad performance caused by the atomic .host_busy counter.
ok, seems reasonable for now.
If you are talking about the current SQ(from blk-mq or scsi-mq viewpoint) HBA
which has multiple reply queue(HPSA, hisilicon SAS, mpt3sas, and megaraid_sas),
they are just the special type. According to scsi-mq's model, they should
belong to SQ HBA.
And Scsi_host.nr_hw_queues: "it is assumed that each hardware queue has a
queue depth of can_queue. In other words, the total queue depth per host is
nr_hw_queues * can_queue."
The above is correct.
I don't read "total queue depth per host" same as "maximum number of
simultaneous commands a given host adapter will accept". If anything, the
nr_hw_queues comment is ambiguous.
The point is simple, because each hw queue has its own independent tags,
that is why I mentioned your Hisilicon SAS can't be converted to MQ
easily cause this hardware has only single shared tags.
Please be aware that HiSilicon SAS HW would not be unique for SCSI HBAs in
this regard, in that the unique hostwide tag is not just for HBA HW IO
management, but also is used as the tag for SCSI TMFs.
Right.
Just checking mpt3sas seems similar:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/mpt3sas/mpt3sas_scsih.c#n2918
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/mpt3sas/mpt3sas_base.c#n3546
Not only mpt3sas, there are also HPSA and more. And these drivers have to
support single hw queue of blk-mq, instead of real MQ. And the reason is that
these HBA has single tags.
We should be able to do better than that.
For a start, at least doesn't the check you remove in scsi_host_is_busy()
limit commands the HBA accepts to .can_queue?
As I mentioned above, that is current blk-mq/scsi-mq's model, each hw
queue has its own independent tags, so the check really doesn't make
sense. >
And if you make the change in this patch, then the changes to improve blk-mq
for CPU hotplug are pointless, as we can't change the SAS HBAs to expose
multiple queues.
No, just the small number of special type SCSI HBAs with multiple reply queue
and single tags can't benefit from the patchset of 'improve blk-mq for CPU hotplug',
and all other normal MQ device/drivers do get improved wrt. CPU hotplug.
TBH, I'm not sure on the group of SCSI drivers which could benefit then.
As I see, only qla2xxx driver sets Scsi_host.nr_hw_queues and also uses
pci_alloc_irq_vectors_affinity().
We have tried hosttags approach for the several drivers, but looks it is
too messy. Given there are only 3 or 4 such device, we still can improve
them via driver private approach in future if no generic way is doable.
ok, I'd rather not go on this path - you may say I'm digging my head in
the sand. Anyway, I'll continue to support the 'improve blk-mq for CPU
hotplug' effort.
Thanks,
John
Thanks,
Ming
.
