public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: smartpqi silence a recursive lock warning
@ 2026-04-14 12:41 Tomas Henzl
  2026-04-14 16:38 ` Bart Van Assche
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Tomas Henzl @ 2026-04-14 12:41 UTC (permalink / raw)
  To: linux-scsi; +Cc: Don.Brace

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)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: smartpqi silence a recursive lock warning
  2026-04-14 12:41 [PATCH] scsi: smartpqi silence a recursive lock warning Tomas Henzl
@ 2026-04-14 16:38 ` Bart Van Assche
  2026-04-15 13:00   ` Tomas Henzl
  2026-04-16 14:14 ` Don.Brace
  2026-04-21  2:27 ` Martin K. Petersen
  2 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2026-04-14 16:38 UTC (permalink / raw)
  To: Tomas Henzl, linux-scsi; +Cc: Don.Brace

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;


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: smartpqi silence a recursive lock warning
  2026-04-14 16:38 ` Bart Van Assche
@ 2026-04-15 13:00   ` Tomas Henzl
  2026-04-15 22:03     ` Bart Van Assche
  0 siblings, 1 reply; 6+ messages in thread
From: Tomas Henzl @ 2026-04-15 13:00 UTC (permalink / raw)
  To: Bart Van Assche, linux-scsi; +Cc: Don.Brace

On 4/14/26 6:38 PM, Bart Van Assche wrote:
> 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.

Each mutex lock ought to have a corresponding unlock.
However, in cases where a driver exits a shutdown function while holding
a mutex, this is not strictly necessary. The shutdown process will
continue, and once the power is off, everything is effectively reset.

I chose to add the unlock rather than use lockdep classes,
mainly because that is what most developers expect.

A potential problem is that, after introducing the unlock, a hypothetical
situation could arise where another thread, which would previously
have remained blocked, is now able to proceed, and something bad could
happen as a result. I looked into this possibility and believe that it
is safe.

I think that the missing unlock was an oversight rather than an intention.


> 
> 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.

You are likely right, but I posted this fix to address the warning and
for now I'd like to stick with that.

Thanks,
tomash

> 
> 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;
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: smartpqi silence a recursive lock warning
  2026-04-15 13:00   ` Tomas Henzl
@ 2026-04-15 22:03     ` Bart Van Assche
  0 siblings, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2026-04-15 22:03 UTC (permalink / raw)
  To: Tomas Henzl, linux-scsi; +Cc: Don.Brace

On 4/15/26 6:00 AM, Tomas Henzl wrote:
> I posted this fix to address the warning and
> for now I'd like to stick with that.

That sounds fair to me.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: smartpqi silence a recursive lock warning
  2026-04-14 12:41 [PATCH] scsi: smartpqi silence a recursive lock warning Tomas Henzl
  2026-04-14 16:38 ` Bart Van Assche
@ 2026-04-16 14:14 ` Don.Brace
  2026-04-21  2:27 ` Martin K. Petersen
  2 siblings, 0 replies; 6+ messages in thread
From: Don.Brace @ 2026-04-16 14:14 UTC (permalink / raw)
  To: thenzl, linux-scsi

________________________________________
From: Tomas Henzl <thenzl@redhat.com>
Sent: Tuesday, April 14, 2026 7:41 AM
To: linux-scsi@vger.kernel.org <linux-scsi@vger.kernel.org>
Cc: Don Brace - C33706 <Don.Brace@microchip.com>
Subject: [PATCH] scsi: smartpqi silence a recursive lock warning
 

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>

Please fix spelling: Supress --- Suppress.
Thank-you for your patch.
Acked-by: Don Brace <don.brace@microchip.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)
--
2.53.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: smartpqi silence a recursive lock warning
  2026-04-14 12:41 [PATCH] scsi: smartpqi silence a recursive lock warning Tomas Henzl
  2026-04-14 16:38 ` Bart Van Assche
  2026-04-16 14:14 ` Don.Brace
@ 2026-04-21  2:27 ` Martin K. Petersen
  2 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2026-04-21  2:27 UTC (permalink / raw)
  To: Tomas Henzl; +Cc: linux-scsi, Don.Brace


Tomas,

> 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.

Applied to 7.1/scsi-staging, thanks!

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-21  2:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-14 12:41 [PATCH] scsi: smartpqi silence a recursive lock warning Tomas Henzl
2026-04-14 16:38 ` Bart Van Assche
2026-04-15 13:00   ` Tomas Henzl
2026-04-15 22:03     ` Bart Van Assche
2026-04-16 14:14 ` Don.Brace
2026-04-21  2:27 ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox