| 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
Hi Krzysztof, Thanks for your review comments. On Tue, 11 Mar 2025 at 23:00, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > 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 Ok. > > > 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. Ok, I will check this next time before submitting the changes. > > > --- > > 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. I thought this should add a global guard to exynos_tmu_data using mutex_lock and mutex_unlock. I'll drop this if it turns out to be unnecessary. > > > + 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. > Ok, I will drop this include of cleanup.h. > > Best regards, > Krzysztof Thanks -Anand
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(-)