Re: [PATCH v2 0/4] tools/thermal: thermal library and tools

[srinivas pandruvada <srinivas.pandruvada@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5296 bytes --]

Hi Daniel,

The attached diff fixes the crash,

Also when you run in daemon mode, you need to use some pid lock file.
Otherwise it launches multiple daemons.

Thanks,
Srinivas



On Sun, 2022-04-17 at 10:20 +0200, Daniel Lezcano wrote:
> 
> Hi Srinivas,
> 
> thanks for the information. I've indeed not tested on x86_64 but on
> ARM64.
> 
> I'll give a try to reproduce and fix the issue.
> 
> Thanks again for taking the time to run it
> 
>    -- Daniel
> 
> On 17/04/2022 06:23, srinivas pandruvada wrote:
> > Hi Daniel,
> > 
> > On Sat, 2022-04-16 at 09:55 +0200, Daniel Lezcano wrote:
> > > 
> > > Hi Srinivas,
> > > 
> > > 
> > > On 16/04/2022 05:50, srinivas pandruvada wrote:
> > > > On Wed, 2022-04-13 at 10:21 -0700, srinivas pandruvada wrote:
> > > > > On Wed, 2022-04-13 at 17:06 +0200, Rafael J. Wysocki wrote:
> > > > > > On Wed, Apr 6, 2022 at 4:44 PM Daniel Lezcano <
> > > > > > daniel.lezcano@linaro.org> wrote:
> > > > > > > 
> > > > 
> > 
> > [...]
> > 
> > > > root@srinivas-otcpl-icl-u:~/development/linux/tools/thermal/the
> > > > rmal-
> > > > engine#
> > > > LD_LIBRARY_PATH=../lib:../../lib/thermal:$LD_LIBRARY_pATH
> > > > ./thermal-engine -l DEBUG
> > > > Segmentation fault (core dumped)
> > > > root@srinivas-otcpl-icl-u:~/development/linux/tools/thermal/the
> > > > rmal-
> > > > engine#
> > > > LD_LIBRARY_PATH=../lib:../../lib/thermal:$LD_LIBRARY_pATH
> > > > ./thermal-engine --loglevel DEBUG
> > > > Segmentation fault (core dumped)
> > > > root@srinivas-otcpl-icl-u:~/development/linux/tools/thermal/the
> > > > rmal-
> > > > engine#
> > > > LD_LIBRARY_PATH=../lib:../../lib/thermal:$LD_LIBRARY_pATH
> > > > ./thermal-engine -s
> > > > Segmentation fault (core dumped)
> > > 
> > > Is CONFIG_THERMAL_NETLINK set ?
> > 
> > Yes, Thermometer program works fine. Just thermal-engine fails.
> > CONFIG_THERMAL=y
> > CONFIG_THERMAL_NETLINK=y
> > CONFIG_THERMAL_STATISTICS=y
> > CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS=0
> > CONFIG_THERMAL_HWMON=y
> > CONFIG_THERMAL_WRITABLE_TRIPS=y
> > CONFIG_THERMAL_DEFAULT_GOV_STEP_WISE=y
> > # CONFIG_THERMAL_DEFAULT_GOV_FAIR_SHARE is not set
> > # CONFIG_THERMAL_DEFAULT_GOV_USER_SPACE is not set
> > CONFIG_THERMAL_GOV_FAIR_SHARE=y
> > CONFIG_THERMAL_GOV_STEP_WISE=y
> > CONFIG_THERMAL_GOV_BANG_BANG=y
> > CONFIG_THERMAL_GOV_USER_SPACE=y
> > CONFIG_DEVFREQ_THERMAL=y
> > CONFIG_THERMAL_EMULATION=y
> > 
> > This may help
> > 
> > #gdb ./thermal-engine
> > GNU gdb (Ubuntu 8.1.1-0ubuntu1) 8.1.1
> > Copyright (C) 2018 Free Software Foundation, Inc.
> > License GPLv3+: GNU GPL version 3 or later
> > <http://gnu.org/licenses/gpl.html>
> > This is free software: you are free to change and redistribute it.
> > There is NO WARRANTY, to the extent permitted by law.  Type "show
> > copying"
> > and "show warranty" for details.
> > This GDB was configured as "x86_64-linux-gnu".
> > Type "show configuration" for configuration details.
> > For bug reporting instructions, please see:
> > <http://www.gnu.org/software/gdb/bugs/>.
> > Find the GDB manual and other documentation resources online at:
> > <http://www.gnu.org/software/gdb/documentation/>.
> > For help, type "help".
> > Type "apropos word" to search for commands related to "word"...
> > Reading symbols from ./thermal-engine...(no debugging symbols
> > found)...done.
> > (gdb) run
> > Starting program:
> > /home/labuser/development/linux/tools/thermal/thermal-
> > engine/thermal-
> > engine
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/lib/x86_64-linux-
> > gnu/libthread_db.so.1".
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00007ffff7402036 in parse_tz_get_trip (info=0x7fffffffdf60,
> > tz=0x555555607898) at commands.c:143
> > 143             __tt[size].id = -1;
> > (gdb) backtrace
> > #0  0x00007ffff7402036 in parse_tz_get_trip (info=0x7fffffffdf60,
> > tz=0x555555607898) at commands.c:143
> > #1  0x00007ffff7402233 in handle_netlink (unused=0x0,
> > cmd=0x7ffff76052f8 <thermal_cmds+120>, info=0x7fffffffdf60,
> > arg=0x555555607898) at commands.c:206
> > #2  0x00007ffff7003435 in ?? () from /lib/x86_64-linux-gnu/libnl-
> > genl-
> > 3.so.200
> > #3  0x00007ffff6810c1c in nl_recvmsgs_report () from /lib/x86_64-
> > linux-
> > gnu/libnl-3.so.200
> > #4  0x00007ffff6811049 in nl_recvmsgs () from /lib/x86_64-linux-
> > gnu/libnl-3.so.200
> > #5  0x00007ffff7402eaf in nl_send_msg (sock=0x555555606040,
> > cb=0x5555556065d0, msg=0x555555606110, rx_handler=0x7ffff70036f0
> > <genl_handle_msg>, data=0x555555607898) at thermal_nl.c:70
> > #6  0x00007ffff7402327 in thermal_genl_auto (th=0x5555556048a0,
> > id=1,
> > cmd=2, flags=0, arg=0x555555607898) at commands.c:282
> > #7  0x00007ffff74023e0 in thermal_cmd_get_trip (th=0x5555556048a0,
> > tz=0x555555607898) at commands.c:304
> > #8  0x00007ffff74039db in __thermal_zone_discover
> > (tz=0x555555607898,
> > th=0x5555556048a0) at thermal.c:71
> > #9  0x00007ffff7403842 in for_each_thermal_zone (tz=0x555555607860,
> > cb=0x7ffff74039b8 <__thermal_zone_discover>, arg=0x5555556048a0) at
> > thermal.c:33
> > #10 0x00007ffff7403a5b in thermal_zone_discover (th=0x5555556048a0)
> > at
> > thermal.c:87
> > #11 0x00005555554019bd in main ()
> > 
> > Thanks,
> > Srinivas
> > 
> > > 
> > 
> > 
> 
> 


[-- Attachment #2: segv.diff --]
[-- Type: text/x-patch, Size: 2642 bytes --]

git diff
diff --git a/tools/lib/thermal/commands.c b/tools/lib/thermal/commands.c
index 539083780107..b8d933f19909 100644
--- a/tools/lib/thermal/commands.c
+++ b/tools/lib/thermal/commands.c
@@ -117,6 +117,8 @@ static int parse_tz_get_trip(struct genl_info *info, struct thermal_zone *tz)
        size_t size = 0;
        int rem;
 
+       tz->trip_count = 0;
+
        nla_for_each_nested(attr, info->attrs[THERMAL_GENL_ATTR_TZ_TRIP], rem) {
 
                if (nla_type(attr) == THERMAL_GENL_ATTR_TZ_TRIP_ID) {
@@ -140,10 +142,15 @@ static int parse_tz_get_trip(struct genl_info *info, struct thermal_zone *tz)
                        __tt[size - 1].hyst = nla_get_u32(attr);
        }
 
+       if (!size)
+               return THERMAL_ERROR;
+
        __tt[size].id = -1;
 
        tz->trip = __tt;
 
+       tz->trip_count = size;
+
        return THERMAL_SUCCESS;
 }
 
diff --git a/tools/lib/thermal/include/thermal.h b/tools/lib/thermal/include/thermal.h
index 1abc560602cf..5cb9ba8c6c0e 100644
--- a/tools/lib/thermal/include/thermal.h
+++ b/tools/lib/thermal/include/thermal.h
@@ -51,6 +51,7 @@ struct thermal_zone {
        char name[THERMAL_NAME_LENGTH];
        char governor[THERMAL_NAME_LENGTH];
        struct thermal_trip *trip;
+       int trip_count;
 };
 
 struct thermal_cdev {
diff --git a/tools/lib/thermal/thermal.c b/tools/lib/thermal/thermal.c
index a80b967ce334..dd4ebfbbf1d0 100644
--- a/tools/lib/thermal/thermal.c
+++ b/tools/lib/thermal/thermal.c
@@ -19,7 +19,10 @@ int for_each_thermal_trip(struct thermal_trip *tt, cb_tt_t cb, void *arg)
 {
        int i, ret = 0;
 
-       for (i = 0; tt[i].id != -1; i++)
+       if (!tt)
+               return -1;
+
+       for (i = 0; &tt[i] && tt[i].id != -1; i++)
                ret |= cb(&tt[i], arg);
 
        return ret;
@@ -29,6 +32,9 @@ int for_each_thermal_zone(struct thermal_zone *tz, cb_tz_t cb, void *arg)
 {
        int i, ret = 0;
 
+       if (!tz)
+               return 0;
+
        for (i = 0; tz[i].id != -1; i++)
                ret |= cb(&tz[i], arg);
 
diff --git a/tools/thermal/thermal-engine/thermal-engine.c b/tools/thermal/thermal-engine/thermal-engine.c
index 525520049aa2..4e446aec22b5 100644
--- a/tools/thermal/thermal-engine/thermal-engine.c
+++ b/tools/thermal/thermal-engine/thermal-engine.c
@@ -68,7 +68,8 @@ static int show_tz(struct thermal_zone *tz, __maybe_unused void *arg)
 {
        INFO("thermal zone '%s', id=%d\n", tz->name, tz->id);
 
-       for_each_thermal_trip(tz->trip, show_trip, NULL);
+       if (tz->trip_count)
+               for_each_thermal_trip(tz->trip, show_trip, NULL);
 
        show_temp(tz, arg);