* [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors
@ 2021-04-12 16:57 Roman Bolshakov
2021-04-13 7:35 ` Daniel Wagner
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Roman Bolshakov @ 2021-04-12 16:57 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi
Cc: linux, GR-QLogic-Storage-Upstream, Roman Bolshakov,
Daniel Wagner, Himanshu Madhani, Quinn Tran, Nilesh Javali,
stable, Aleksandr Volkov, Aleksandr Miloserdov
Commit a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number
of CPUs") lowers the number of allocated MSI-X vectors to the number of
CPUs.
That breaks vector allocation assumptions in qla83xx_iospace_config(),
qla24xx_enable_msix() and qla2x00_iospace_config(). Either of the
functions computes maximum number of qpairs as:
ha->max_qpairs = ha->msix_count - 1 (MB interrupt) - 1 (default
response queue) - 1 (ATIO, in dual or pure target mode)
max_qpairs is set to zero in case of two CPUs and initiator mode. The
number is then used to allocate ha->queue_pair_map inside
qla2x00_alloc_queues(). No allocation happens and ha->queue_pair_map is
left NULL but the driver thinks there are queue pairs available.
qla2xxx_queuecommand() tries to find a qpair in the map and crashes:
if (ha->mqenable) {
uint32_t tag;
uint16_t hwq;
struct qla_qpair *qpair = NULL;
tag = blk_mq_unique_tag(cmd->request);
hwq = blk_mq_unique_tag_to_hwq(tag);
qpair = ha->queue_pair_map[hwq]; # <- HERE
if (qpair)
return qla2xxx_mqueuecommand(host, cmd, qpair);
}
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 0 PID: 72 Comm: kworker/u4:3 Tainted: G W 5.10.0-rc1+ #25
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014
Workqueue: scsi_wq_7 fc_scsi_scan_rport [scsi_transport_fc]
RIP: 0010:qla2xxx_queuecommand+0x16b/0x3f0 [qla2xxx]
Call Trace:
scsi_queue_rq+0x58c/0xa60
blk_mq_dispatch_rq_list+0x2b7/0x6f0
? __sbitmap_get_word+0x2a/0x80
__blk_mq_sched_dispatch_requests+0xb8/0x170
blk_mq_sched_dispatch_requests+0x2b/0x50
__blk_mq_run_hw_queue+0x49/0xb0
__blk_mq_delay_run_hw_queue+0xfb/0x150
blk_mq_sched_insert_request+0xbe/0x110
blk_execute_rq+0x45/0x70
__scsi_execute+0x10e/0x250
scsi_probe_and_add_lun+0x228/0xda0
__scsi_scan_target+0xf4/0x620
? __pm_runtime_resume+0x4f/0x70
scsi_scan_target+0x100/0x110
fc_scsi_scan_rport+0xa1/0xb0 [scsi_transport_fc]
process_one_work+0x1ea/0x3b0
worker_thread+0x28/0x3b0
? process_one_work+0x3b0/0x3b0
kthread+0x112/0x130
? kthread_park+0x80/0x80
ret_from_fork+0x22/0x30
The driver should allocate enough vectors to provide every CPU it's own HW
queue and still handle reserved (MB, RSP, ATIO) interrupts.
The change fixes the crash on dual core VM and prevents unbalanced QP
allocation where nr_hw_queues is two less than the number of CPUs.
Cc: Daniel Wagner <daniel.wagner@suse.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: stable@vger.kernel.org # 5.11+
Fixes: a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number of CPUs")
Reported-by: Aleksandr Volkov <a.y.volkov@yadro.com>
Reported-by: Aleksandr Miloserdov <a.miloserdov@yadro.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
drivers/scsi/qla2xxx/qla_isr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 11d6e0db07fe..6e8f737a4af3 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -3998,11 +3998,11 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
if (USER_CTRL_IRQ(ha) || !ha->mqiobase) {
/* user wants to control IRQ setting for target mode */
ret = pci_alloc_irq_vectors(ha->pdev, min_vecs,
- min((u16)ha->msix_count, (u16)num_online_cpus()),
+ min((u16)ha->msix_count, (u16)(num_online_cpus() + min_vecs)),
PCI_IRQ_MSIX);
} else
ret = pci_alloc_irq_vectors_affinity(ha->pdev, min_vecs,
- min((u16)ha->msix_count, (u16)num_online_cpus()),
+ min((u16)ha->msix_count, (u16)(num_online_cpus() + min_vecs)),
PCI_IRQ_MSIX | PCI_IRQ_AFFINITY,
&desc);
--
2.30.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors
2021-04-12 16:57 [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors Roman Bolshakov
@ 2021-04-13 7:35 ` Daniel Wagner
2021-04-13 15:48 ` Himanshu Madhani
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Daniel Wagner @ 2021-04-13 7:35 UTC (permalink / raw)
To: Roman Bolshakov
Cc: Martin K . Petersen, linux-scsi, linux,
GR-QLogic-Storage-Upstream, Daniel Wagner, Himanshu Madhani,
Quinn Tran, Nilesh Javali, stable, Aleksandr Volkov,
Aleksandr Miloserdov
On Mon, Apr 12, 2021 at 07:57:40PM +0300, Roman Bolshakov wrote:
> Commit a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number
> of CPUs") lowers the number of allocated MSI-X vectors to the number of
> CPUs.
>
> That breaks vector allocation assumptions in qla83xx_iospace_config(),
> qla24xx_enable_msix() and qla2x00_iospace_config(). Either of the
> functions computes maximum number of qpairs as:
>
> ha->max_qpairs = ha->msix_count - 1 (MB interrupt) - 1 (default
> response queue) - 1 (ATIO, in dual or pure target mode)
>
> max_qpairs is set to zero in case of two CPUs and initiator mode. The
> number is then used to allocate ha->queue_pair_map inside
> qla2x00_alloc_queues(). No allocation happens and ha->queue_pair_map is
> left NULL but the driver thinks there are queue pairs available.
>
> qla2xxx_queuecommand() tries to find a qpair in the map and crashes:
>
> if (ha->mqenable) {
> uint32_t tag;
> uint16_t hwq;
> struct qla_qpair *qpair = NULL;
>
> tag = blk_mq_unique_tag(cmd->request);
> hwq = blk_mq_unique_tag_to_hwq(tag);
> qpair = ha->queue_pair_map[hwq]; # <- HERE
>
> if (qpair)
> return qla2xxx_mqueuecommand(host, cmd, qpair);
> }
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: 0000 [#1] SMP PTI
> CPU: 0 PID: 72 Comm: kworker/u4:3 Tainted: G W 5.10.0-rc1+ #25
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014
> Workqueue: scsi_wq_7 fc_scsi_scan_rport [scsi_transport_fc]
> RIP: 0010:qla2xxx_queuecommand+0x16b/0x3f0 [qla2xxx]
> Call Trace:
> scsi_queue_rq+0x58c/0xa60
> blk_mq_dispatch_rq_list+0x2b7/0x6f0
> ? __sbitmap_get_word+0x2a/0x80
> __blk_mq_sched_dispatch_requests+0xb8/0x170
> blk_mq_sched_dispatch_requests+0x2b/0x50
> __blk_mq_run_hw_queue+0x49/0xb0
> __blk_mq_delay_run_hw_queue+0xfb/0x150
> blk_mq_sched_insert_request+0xbe/0x110
> blk_execute_rq+0x45/0x70
> __scsi_execute+0x10e/0x250
> scsi_probe_and_add_lun+0x228/0xda0
> __scsi_scan_target+0xf4/0x620
> ? __pm_runtime_resume+0x4f/0x70
> scsi_scan_target+0x100/0x110
> fc_scsi_scan_rport+0xa1/0xb0 [scsi_transport_fc]
> process_one_work+0x1ea/0x3b0
> worker_thread+0x28/0x3b0
> ? process_one_work+0x3b0/0x3b0
> kthread+0x112/0x130
> ? kthread_park+0x80/0x80
> ret_from_fork+0x22/0x30
>
> The driver should allocate enough vectors to provide every CPU it's own HW
> queue and still handle reserved (MB, RSP, ATIO) interrupts.
>
> The change fixes the crash on dual core VM and prevents unbalanced QP
> allocation where nr_hw_queues is two less than the number of CPUs.
>
> Cc: Daniel Wagner <daniel.wagner@suse.com>
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Nilesh Javali <njavali@marvell.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: stable@vger.kernel.org # 5.11+
> Fixes: a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number of CPUs")
> Reported-by: Aleksandr Volkov <a.y.volkov@yadro.com>
> Reported-by: Aleksandr Miloserdov <a.miloserdov@yadro.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Make sense to limit the _max_ numbers of requested IRQs to
num_online_cpus() + min_vecs.
Reviewed-by: Daniel Wagner <dwagner@suse.de>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors
2021-04-12 16:57 [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors Roman Bolshakov
2021-04-13 7:35 ` Daniel Wagner
@ 2021-04-13 15:48 ` Himanshu Madhani
2021-04-16 2:06 ` Martin K. Petersen
2021-04-20 2:29 ` Martin K. Petersen
3 siblings, 0 replies; 5+ messages in thread
From: Himanshu Madhani @ 2021-04-13 15:48 UTC (permalink / raw)
To: Roman Bolshakov
Cc: Martin Petersen, linux-scsi, linux, GR-QLogic-Storage-Upstream,
Daniel Wagner, Quinn Tran, Nilesh Javali, stable,
Aleksandr Volkov, Aleksandr Miloserdov
> On Apr 12, 2021, at 11:57 AM, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
>
> Commit a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number
> of CPUs") lowers the number of allocated MSI-X vectors to the number of
> CPUs.
>
> That breaks vector allocation assumptions in qla83xx_iospace_config(),
> qla24xx_enable_msix() and qla2x00_iospace_config(). Either of the
> functions computes maximum number of qpairs as:
>
> ha->max_qpairs = ha->msix_count - 1 (MB interrupt) - 1 (default
> response queue) - 1 (ATIO, in dual or pure target mode)
>
> max_qpairs is set to zero in case of two CPUs and initiator mode. The
> number is then used to allocate ha->queue_pair_map inside
> qla2x00_alloc_queues(). No allocation happens and ha->queue_pair_map is
> left NULL but the driver thinks there are queue pairs available.
>
> qla2xxx_queuecommand() tries to find a qpair in the map and crashes:
>
> if (ha->mqenable) {
> uint32_t tag;
> uint16_t hwq;
> struct qla_qpair *qpair = NULL;
>
> tag = blk_mq_unique_tag(cmd->request);
> hwq = blk_mq_unique_tag_to_hwq(tag);
> qpair = ha->queue_pair_map[hwq]; # <- HERE
>
> if (qpair)
> return qla2xxx_mqueuecommand(host, cmd, qpair);
> }
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: 0000 [#1] SMP PTI
> CPU: 0 PID: 72 Comm: kworker/u4:3 Tainted: G W 5.10.0-rc1+ #25
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014
> Workqueue: scsi_wq_7 fc_scsi_scan_rport [scsi_transport_fc]
> RIP: 0010:qla2xxx_queuecommand+0x16b/0x3f0 [qla2xxx]
> Call Trace:
> scsi_queue_rq+0x58c/0xa60
> blk_mq_dispatch_rq_list+0x2b7/0x6f0
> ? __sbitmap_get_word+0x2a/0x80
> __blk_mq_sched_dispatch_requests+0xb8/0x170
> blk_mq_sched_dispatch_requests+0x2b/0x50
> __blk_mq_run_hw_queue+0x49/0xb0
> __blk_mq_delay_run_hw_queue+0xfb/0x150
> blk_mq_sched_insert_request+0xbe/0x110
> blk_execute_rq+0x45/0x70
> __scsi_execute+0x10e/0x250
> scsi_probe_and_add_lun+0x228/0xda0
> __scsi_scan_target+0xf4/0x620
> ? __pm_runtime_resume+0x4f/0x70
> scsi_scan_target+0x100/0x110
> fc_scsi_scan_rport+0xa1/0xb0 [scsi_transport_fc]
> process_one_work+0x1ea/0x3b0
> worker_thread+0x28/0x3b0
> ? process_one_work+0x3b0/0x3b0
> kthread+0x112/0x130
> ? kthread_park+0x80/0x80
> ret_from_fork+0x22/0x30
>
> The driver should allocate enough vectors to provide every CPU it's own HW
> queue and still handle reserved (MB, RSP, ATIO) interrupts.
>
> The change fixes the crash on dual core VM and prevents unbalanced QP
> allocation where nr_hw_queues is two less than the number of CPUs.
>
> Cc: Daniel Wagner <daniel.wagner@suse.com>
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Nilesh Javali <njavali@marvell.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: stable@vger.kernel.org # 5.11+
> Fixes: a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number of CPUs")
> Reported-by: Aleksandr Volkov <a.y.volkov@yadro.com>
> Reported-by: Aleksandr Miloserdov <a.miloserdov@yadro.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> drivers/scsi/qla2xxx/qla_isr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 11d6e0db07fe..6e8f737a4af3 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -3998,11 +3998,11 @@ qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
> if (USER_CTRL_IRQ(ha) || !ha->mqiobase) {
> /* user wants to control IRQ setting for target mode */
> ret = pci_alloc_irq_vectors(ha->pdev, min_vecs,
> - min((u16)ha->msix_count, (u16)num_online_cpus()),
> + min((u16)ha->msix_count, (u16)(num_online_cpus() + min_vecs)),
> PCI_IRQ_MSIX);
> } else
> ret = pci_alloc_irq_vectors_affinity(ha->pdev, min_vecs,
> - min((u16)ha->msix_count, (u16)num_online_cpus()),
> + min((u16)ha->msix_count, (u16)(num_online_cpus() + min_vecs)),
> PCI_IRQ_MSIX | PCI_IRQ_AFFINITY,
> &desc);
>
> --
> 2.30.1
>
Change looks sensible.
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
--
Himanshu Madhani Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors
2021-04-12 16:57 [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors Roman Bolshakov
2021-04-13 7:35 ` Daniel Wagner
2021-04-13 15:48 ` Himanshu Madhani
@ 2021-04-16 2:06 ` Martin K. Petersen
2021-04-20 2:29 ` Martin K. Petersen
3 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2021-04-16 2:06 UTC (permalink / raw)
To: Roman Bolshakov
Cc: Martin K . Petersen, linux-scsi, linux,
GR-QLogic-Storage-Upstream, Daniel Wagner, Himanshu Madhani,
Quinn Tran, Nilesh Javali, stable, Aleksandr Volkov,
Aleksandr Miloserdov
Roman,
> Commit a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number
> of CPUs") lowers the number of allocated MSI-X vectors to the number
> of CPUs.
Applied to 5.13/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors
2021-04-12 16:57 [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors Roman Bolshakov
` (2 preceding siblings ...)
2021-04-16 2:06 ` Martin K. Petersen
@ 2021-04-20 2:29 ` Martin K. Petersen
3 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2021-04-20 2:29 UTC (permalink / raw)
To: Roman Bolshakov, linux-scsi
Cc: Martin K . Petersen, Himanshu Madhani, linux, Aleksandr Volkov,
Daniel Wagner, GR-QLogic-Storage-Upstream, Nilesh Javali, stable,
Quinn Tran, Aleksandr Miloserdov
On Mon, 12 Apr 2021 19:57:40 +0300, Roman Bolshakov wrote:
> Commit a6dcfe08487e ("scsi: qla2xxx: Limit interrupt vectors to number
> of CPUs") lowers the number of allocated MSI-X vectors to the number of
> CPUs.
>
> That breaks vector allocation assumptions in qla83xx_iospace_config(),
> qla24xx_enable_msix() and qla2x00_iospace_config(). Either of the
> functions computes maximum number of qpairs as:
>
> [...]
Applied to 5.13/scsi-queue, thanks!
[1/1] scsi: qla2xxx: Reserve extra IRQ vectors
https://git.kernel.org/mkp/scsi/c/f02d4086a8f3
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-20 2:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 16:57 [PATCH] scsi: qla2xxx: Reserve extra IRQ vectors Roman Bolshakov
2021-04-13 7:35 ` Daniel Wagner
2021-04-13 15:48 ` Himanshu Madhani
2021-04-16 2:06 ` Martin K. Petersen
2021-04-20 2:29 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.