Re: [PATCH V2 5/9] scsi: hisi: take blk_mq_max_nr_hw_queues() into account for calculating io vectors

[John Garry <john.g.garry@oracle.com>]

On 27/07/2023 02:15, Ming Lei wrote:
>>> hisi_sas_v3_hw.c
>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
>>> @@ -2550,6 +2550,9 @@ static int interrupt_preinit_v3_hw(struct hisi_hba *hisi_hba)
>>>    	hisi_hba->cq_nvecs = vectors - BASE_VECTORS_V3_HW - hisi_hba->iopoll_q_cnt;
>>> +	if (hisi_hba->cq_nvecs > scsi_max_nr_hw_queues())
>>> +		hisi_hba->cq_nvecs = scsi_max_nr_hw_queues();
>>> +
>>>    	shost->nr_hw_queues = hisi_hba->cq_nvecs + hisi_hba->iopoll_q_cnt;
>> For other drivers you limit the max MSI vectors which we try to allocate
>> according to scsi_max_nr_hw_queues(), but here you continue to alloc the
>> same max vectors but then limit the driver's completion queue count. Why not
>> limit the max MSI vectors also here?

Ah, checking again, I think that this driver always allocates maximum 
possible MSI due to arm interrupt controller driver bug - see comment at 
top of function interrupt_preinit_v3_hw(). IIRC, there was a problem if 
we remove and re-insert the device driver that the GIC ITS fails to 
allocate MSI unless all MSI were previously allocated.

Xiang Chen can confirm.

> Good catch!
> 
> I guess it is because of the following line:
> 
>     	hisi_hba->cq_nvecs = vectors - BASE_VECTORS_V3_HW - hisi_hba->iopoll_q_cnt;
> 
> I am a bit confused why hisi_sas_v3 takes ->iopoll_q_cnt into account
> allocated msi vectors?  ->iopoll_q_cnt supposes to not consume msi
> vectors, so I think we need the following fix first:
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index 20e1607c6282..032c13ce8373 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> @@ -2549,7 +2549,7 @@ static int interrupt_preinit_v3_hw(struct hisi_hba *hisi_hba)
>                  return -ENOENT;
> 
> 
> -       hisi_hba->cq_nvecs = vectors - BASE_VECTORS_V3_HW - hisi_hba->iopoll_q_cnt;
> +       hisi_hba->cq_nvecs = vectors - BASE_VECTORS_V3_HW;
>          shost->nr_hw_queues = hisi_hba->cq_nvecs + hisi_hba->iopoll_q_cnt;

Thanks,
John