Re: [PATCH] scsi: smartpqi silence a recursive lock warning

Bart Van Assche
2026-04-14T16:38:11Z

On 4/14/26 5:41 AM, Tomas Henzl wrote:
> On systems with multiple controllers debug kernel shows
> WARNING: possible recursive locking detected
> during shutdown.
> Each controller does have its own ctrl_info (and mutex)
> and that isn't correctly recognized by debug kernel.
> Supress the warning by releasing the mutex at the end of pqi_shutdown.
>
> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
> ---
> drivers/scsi/smartpqi/smartpqi_init.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index b4ed991976d0..2026ac645d6a 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -9427,6 +9427,7 @@ static void pqi_shutdown(struct pci_dev *pci_dev)
>
> pqi_crash_if_pending_command(ctrl_info);
> pqi_reset(ctrl_info);
> + pqi_ctrl_unblock_device_reset(ctrl_info);
> }
>
> static void pqi_process_lockup_action_param(void)

This patch looks fine to me but the description not. I think this patch
fixes a real bug rather than only suppressing a lockdep complaint.

Additionally, all uses of mutexes in the entire driver probably should
be reviewed. Here are other questionable constructs in this driver I
know of:
- pqi_ofa_memory_alloc_worker() locks &ctrl_info->ofa_mutex but doesn't
unlock it. This mutex probably gets unlocked from the context of
another thread, something that is not allowed.
- There is code in this driver that calls
mutex_is_locked(&ctrl_info->ofa_mutex) (pqi_ofa_in_progress()) and
next calls mutex_unlock(&ctrl_info->ofa_mutex) without checking
whether the mutex_lock() call happened by the same thread that calls
mutex_unlock().

All the __acquire() and __release() statements in the patch below should
be reviewed.

Thanks,

Bart.

diff --git a/drivers/scsi/smartpqi/Makefile b/drivers/scsi/smartpqi/Makefile
index 28985e508b5c..71db5cd96284 100644
--- a/drivers/scsi/smartpqi/Makefile
+++ b/drivers/scsi/smartpqi/Makefile
@@ -1,3 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
+
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_SCSI_SMARTPQI) += smartpqi.o
smartpqi-objs := smartpqi_init.o smartpqi_sis.o smartpqi_sas_transport.o
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index b4ed991976d0..f99eef39ede4 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -306,12 +306,14 @@ static inline void
pqi_save_fw_triage_setting(struct pqi_ctrl_info *ctrl_info, b
}

static inline void pqi_ctrl_block_scan(struct pqi_ctrl_info *ctrl_info)
+ __acquires(ctrl_info->scan_mutex)
{
ctrl_info->scan_blocked = true;
mutex_lock(&ctrl_info->scan_mutex);
}

static inline void pqi_ctrl_unblock_scan(struct pqi_ctrl_info *ctrl_info)
+ __releases(ctrl_info->scan_mutex)
{
ctrl_info->scan_blocked = false;
mutex_unlock(&ctrl_info->scan_mutex);
@@ -323,11 +325,13 @@ static inline bool pqi_ctrl_scan_blocked(struct
pqi_ctrl_info *ctrl_info)
}

static inline void pqi_ctrl_block_device_reset(struct pqi_ctrl_info
*ctrl_info)
+ __acquires(ctrl_info->lun_reset_mutex)
{
mutex_lock(&ctrl_info->lun_reset_mutex);
}

static inline void pqi_ctrl_unblock_device_reset(struct pqi_ctrl_info
*ctrl_info)
+ __releases(ctrl_info->lun_reset_mutex)
{
mutex_unlock(&ctrl_info->lun_reset_mutex);
}
@@ -430,11 +434,13 @@ static inline bool pqi_device_offline(struct
pqi_scsi_dev *device)
}

static inline void pqi_ctrl_ofa_start(struct pqi_ctrl_info *ctrl_info)
+ __acquires(ctrl_info->ofa_mutex)
{
mutex_lock(&ctrl_info->ofa_mutex);
}

static inline void pqi_ctrl_ofa_done(struct pqi_ctrl_info *ctrl_info)
+ __releases(ctrl_info->ofa_mutex)
{
mutex_unlock(&ctrl_info->ofa_mutex);
}
@@ -2299,6 +2305,7 @@ static void pqi_update_device_list(struct
pqi_ctrl_info *ctrl_info,
* requests before removal.
*/
if (pqi_ofa_in_progress(ctrl_info)) {
+ __acquire(&ctrl_info->lun_reset_mutex);
list_for_each_entry_safe(device, next, &delete_list, delete_list_entry)
if (pqi_is_device_added(device))
pqi_device_remove_start(device);
@@ -3661,6 +3668,8 @@ static void pqi_process_soft_reset(struct
pqi_ctrl_info *ctrl_info)
pqi_save_ctrl_mode(ctrl_info, SIS_MODE);
rc = pqi_ofa_ctrl_restart(ctrl_info, delay_secs);
pqi_host_free_buffer(ctrl_info, &ctrl_info->ofa_memory);
+ /* What guarantees that &ctrl_info->ofa_mutex is held here? */
+ __acquire(&ctrl_info->ofa_mutex);
pqi_ctrl_ofa_done(ctrl_info);
dev_info(&ctrl_info->pci_dev->dev,
"Online Firmware Activation: %s\n",
@@ -3672,6 +3681,8 @@ static void pqi_process_soft_reset(struct
pqi_ctrl_info *ctrl_info)
if (ctrl_info->soft_reset_handshake_supported)
pqi_clear_soft_reset_status(ctrl_info);
pqi_host_free_buffer(ctrl_info, &ctrl_info->ofa_memory);
+ /* What guarantees that &ctrl_info->ofa_mutex is held here? */
+ __acquire(&ctrl_info->ofa_mutex);
pqi_ctrl_ofa_done(ctrl_info);
pqi_ofa_ctrl_unquiesce(ctrl_info);
break;
@@ -3682,6 +3693,8 @@ static void pqi_process_soft_reset(struct
pqi_ctrl_info *ctrl_info)
"unexpected Online Firmware Activation reset status: 0x%x\n",
reset_status);
pqi_host_free_buffer(ctrl_info, &ctrl_info->ofa_memory);
+ /* What guarantees that &ctrl_info->ofa_mutex is held here? */
+ __acquire(&ctrl_info->ofa_mutex);
pqi_ctrl_ofa_done(ctrl_info);
pqi_ofa_ctrl_unquiesce(ctrl_info);
pqi_take_ctrl_offline(ctrl_info, PQI_OFA_RESPONSE_TIMEOUT);
@@ -3698,6 +3711,8 @@ static void pqi_ofa_memory_alloc_worker(struct
work_struct *work)
pqi_ctrl_ofa_start(ctrl_info);
pqi_host_setup_buffer(ctrl_info, &ctrl_info->ofa_memory,
ctrl_info->ofa_bytes_requested, ctrl_info->ofa_bytes_requested);
pqi_host_memory_update(ctrl_info, &ctrl_info->ofa_memory,
PQI_VENDOR_GENERAL_OFA_MEMORY_UPDATE);
+ /* This function acquires &ctrl_info->ofa_mutex and doesn't release it. */
+ __release(&ctrl_info->ofa_mutex);
}

static void pqi_ofa_quiesce_worker(struct work_struct *work)
@@ -3738,6 +3753,8 @@ static bool pqi_ofa_process_event(struct
pqi_ctrl_info *ctrl_info,
"received Online Firmware Activation cancel request: reason: %u\n",
ctrl_info->ofa_cancel_reason);
pqi_host_free_buffer(ctrl_info, &ctrl_info->ofa_memory);
+ /* What guarantees that &ctrl_info->ofa_mutex is held here? */
+ __acquire(&ctrl_info->ofa_mutex);
pqi_ctrl_ofa_done(ctrl_info);
break;
default:
@@ -8726,6 +8743,7 @@ static int pqi_ctrl_init_resume(struct
pqi_ctrl_info *ctrl_info)
}

if (pqi_ofa_in_progress(ctrl_info)) {
+ __acquire(&ctrl_info->scan_mutex);
pqi_ctrl_unblock_scan(ctrl_info);
if (ctrl_info->ctrl_logging_supported) {
if (!ctrl_info->ctrl_log_memory.host_memory)
@@ -8938,6 +8956,8 @@ static void pqi_remove_ctrl(struct pqi_ctrl_info
*ctrl_info)
}

static void pqi_ofa_ctrl_quiesce(struct pqi_ctrl_info *ctrl_info)
+ __acquires(&ctrl_info->scan_mutex)
+ __acquires(&ctrl_info->lun_reset_mutex)
{
pqi_ctrl_block_scan(ctrl_info);
pqi_scsi_block_requests(ctrl_info);
@@ -8948,6 +8968,8 @@ static void pqi_ofa_ctrl_quiesce(struct
pqi_ctrl_info *ctrl_info)
}

static void pqi_ofa_ctrl_unquiesce(struct pqi_ctrl_info *ctrl_info)
+ __releases(&ctrl_info->lun_reset_mutex)
+ __releases(&ctrl_info->scan_mutex)
{
pqi_start_heartbeat_timer(ctrl_info);
pqi_ctrl_unblock_requests(ctrl_info);
@@ -9410,6 +9432,7 @@ static void pqi_shutdown(struct pci_dev *pci_dev)
pqi_ctrl_block_device_reset(ctrl_info);
pqi_ctrl_block_requests(ctrl_info);
pqi_ctrl_wait_until_quiesced(ctrl_info);
+ __release(&ctrl_info->lun_reset_mutex);

if (system_state == SYSTEM_RESTART)
shutdown_event = RESTART;
@@ -9486,6 +9509,8 @@ static inline enum bmic_flush_cache_shutdown_event
pqi_get_flush_cache_shutdown_
}

static int pqi_suspend_or_freeze(struct device *dev, bool suspend)
+ __acquires(&((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->scan_mutex)
+ __acquires(&((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->lun_reset_mutex)
{
struct pci_dev *pci_dev;
struct pqi_ctrl_info *ctrl_info;
@@ -9519,11 +9544,15 @@ static int pqi_suspend_or_freeze(struct device
*dev, bool suspend)
}

static __maybe_unused int pqi_suspend(struct device *dev)
+ __acquires(&((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->scan_mutex)
+ __acquires(&((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->lun_reset_mutex)
{
return pqi_suspend_or_freeze(dev, true);
}

static int pqi_resume_or_restore(struct device *dev)
+ __cond_releases(0, &((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->lun_reset_mutex)
+ __cond_releases(0, &((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->scan_mutex)
{
int rc;
struct pci_dev *pci_dev;
@@ -9547,11 +9576,15 @@ static int pqi_resume_or_restore(struct device *dev)
}

static int pqi_freeze(struct device *dev)
+ __acquires(&((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->scan_mutex)
+ __acquires(&((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->lun_reset_mutex)
{
return pqi_suspend_or_freeze(dev, false);
}

static int pqi_thaw(struct device *dev)
+ __cond_releases(0, &((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->lun_reset_mutex)
+ __cond_releases(0, &((struct pqi_ctrl_info
*)pci_get_drvdata(to_pci_dev(dev)))->scan_mutex)
{
int rc;
struct pci_dev *pci_dev;