linux-kernel - [PATCH v2] scsi: ufs: core: Do not hold any lock in ufshcd_hba

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20241128072542.219170-1-avri.altman@wdc.com>
Date: Thu, 28 Nov 2024 09:25:42 +0200
From: Avri Altman <avri.altman@....com>
To: "Martin K . Petersen" <martin.petersen@...cle.com>
Cc: linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Bart Van Assche <bvanassche@....org>,
	Avri Altman <avri.altman@....com>
Subject: [PATCH v2] scsi: ufs: core: Do not hold any lock in ufshcd_hba_stop

This change is motivated by Bart's suggestion in [1], which enables to
further reduce the scsi host lock usage in the ufs driver. The reason
why it make sense, because although the legacy interrupt is disabled by
some but not all ufshcd_hba_stop() callers, it is safe to nest
disable_irq() calls as it checks the irq depth.

[1] https://lore.kernel.org/linux-scsi/c58e4fce-0a74-4469-ad16-f1edbd670728@acm.org/

Suggested-by: Bart Van Assche <bvanassche@....org>
Signed-off-by: Avri Altman <avri.altman@....com>

---
Changes compared to v1:
 - ensure that interrupts are only re-enabled after the controller has
   been fully disabled (Bart)
---
 drivers/ufs/core/ufshcd.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index acc3607bbd9c..2e4748077275 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4811,20 +4811,14 @@ EXPORT_SYMBOL_GPL(ufshcd_make_hba_operational);
  */
 void ufshcd_hba_stop(struct ufs_hba *hba)
 {
-	unsigned long flags;
 	int err;
 
-	/*
-	 * Obtain the host lock to prevent that the controller is disabled
-	 * while the UFS interrupt handler is active on another CPU.
-	 */
-	spin_lock_irqsave(hba->host->host_lock, flags);
+	ufshcd_disable_irq(hba);
 	ufshcd_writel(hba, CONTROLLER_DISABLE,  REG_CONTROLLER_ENABLE);
-	spin_unlock_irqrestore(hba->host->host_lock, flags);
-
 	err = ufshcd_wait_for_register(hba, REG_CONTROLLER_ENABLE,
 					CONTROLLER_ENABLE, CONTROLLER_DISABLE,
 					10, 1);
+	ufshcd_enable_irq(hba);
 	if (err)
 		dev_err(hba->dev, "%s: Controller disable failed\n", __func__);
 }
-- 
2.25.1