-----Original Message-----
From: James Bottomley <jejb@xxxxxxxxxxxxx>
Sent: Tuesday, April 18, 2023 10:13 AM
To: Sagar Biradar - C34249 <Sagar.Biradar@xxxxxxxxxxxxx>; john.g.garry@xxxxxxxxxx; Don Brace - C33706 <Don.Brace@xxxxxxxxxxxxx>; Gilbert Wu - C33504 <Gilbert.Wu@xxxxxxxxxxxxx>; linux-scsi@xxxxxxxxxxxxxxx; martin.petersen@xxxxxxxxxx; brking@xxxxxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx; Tom White - C33503 <Tom.White@xxxxxxxxxxxxx>
Subject: Re: [PATCH] aacraid: reply queue mapping to CPUs based of IRQ affinity
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
[I'm with Jon: your email style makes digging information out of the emails very hard, which is why I only quote this section] On Mon, 2023-04-10 at 21:17 +0000, Sagar.Biradar@xxxxxxxxxxxxx wrote:
> ***blk-mq already does what you want here, including handling for the
> case I mention above. It maintains a CPU -> HW queue mapping, and
> using a reply map in the LLD is the old way of doing this.
>
> We also tried implementing the blk-mq mechanism in the driver and we
> saw command timeouts.
> The firmware has limitation of fixed number of queues per vector and
> the blk-mq changes would saturate that limit.
> That answers the possible command timeout.
Could we have more details on this, please? The problem is that this is a very fragile area of the kernel, so you rolling your own special snowflake implementation in the driver is going to be an ongoing maintenance problem (and the fact that you need this at all indicates you have long tail customers who will be around for a while yet). If the only issue is limiting the number of queues per vector, we can look at getting the block layer to do that. Although I was under the impression that you can do it yourself with the ->map_queues() callback. Can you say a bit about why this didn't work?
[Sagar Biradar]
Thank you for your response.
We did venture trying into something like what you pointed to.
We mapped the hardware queues, and we still see command timeout. This change doesn’t work for us at this stage.
Also, we observed that the load is not balanced across all the CPUs.
I am pasting the code snippets for better understanding.
During the probe, we assigned the hardware queues.
shost->nr_hw_queues = shost->can_queue; //inside aac_probe_one().
We also wrote a new routine "blk_mq_pci_map_queues" (mapped to .map_queues in scsi_host_template).
static void aac_map_queues(struct Scsi_Host *shost)
{
struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
blk_mq_pci_map_queues(&shost->tag_set.map[HCTX_TYPE_DEFAULT],
aac->pdev, 0);
}
With the above changes, we see command timeouts in the firmware space and the commands never return to the driver.
This may need some changes in the firmware, but the firmware changes are restricted (since this is EOL product).
Also, we saw that the load was entirely upon one CPU and it was not balanced across other CPUs.
We have had this reply_queue mechanism (patch) in our Out Of Box driver (OOB) for more than three years.
We(vendors/customers included) have not observed any issues.
James
