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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20251005-remoteproc-cleanup-v1-4-09a9fdea0063@nxp.com>
Date: Sun, 05 Oct 2025 22:14:29 +0800
From: Peng Fan <peng.fan@....com>
To: Bjorn Andersson <andersson@...nel.org>, 
 Mathieu Poirier <mathieu.poirier@...aro.org>, Andrew Davis <afd@...com>, 
 Arnaud Pouliquen <arnaud.pouliquen@...s.st.com>, 
 Daniel Baluta <daniel.baluta@....com>
Cc: linux-remoteproc@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Peng Fan <peng.fan@....com>
Subject: [PATCH 4/6] remoteproc: core: Use cleanup.h macros to simplify
 lock handling

Replace manual mutex_lock/unlock and error-handling patterns with cleanup.h
macros (ACQUIRE, ACQUIRE_ERR, and scoped_guard) to streamline lock
management. As a result, several goto labels and redundant error paths are
eliminated.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@....com>
---
 drivers/remoteproc/remoteproc_core.c | 113 ++++++++++++++---------------------
 1 file changed, 45 insertions(+), 68 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 8004a480348378abef78ad5641a8c8b5766c20a6..dd859378f6ff6dec2728980cc82d31687aa7a3dc 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -17,6 +17,7 @@
 #define pr_fmt(fmt)    "%s: " fmt, __func__
 
 #include <asm/byteorder.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/dma-mapping.h>
@@ -1830,13 +1831,14 @@ int rproc_trigger_recovery(struct rproc *rproc)
 	struct device *dev = &rproc->dev;
 	int ret;
 
-	ret = mutex_lock_interruptible(&rproc->lock);
+	ACQUIRE(mutex_intr, lock)(&rproc->lock);
+	ret = ACQUIRE_ERR(mutex_intr, &lock);
 	if (ret)
 		return ret;
 
 	/* State could have changed before we got the mutex */
 	if (rproc->state != RPROC_CRASHED)
-		goto unlock_mutex;
+		return ret;
 
 	dev_err(dev, "recovering %s\n", rproc->name);
 
@@ -1845,8 +1847,6 @@ int rproc_trigger_recovery(struct rproc *rproc)
 	else
 		ret = rproc_boot_recovery(rproc);
 
-unlock_mutex:
-	mutex_unlock(&rproc->lock);
 	return ret;
 }
 
@@ -1864,25 +1864,19 @@ static void rproc_crash_handler_work(struct work_struct *work)
 
 	dev_dbg(dev, "enter %s\n", __func__);
 
-	mutex_lock(&rproc->lock);
-
-	if (rproc->state == RPROC_CRASHED) {
+	scoped_guard(mutex, &rproc->lock) {
 		/* handle only the first crash detected */
-		mutex_unlock(&rproc->lock);
-		return;
-	}
+		if (rproc->state == RPROC_CRASHED)
+			return;
 
-	if (rproc->state == RPROC_OFFLINE) {
 		/* Don't recover if the remote processor was stopped */
-		mutex_unlock(&rproc->lock);
-		goto out;
-	}
-
-	rproc->state = RPROC_CRASHED;
-	dev_err(dev, "handling crash #%u in %s\n", ++rproc->crash_cnt,
-		rproc->name);
+		if (rproc->state == RPROC_OFFLINE)
+			goto out;
 
-	mutex_unlock(&rproc->lock);
+		rproc->state = RPROC_CRASHED;
+		dev_err(dev, "handling crash #%u in %s\n", ++rproc->crash_cnt,
+			rproc->name);
+	}
 
 	if (!rproc->recovery_disabled)
 		rproc_trigger_recovery(rproc);
@@ -1915,23 +1909,21 @@ int rproc_boot(struct rproc *rproc)
 
 	dev = &rproc->dev;
 
-	ret = mutex_lock_interruptible(&rproc->lock);
+	ACQUIRE(mutex_intr, lock)(&rproc->lock);
+	ret = ACQUIRE_ERR(mutex_intr, &lock);
 	if (ret) {
 		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
 		return ret;
 	}
 
 	if (rproc->state == RPROC_DELETED) {
-		ret = -ENODEV;
 		dev_err(dev, "can't boot deleted rproc %s\n", rproc->name);
-		goto unlock_mutex;
+		return -ENODEV;
 	}
 
 	/* skip the boot or attach process if rproc is already powered up */
-	if (atomic_inc_return(&rproc->power) > 1) {
-		ret = 0;
-		goto unlock_mutex;
-	}
+	if (atomic_inc_return(&rproc->power) > 1)
+		return 0;
 
 	if (rproc->state == RPROC_DETACHED) {
 		dev_info(dev, "attaching to %s\n", rproc->name);
@@ -1955,8 +1947,7 @@ int rproc_boot(struct rproc *rproc)
 downref_rproc:
 	if (ret)
 		atomic_dec(&rproc->power);
-unlock_mutex:
-	mutex_unlock(&rproc->lock);
+
 	return ret;
 }
 EXPORT_SYMBOL(rproc_boot);
@@ -1987,26 +1978,24 @@ int rproc_shutdown(struct rproc *rproc)
 	struct device *dev = &rproc->dev;
 	int ret;
 
-	ret = mutex_lock_interruptible(&rproc->lock);
+	ACQUIRE(mutex_intr, lock)(&rproc->lock);
+	ret = ACQUIRE_ERR(mutex_intr, &lock);
 	if (ret) {
 		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
 		return ret;
 	}
 
-	if (rproc->state != RPROC_RUNNING &&
-	    rproc->state != RPROC_ATTACHED) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (rproc->state != RPROC_RUNNING && rproc->state != RPROC_ATTACHED)
+		return -EINVAL;
 
 	/* if the remote proc is still needed, bail out */
 	if (!atomic_dec_and_test(&rproc->power))
-		goto out;
+		return ret;
 
 	ret = rproc_stop(rproc, false);
 	if (ret) {
 		atomic_inc(&rproc->power);
-		goto out;
+		return ret;
 	}
 
 	/* clean up all acquired resources */
@@ -2021,8 +2010,7 @@ int rproc_shutdown(struct rproc *rproc)
 	kfree(rproc->cached_table);
 	rproc->cached_table = NULL;
 	rproc->table_ptr = NULL;
-out:
-	mutex_unlock(&rproc->lock);
+
 	return ret;
 }
 EXPORT_SYMBOL(rproc_shutdown);
@@ -2052,27 +2040,25 @@ int rproc_detach(struct rproc *rproc)
 	struct device *dev = &rproc->dev;
 	int ret;
 
-	ret = mutex_lock_interruptible(&rproc->lock);
+	ACQUIRE(mutex_intr, lock)(&rproc->lock);
+	ret = ACQUIRE_ERR(mutex_intr, &lock);
 	if (ret) {
 		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
 		return ret;
 	}
 
 	if (rproc->state != RPROC_ATTACHED) {
-		ret = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	/* if the remote proc is still needed, bail out */
-	if (!atomic_dec_and_test(&rproc->power)) {
-		ret = 0;
-		goto out;
-	}
+	if (!atomic_dec_and_test(&rproc->power))
+		return 0;
 
 	ret = __rproc_detach(rproc);
 	if (ret) {
 		atomic_inc(&rproc->power);
-		goto out;
+		return ret;
 	}
 
 	/* clean up all acquired resources */
@@ -2087,8 +2073,7 @@ int rproc_detach(struct rproc *rproc)
 	kfree(rproc->cached_table);
 	rproc->cached_table = NULL;
 	rproc->table_ptr = NULL;
-out:
-	mutex_unlock(&rproc->lock);
+
 	return ret;
 }
 EXPORT_SYMBOL(rproc_detach);
@@ -2192,7 +2177,8 @@ int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
 
 	dev = rproc->dev.parent;
 
-	ret = mutex_lock_interruptible(&rproc->lock);
+	ACQUIRE(mutex_intr, lock)(&rproc->lock);
+	ret = ACQUIRE_ERR(mutex_intr, &lock);
 	if (ret) {
 		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
 		return -EINVAL;
@@ -2200,28 +2186,22 @@ int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
 
 	if (rproc->state != RPROC_OFFLINE) {
 		dev_err(dev, "can't change firmware while running\n");
-		ret = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 
 	len = strcspn(fw_name, "\n");
 	if (!len) {
 		dev_err(dev, "can't provide empty string for firmware name\n");
-		ret = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	p = kstrndup(fw_name, len, GFP_KERNEL);
-	if (!p) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!p)
+		return -ENOMEM;
 
 	kfree_const(rproc->firmware);
 	rproc->firmware = p;
 
-out:
-	mutex_unlock(&rproc->lock);
 	return ret;
 }
 EXPORT_SYMBOL(rproc_set_firmware);
@@ -2316,9 +2296,8 @@ int rproc_add(struct rproc *rproc)
 	}
 
 	/* expose to rproc_get_by_phandle users */
-	mutex_lock(&rproc_list_mutex);
-	list_add_rcu(&rproc->node, &rproc_list);
-	mutex_unlock(&rproc_list_mutex);
+	scoped_guard(mutex, &rproc_list_mutex)
+		list_add_rcu(&rproc->node, &rproc_list);
 
 	return 0;
 
@@ -2582,16 +2561,14 @@ int rproc_del(struct rproc *rproc)
 	/* TODO: make sure this works with rproc->power > 1 */
 	rproc_shutdown(rproc);
 
-	mutex_lock(&rproc->lock);
-	rproc->state = RPROC_DELETED;
-	mutex_unlock(&rproc->lock);
+	scoped_guard(mutex, &rproc->lock)
+		rproc->state = RPROC_DELETED;
 
 	rproc_delete_debug_dir(rproc);
 
 	/* the rproc is downref'ed as soon as it's removed from the klist */
-	mutex_lock(&rproc_list_mutex);
-	list_del_rcu(&rproc->node);
-	mutex_unlock(&rproc_list_mutex);
+	scoped_guard(mutex, &rproc_list_mutex)
+		list_del_rcu(&rproc->node);
 
 	/* Ensure that no readers of rproc_list are still active */
 	synchronize_rcu();

-- 
2.37.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ