Thread (10 messages) 10 messages, 2 authors, 2025-03-13

Re: [PATCH v4 4/4] drivers/thermal/exymos: Use guard notation when acquiring mutex

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-03-11 17:30:14
Also in: linux-pm, linux-samsung-soc, lkml

On 10/03/2025 15:34, Anand Moon wrote:
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Subject: typo, exynos
Signed-off-by: Anand Moon <redacted>
---
v4: used DEFINE_GUARD macro to guard exynos_tmu_data structure.
    However, incorporating guard(exynos_tmu_data)(data); results
    in a recursive deadlock with the mutex during initialization, as this
    data structure is common to all the code configurations of Exynos TMU
v3: New patch
If you ever use cleanup or guards, you must build your code with recent
clang and W=1. Failure to do so means you ask reviewers manually to spot
issues not visible in the context, instead of using tools. It's a NAK
for me.
quoted hunk ↗ jump to hunk
---
 drivers/thermal/samsung/exynos_tmu.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index a71cde0a4b17e..85f88c5e0f11c 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/cleanup.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
@@ -199,6 +200,9 @@ struct exynos_tmu_data {
 	void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
 };
 
+DEFINE_GUARD(exynos_tmu_data, struct exynos_tmu_data *,
I do not understand why do you need custom guard.
quoted hunk ↗ jump to hunk
+	     mutex_lock(&_T->lock), mutex_unlock(&_T->lock))
+
 /*
  * TMU treats temperature as a mapped temperature code.
  * The temperature is converted differently depending on the calibration type.
@@ -256,7 +260,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	unsigned int status;
 	int ret = 0;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
Which you do not use... Please don't use cleanup.h if you do not know
it. It leads to bugs.


Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help