vmatare / thinkfan

The minimalist fan control program
GNU General Public License v3.0
552 stars 62 forks source link

ERROR: Segmentation fault #14

Closed Andrei-Pozolotin closed 8 years ago

Andrei-Pozolotin commented 8 years ago

package:

https://aur.archlinux.org/packages/thinkfan-git/

system:

uname -a
Linux work2 4.6.2-1-ARCH #1 SMP PREEMPT Wed Jun 8 08:40:59 CEST 2016 x86_64 GNU/Linux

journal:

thinkfan[25665]: Temperatures(bias): 57(4), 75(31) ->
thinkfan[25665]: ERROR: Segmentation fault.
systemd[1]: thinkfan.service: Main process exited, code=exited, status=2/INVALIDARGUMENT thinkfan[25665]: Backtrace:
thinkfan[25665]: /usr/bin/thinkfan() [0x421f7d]
                                       /usr/bin/thinkfan(_ZN8thinkfan3BugC2ERKSs+0x33) [0x422313]
                                       /usr/bin/thinkfan(_ZN8thinkfan11sig_handlerEi+0xae) [0x40b53e]
                                       /usr/lib/libc.so.6(+0x33310) [0x7f6409954310]
                                       /usr/lib/libstdc++.so.6(_ZNSs6appendERKSs+0x10) [0x7f6409faa330]
                                       /usr/bin/thinkfan(_ZN8thinkfan6LoggerlsERKSs+0xd) [0x41a72d]
                                       /usr/bin/thinkfan(_ZN8thinkfan3runERKNS_6ConfigE+0x328) [0x40d9a8]
                                       /usr/bin/thinkfan(main+0x423) [0x40abd3]
                                       /usr/lib/libc.so.6(__libc_start_main+0xf1) [0x7f6409941741]
                                       /usr/bin/thinkfan(_start+0x29) [0x40b389]
Andrei-Pozolotin commented 8 years ago

/etc/thinkfan.conf

# module: thermal
hwmon /sys/class/thermal/thermal_zone0/temp

# module: x86_pkg_temp_thermal
hwmon /sys/class/thermal/thermal_zone1/temp

# module: thinkpad_acpi
tp_fan /proc/acpi/ibm/fan

# mapping
(0, 0,  45) # 0
(1, 40, 55) # 1900
(2, 50, 60) #
(3, 53, 63) #
(4, 55, 65) #
(5, 60, 70) #
(6, 63, 73) #
(7, 67, 77) # 3600
#(127, 75, 32767) # 5000 # out of range
Andrei-Pozolotin commented 8 years ago

BTW, config entry

(127, 75, 32767) # 5000 # unlimited rpm

used to work fine before v 1.0, now it reports as invalid value; how can I restore original behavior?

thank you.

vmatare commented 8 years ago

Thanks for reporting. Apparently level 127 used to be handled specially in 0.9. Now it's just written to /proc/acpi/ibm/fan, which is of course invalid. Workaround: use "level full-speed" instead of 127 (including the double quotes).

Andrei-Pozolotin commented 8 years ago

I confirm that full speed entry works: ("level full-speed", 75, 32767) # 5000 rpm BTW, is it too hard to add RPM logging to current format:

thinkfan[20051]: Temperatures(bias): 49(0), 49(0) -> level 1

for example:

thinkfan[20051]: Temperatures(bias): 49(0), 49(0) -> level 1 [rpm=2345]

? or even expose log format string in thinkfan.conf ? with a dozen or so available state variables? :-)

vmatare commented 8 years ago

Well, that would be a nice feature indeed. However there are two caveats:

  1. I don't want to increase the number of per-loop CPU operations. Keep in mind that thinkfan wakes up the CPU every 5 seconds or so to poll temperatures, and we want to be done with that as quickly as possible. However we can't output RPM immediately after increasing the fan speed since physical RPM will always take a bunch of seconds to ramp up. So that would require additional per-loop logic of which I'm not sure whether it's worth the cost.
  2. The config syntax is quite convoluted by now, since it evolved from a very simplistic format with a trivial parser into its current state that requires a pretty involved, hand-written parser. Either I come up with a very clean redesign of that parser, or I'll have to migrate to an existing, clean grammar (like YAML) before I implement additional config syntax.
Andrei-Pozolotin commented 8 years ago

thank you for considering this. re: 1) "will always take a bunch of seconds" - no need to be exact, current or average is fine re: 2) "migrate to an existing, clean grammar (like YAML) " - yes, please switch to yaml

vmatare commented 8 years ago

Alright, tracking the YAML topic as issue #15 now.

vmatare commented 8 years ago

The segfault reported in the initial comment is actually the same as in issue #18, which is fixed now, too. So this bug is also done.