Re: [PATCH v3 27/57] scsi: hisi_sas: Enable lock context analysis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling synchronize_irq() inside an irqsave spinlock critical section causes a 'scheduling while atomic' bug.
--

--- Patch [27]: [PATCH v3 27/57] scsi: hisi_sas: Enable lock context analysis ---
commit 289c0dd12dd00005d6efe11e9c29f7fd715a2464
Author: Bart Van Assche <bvanassche@acm.org>

scsi: hisi_sas: Enable lock context analysis

This commit enables lock context analysis for the hisi_sas driver via the
Makefile.

> diff --git a/drivers/scsi/hisi_sas/Makefile b/drivers/scsi/hisi_sas/Makefile
> index 742e732cd51d5..8576c78d4afe3 100644
> --- a/drivers/scsi/hisi_sas/Makefile
> +++ b/drivers/scsi/hisi_sas/Makefile
> @@ -1,4 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +
> +CONTEXT_ANALYSIS := y

[Severity: High]
This is a pre-existing issue, but enabling context analysis here might
immediately trigger warnings or panics due to a scheduling while atomic bug.

When hardware timeouts or aborts trigger SCSI error handling, and the
task is already complete, hisi_sas_abort_task() calls hisi_sas_sync_cq()
while holding a spinlock with interrupts disabled:

drivers/scsi/hisi_sas/hisi_sas_main.c:hisi_sas_abort_task() {
    ...
    spin_lock_irqsave(&task->task_state_lock, flags);
    if (task->task_state_flags & SAS_TASK_STATE_DONE) {
        struct hisi_sas_cq *cq;

        if (slot) {
            cq = &hisi_hba->cq[slot->dlvry_queue];
            hisi_sas_sync_cq(cq);
        }
        spin_unlock_irqrestore(&task->task_state_lock, flags);
    ...
}

The hisi_sas_sync_cq() wrapper then directly calls synchronize_irq() for
non-poll queues:

drivers/scsi/hisi_sas/hisi_sas_main.c:hisi_sas_sync_cq() {
    if (hisi_sas_queue_is_poll(cq))
        hisi_sas_sync_poll_cq(cq);
    else
        synchronize_irq(cq->irq_no);
}

Will this cause a panic or deadlock when synchronize_irq() calls
wait_event() and attempts to sleep while the CPU is holding the
task_state_lock spinlock?

> +
>  obj-$(CONFIG_SCSI_HISI_SAS)		+= hisi_sas_main.o
>  obj-$(CONFIG_SCSI_HISI_SAS)		+= hisi_sas_v1_hw.o hisi_sas_v2_hw.o
>  obj-$(CONFIG_SCSI_HISI_SAS_PCI)		+= hisi_sas_v3_hw.o

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785451174.git.bvanassche@acm.org?part=27