Hi all, As newer kernels have improved support for ACPI-based hardware monitoring (be it using the generic thermal_sys driver, or asus_atk0110), it can happen that a kernel upgrade causes hardware monitoring class devices (/sys/class/hwmon/*) to change without any user interaction. While libsensors doesn't care too much, fancontrol does, because it uses the hwmon class device numbers for identification. This can result in a totally broken fan control behavior, as was reported here: https://bugzilla.novell.com/show_bug.cgi?id=529483 At this point, best would probably be to admit that fancontrol was just a quick hack and was never meant for production, and rewrite it in a language that can make use of libsensors. This is however more work than I can afford spending on this myself. So, as a temporary workaround, I propose to extend the /etc/fancontrol configuration file syntax to record the physical device path and name of each used hwmon class device. pwmconfig will be in charge of writing the information at configuration time, and fancontrol will have to check it at run time. If the values do not match, fancontrol will exit immediately: better no fan control at all than fan control based on broken assumptions. I decided to record both the path and the name because: * Not all hwmon devices have a physical device path. Purely virtual devices (for example thermal_sys) don't. So checking only the path wouldn't be sufficient. * The device name is not unique. It is perfectly possible for two devices to have the same name (for example "coretemp"). So neither the path nor the name is sufficient. I hope that having both will be good enough, even though I am well aware it isn't bullet-proof. One drawback is that _everyone_ will have to run pwmconfig again when using this new version of fancontrol. But without a time machine, I just can't think of a way to avoid it. Here's the patch, comments welcome: Index: prog/pwm/fancontrol =================================================================== --- prog/pwm/fancontrol (révision 5766) +++ prog/pwm/fancontrol (copie de travail) @@ -3,12 +3,12 @@ # Simple script implementing a temperature dependent fan speed control # Supported Linux kernel versions: 2.6.5 and later # -# Version 0.69 +# Version 0.70 # # Usage: fancontrol [CONFIGFILE] # # Dependencies: -# bash, egrep, sed, cut, sleep, lm_sensors :) +# bash, egrep, sed, cut, sleep, readlink, lm_sensors :) # # Please send any questions, comments or success stories to # marius.reiner@xxxxxxx @@ -55,6 +55,8 @@ # grep configuration from file INTERVAL=`egrep '^INTERVAL=.*$' $1 | sed -e 's/INTERVAL=//g'` + DEVPATH=`egrep '^DEVPATH=.*$' $1 | sed -e 's/DEVPATH= *//g'` + DEVNAME=`egrep '^DEVNAME=.*$' $1 | sed -e 's/DEVNAME= *//g'` FCTEMPS=`egrep '^FCTEMPS=.*$' $1 | sed -e 's/FCTEMPS=//g'` MINTEMP=`egrep '^MINTEMP=.*$' $1 | sed -e 's/MINTEMP=//g'` MAXTEMP=`egrep '^MAXTEMP=.*$' $1 | sed -e 's/MAXTEMP=//g'` @@ -152,6 +154,57 @@ echo } +function DevicePath() +{ + if [ -d "$1/device" ] + then + readlink -f "$1/device" | sed -e 's/^\/sys\///' + fi +} + +function DeviceName() +{ + if [ -r "$1/name" ] + then + cat "$1/name" | sed -e 's/[[:space:]=]/_/g' + elif [ -r "$1/device/name" ] + then + cat "$1/device/name" | sed -e 's/[[:space:]=]/_/g' + fi +} + +function ValidateDevices() +{ + local OLD_DEVPATH="$1" OLD_DEVNAME="$2" outdated=0 + local entry device name path + + for entry in $OLD_DEVPATH + do + device=`echo "$entry" | sed -e 's/=[^=]*$//'` + path=`echo "$entry" | sed -e 's/^[^=]*=//'` + + if [ "`DevicePath "$device"`" != "$path" ] + then + echo "Device path of $device has changed" + outdated=1 + fi + done + + for entry in $OLD_DEVNAME + do + device=`echo "$entry" | sed -e 's/=[^=]*$//'` + name=`echo "$entry" | sed -e 's/^[^=]*=//'` + + if [ "`DeviceName "$device"`" != "$name" ] + then + echo "Device name of $device has changed" + outdated=1 + fi + done + + return $outdated +} + # Check that all referenced sysfs files exist function CheckFiles { local outdated=0 @@ -232,6 +285,17 @@ fi cd $DIR +# Check for configuration change +if [ -z "$DEVPATH" -o -z "$DEVNAME" ] +then + echo "Configuration is too old, please run pwmconfig again" + exit 1 +fi +if ! ValidateDevices "$DEVPATH" "$DEVNAME" +then + echo "Configuration appears to be outdated, please run pwmconfig again" + exit 1 +fi CheckFiles || exit 1 if [ -f "$PIDFILE" ] Index: prog/pwm/pwmconfig =================================================================== --- prog/pwm/pwmconfig (révision 5767) +++ prog/pwm/pwmconfig (copie de travail) @@ -539,6 +539,57 @@ exit 1 fi +function DevicePath() +{ + if [ -d "$1/device" ] + then + readlink -f "$1/device" | sed -e 's/^\/sys\///' + fi +} + +function DeviceName() +{ + if [ -r "$1/name" ] + then + cat "$1/name" | sed -e 's/[[:space:]=]/_/g' + elif [ -r "$1/device/name" ] + then + cat "$1/device/name" | sed -e 's/[[:space:]=]/_/g' + fi +} + +function ValidateDevices() +{ + local OLD_DEVPATH="$1" OLD_DEVNAME="$2" outdated=0 + local entry device name path + + for entry in $OLD_DEVPATH + do + device=`echo "$entry" | sed -e 's/=[^=]*$//'` + path=`echo "$entry" | sed -e 's/^[^=]*=//'` + + if [ "`DevicePath "$device"`" != "$path" ] + then + echo "Device path of $device has changed" + outdated=1 + fi + done + + for entry in $OLD_DEVNAME + do + device=`echo "$entry" | sed -e 's/=[^=]*$//'` + name=`echo "$entry" | sed -e 's/^[^=]*=//'` + + if [ "`DeviceName "$device"`" != "$name" ] + then + echo "Device name of $device has changed" + outdated=1 + fi + done + + return $outdated +} + function AskPath() { echo -n 'What should be the path to your fancontrol config file (/etc/fancontrol)? ' @@ -566,6 +617,8 @@ function LoadConfig() { + local OLD_DEVPATH OLD_DEVNAME + # Nothing to do if [ ! -f "$1" ] then @@ -575,6 +628,8 @@ echo "Loading configuration from $1 ..." INTERVAL=`egrep '^INTERVAL=.*$' $1 | sed -e 's/INTERVAL= *//g'` + OLD_DEVPATH=`egrep '^DEVPATH=.*$' $1 | sed -e 's/DEVPATH= *//g'` + OLD_DEVNAME=`egrep '^DEVNAME=.*$' $1 | sed -e 's/DEVNAME= *//g'` FCTEMPS=`egrep '^FCTEMPS=.*$' $1 | sed -e 's/FCTEMPS= *//g'` FCFANS=`egrep '^FCFANS=.*$' $1 | sed -e 's/FCFANS= *//g'` MINTEMP=`egrep '^MINTEMP=.*$' $1 | sed -e 's/MINTEMP= *//g'` @@ -585,16 +640,12 @@ MAXPWM=`egrep '^MAXPWM=.*$' $1 | sed -e 's/MAXPWM= *//g'` # Check for configuration change - local item - for item in $FCFANS - do - if [ ! -f "`echo $item | sed -e 's/=.*$//'`" ] - then - echo "Configuration appears to be outdated, discarded" - ClearConfig - return 0 - fi - done + if ! ValidateDevices "$OLD_DEVPATH" "$OLD_DEVNAME" + then + echo "Configuration appears to be outdated, discarded" + ClearConfig + return 0 + fi } LoadConfig $FCCONFIG @@ -675,14 +726,68 @@ echo "OK, using $fanval" } +# Remember the path and name of each device with at least one +# reference (pwm, temp or fan) in the configuration file. +# This function sets globals DEVPATH and DEVNAME as a side effect. +function RememberDevices() +{ + local used entry device name path tempfandev pwmdev + DEVPATH="" + DEVNAME="" + + for device in $DEVICES + do + device=`echo "$device" | sed -e 's/\/.*$//'` + + used=0 + for entry in $1 $2 + do + pwmdev=`echo "$entry" | sed -e 's/\/.*$//'` + tempfandev=`echo "$entry" | sed -e 's/^[^=]*=//' -e 's/\/.*$//'` + + if [ "$device" = "$pwmdev" -o "$device" = "$tempfandev" ] + then + used=1 + fi + done + if [ "$used" -eq 0 ] + then + continue + fi + + # Record the device path and name. This lets the fancontrol + # script check that they didn't change. If they did, then the + # configuration file can no longer be trusted. + path=`DevicePath "$device"` + if [ -z "$DEVPATH" ] + then + DEVPATH="$device=$path" + else + DEVPATH="$DEVPATH $device=$path" + fi + + name=`DeviceName "$device"` + if [ -z "$DEVNAME" ] + then + DEVNAME="$device=$name" + else + DEVNAME="$DEVNAME $device=$name" + fi + done +} + function SaveConfig() { + RememberDevices "$FCTEMPS" "$FCFANS" + echo echo "Saving configuration to $FCCONFIG..." tmpfile=`mktemp -t pwmcfg.XXXXXXXXXX` || { echo "$0: Cannot create temporary file" >&2; exit 1; } trap " [ -f \"$tmpfile\" ] && /bin/rm -f -- \"$tmpfile\"" 0 1 2 3 13 15 echo "# Configuration file generated by pwmconfig, changes will be lost" >$tmpfile echo "INTERVAL=$INTERVAL" >>$tmpfile + echo "DEVPATH=$DEVPATH" >>$tmpfile + echo "DEVNAME=$DEVNAME" >>$tmpfile echo "FCTEMPS=$FCTEMPS" >>$tmpfile echo "FCFANS=$FCFANS" >>$tmpfile echo "MINTEMP=$MINTEMP" >>$tmpfile Index: prog/pwm/fancontrol.8 =================================================================== --- prog/pwm/fancontrol.8 (révision 5765) +++ prog/pwm/fancontrol.8 (copie de travail) @@ -92,9 +92,11 @@ setup I recommend using the \fBpwmconfig\fP script. Small changes can be made by editing the config file directly following the rules above. -Upon starting, fancontrol will make sure that all referenced sysfs files -do exist. If not, it will quit immediately, upon the assumption that the -configuration file may be out-of-sync with the loaded kernel drivers. +Upon starting, fancontrol will make sure that all referenced devices +do exist and match what they were at configuration time, and that all +referenced sysfs files do exist. If not, it will quit immediately, upon +the assumption that the configuration file may be out-of-sync with the +loaded kernel drivers. .SH THE ALGORITHM -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
