| Message ID | 20250310143450.8276-5-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Exynos Thermal code improvement | expand |
Hi Anand,
kernel test robot noticed the following build errors:
[auto build test ERROR on 80e54e84911a923c40d7bee33a34c1b4be148d7a]
url: https://github.com/intel-lab-lkp/linux/commits/Anand-Moon/drivers-thermal-exynos-Refactor-clk_sec-initialization-inside-SOC-specific-case/20250310-223732
base: 80e54e84911a923c40d7bee33a34c1b4be148d7a
patch link: https://lore.kernel.org/r/20250310143450.8276-5-linux.amoon%40gmail.com
patch subject: [PATCH v4 4/4] drivers/thermal/exymos: Use guard notation when acquiring mutex
config: s390-randconfig-001-20250311 (https://download.01.org/0day-ci/archive/20250312/202503120028.ZL9zhHXe-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250312/202503120028.ZL9zhHXe-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503120028.ZL9zhHXe-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/thermal/samsung/exynos_tmu.c:721:3: error: cannot jump from this goto statement to its label
goto out;
^
drivers/thermal/samsung/exynos_tmu.c:723:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
guard(mutex)(&data->lock);
^
include/linux/cleanup.h:309:15: note: expanded from macro 'guard'
CLASS(_name, __UNIQUE_ID(guard))
^
include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
^
include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:78:1: note: expanded from here
__UNIQUE_ID_guard398
^
drivers/thermal/samsung/exynos_tmu.c:718:3: error: cannot jump from this goto statement to its label
goto out;
^
drivers/thermal/samsung/exynos_tmu.c:723:2: note: jump bypasses initialization of variable with __attribute__((cleanup))
guard(mutex)(&data->lock);
^
include/linux/cleanup.h:309:15: note: expanded from macro 'guard'
CLASS(_name, __UNIQUE_ID(guard))
^
include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
#define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
^
include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
#define __PASTE(a,b) ___PASTE(a,b)
^
include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
#define ___PASTE(a,b) a##b
^
<scratch space>:78:1: note: expanded from here
__UNIQUE_ID_guard398
^
2 errors generated.
vim +721 drivers/thermal/samsung/exynos_tmu.c
285d994a51e45ca drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13 711
7ea98f70c73ea37 drivers/thermal/samsung/exynos_tmu.c Daniel Lezcano 2022-08-05 712 static int exynos_tmu_set_emulation(struct thermal_zone_device *tz, int temp)
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 713 {
5f68d0785e5258a drivers/thermal/samsung/exynos_tmu.c Daniel Lezcano 2023-03-01 714 struct exynos_tmu_data *data = thermal_zone_device_priv(tz);
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 715 int ret = -EINVAL;
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 716
ef3f80fc7f79c32 drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13 717 if (data->soc == SOC_ARCH_EXYNOS4210)
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 718 goto out;
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 719
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 720 if (temp && temp < MCELSIUS)
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 @721 goto out;
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 722
0c7c68a34f7d0f7 drivers/thermal/samsung/exynos_tmu.c Anand Moon 2025-03-10 723 guard(mutex)(&data->lock);
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 724 clk_enable(data->clk);
285d994a51e45ca drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13 725 data->tmu_set_emulation(data, temp);
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 726 clk_disable(data->clk);
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 727 return 0;
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 728 out:
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 729 return ret;
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 730 }
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 731 #else
285d994a51e45ca drivers/thermal/samsung/exynos_tmu.c Bartlomiej Zolnierkiewicz 2014-11-13 732 #define exynos4412_tmu_set_emulation NULL
7ea98f70c73ea37 drivers/thermal/samsung/exynos_tmu.c Daniel Lezcano 2022-08-05 733 static int exynos_tmu_set_emulation(struct thermal_zone_device *tz, int temp)
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 734 { return -EINVAL; }
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 735 #endif /* CONFIG_THERMAL_EMULATION */
bffd1f8ac87a798 drivers/thermal/exynos_thermal.c Amit Daniel Kachhap 2013-02-11 736
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 <linux.amoon@gmail.com> > --- > 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. > --- > 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. > + 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
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 *, + 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); clk_enable(data->clk); clk_enable(data->clk_sec); @@ -270,7 +274,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev) clk_disable(data->clk_sec); clk_disable(data->clk); - mutex_unlock(&data->lock); return ret; } @@ -292,13 +295,12 @@ static int exynos_thermal_zone_configure(struct platform_device *pdev) return ret; } - mutex_lock(&data->lock); + guard(mutex)(&data->lock); clk_enable(data->clk); data->tmu_set_crit_temp(data, temp / MCELSIUS); clk_disable(data->clk); - mutex_unlock(&data->lock); return 0; } @@ -325,12 +327,11 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on) { struct exynos_tmu_data *data = platform_get_drvdata(pdev); - mutex_lock(&data->lock); + guard(mutex)(&data->lock); clk_enable(data->clk); data->tmu_control(pdev, on); data->enabled = on; clk_disable(data->clk); - mutex_unlock(&data->lock); } static void exynos_tmu_update_bit(struct exynos_tmu_data *data, int reg_off, @@ -645,7 +646,7 @@ static int exynos_get_temp(struct thermal_zone_device *tz, int *temp) */ return -EAGAIN; - mutex_lock(&data->lock); + guard(mutex)(&data->lock); clk_enable(data->clk); value = data->tmu_read(data); @@ -655,7 +656,6 @@ static int exynos_get_temp(struct thermal_zone_device *tz, int *temp) *temp = code_to_temp(data, value) * MCELSIUS; clk_disable(data->clk); - mutex_unlock(&data->lock); return ret; } @@ -720,11 +720,10 @@ static int exynos_tmu_set_emulation(struct thermal_zone_device *tz, int temp) if (temp && temp < MCELSIUS) goto out; - mutex_lock(&data->lock); + guard(mutex)(&data->lock); clk_enable(data->clk); data->tmu_set_emulation(data, temp); clk_disable(data->clk); - mutex_unlock(&data->lock); return 0; out: return ret; @@ -760,14 +759,13 @@ static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id) thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED); - mutex_lock(&data->lock); + guard(mutex)(&data->lock); clk_enable(data->clk); /* TODO: take action based on particular interrupt */ data->tmu_clear_irqs(data); clk_disable(data->clk); - mutex_unlock(&data->lock); return IRQ_HANDLED; } @@ -987,7 +985,7 @@ static int exynos_set_trips(struct thermal_zone_device *tz, int low, int high) { struct exynos_tmu_data *data = thermal_zone_device_priv(tz); - mutex_lock(&data->lock); + guard(mutex)(&data->lock); clk_enable(data->clk); if (low > INT_MIN) @@ -1000,7 +998,6 @@ static int exynos_set_trips(struct thermal_zone_device *tz, int low, int high) data->tmu_disable_high(data); clk_disable(data->clk); - mutex_unlock(&data->lock); return 0; }
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. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- 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 --- drivers/thermal/samsung/exynos_tmu.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)