Skip to content
This repository has been archived by the owner on Apr 7, 2021. It is now read-only.

Fix not being able to start service because temperature not yet available #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

y0ast
Copy link

@y0ast y0ast commented May 3, 2020

sometimes it takes a while for the temperature of the gpu to become available, leading to

FILE_TEMP=$(echo /sys/class/drm/card0/device/hwmon/hwmon?/temp1_input)

being empty and the service failing with "invalid hwmon files".

I tried to look for a valid service to start after, but it doesn't seem to exist. Alternative to this solution there's a few others:

  • Restart (with 5 sec delays) until service starts successfully.
  • Add a path watcher for /sys/class/drm/card0/device/hwmon/hwmon?/temp1_input

The first could lead to an infinite restart loop, the second muddles logic from the amdgpu_control script and the service.

Open to suggestions. The fix suggested in #25 is not sufficient for me, but this works.

fixes #25

sometimes it takes a while for the temperature of the gpu to become available, leading to
```
FILE_TEMP=$(echo /sys/class/drm/card0/device/hwmon/hwmon?/temp1_input)
```
being empty and the service failing with "invalid hwmon files".

I tried to look for a valid service to start after, but it doesn't seem to exist. Alternative to this solution there's a few others:

- Restart (with 5 sec delays) until service starts successfully.
- Add a path watcher for `/sys/class/drm/card0/device/hwmon/hwmon?/temp1_input`

The first could lead to an infinite restart loop, the second muddles logic from the `amdgpu_control` script and the service.
@y0ast y0ast changed the title Fix not being able to start because driver unavailable (fix #25) Fix not being able to start service because temperature not yet available (fix #25) May 3, 2020
@grmat
Copy link
Owner

grmat commented May 25, 2020

Hi, thanks for reaching out.

I'm not going to merge a sleep patch, but keep this one open for documentation purposes for now, it might help people after all. I'll add some additional thoughts in #25

@y0ast y0ast changed the title Fix not being able to start service because temperature not yet available (fix #25) Fix not being able to start service because temperature not yet available Jun 7, 2020
@y0ast
Copy link
Author

y0ast commented Jun 7, 2020

I've now updated it to just do a restart on failure. Would you be open to merging that?

Despite your intention of this code being only a demo script, it is now directly mentioned in the Arch wiki: https://wiki.archlinux.org/index.php/Fan_speed_control#fancurve_script

Therefore it would be nice if the default setup at least starts on most machines.

Copy link

@mik13ST mik13ST left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using StartLimitIntervalSec with the StartLimitBurst directive to prevent infinite restart loop. See man systemd.unit. I don't know what values to choose for these two directives. It might be necessary to make some measurements of when the hwmon files are accessible.

It still wouldn't as good as waiting for the file to exist though.

Yes, I am facing the same issue.


[Install]
WantedBy=multi-user.target
WantedBy=default.target
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary? Setting the restart properties does not require changing the WantedBy property, right? The ArchWiki says default.target is usually a symlink to graphical.target which is different from multi-user.target.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe default.target is a bit more common for these type of services. Happy to change it back if @grmat expresses interest in merging this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Systemd service error
3 participants