This patch fixes (at least) the following race conditions:
1. If a device is attached while iterating assigned devices, the
MonitorObserver thread can attempt to insert the newly attached
device to the assigned_devices set while the set is being iterated,
which results in the following exception:
RuntimeError: Set changed size during iteration
2. Some devices can be missed when applying a tuning - devices are enumerated
(i.e., Plugin._init_devices() gets called) before udev device monitoring is
started (hotplug.Plugin._hardware_events_init() gets called), so devices
that appear between these two actions are not tuned.
3. Device monitoring is stopped too late, which can result in some tunings
not being unapplied after stopping a profile. This can happen for devices
that get added during profile rollback after unit_manager.stop_tuning()
gets called, but before unit_manager.destroy_all() gets called.
4. It can happen that tuning is applied twice for a device if it is added
during profile activation, e.g. after unit_manager.create() is called in
Daemon._thread_code(), but before unit_manager.start_tuning() is called.
Apart from unnecessarily applying the tuning twice, it can result in
overwriting saved original settings for the device and hence our inability
to properly roll back our changes to the settings.
5. The observer thread can attempt to use load_monitor before it's created
in Plugin._instance_init(), which can result in AttributeError.
Hopefully it doesn't introduce new race conditions :).
The fix is to:
1. rearrange the sequence of certain actions,
2. separate Instance.devices to two separate sets: processed_devices
and assigned_devices.
processed_devices are never iterated when the MonitorObserver thread
is running (*), so the first problem described above cannot happen.
The set is used to store devices, which have already been tuned.
The assigned_devices set is now the set of devices that are going
to be tuned. The set can only be accessed by the main thread.
(*) Except when verifying tuning - this is fixed in a follow-up patch
I tried to separate the changes into more digestable patches, but I
couldn't figure out how.
Resolves: rhbz#1592743
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
If a device, namely a disk, is hotplugged after tuned is started, it
won't be present in Monitor._available_devices. Consequently, it won't
get added to the monitor and so dynamic tuning won't work for the
device. Let's fix that by refreshing the list of available devices when
adding a device to a monitor.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
_remove_unused_filters() is not used anywhere and the udev's
remove_filter() is broken anyway:
https://github.com/systemd/systemd/issues/11529
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
Hiding the full stack trace in the debug output is not useful - it hides
information that would be useful for us when diagnosing a failure. It's
not always possible (and it's certainly an unnecessary burden for the
users) to later reproduce the issue with debug mode on. As a bonus, the
traceback will now stand out in the log, increasing the likelihood
that it will get noticed and reported.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
Since commit cd4943bdbf, it is recommended to use ${i:PROFILE_DIR}
in paths to scripts, in order to make the paths absolute. So let's make
the profile generated by powertop2tuned conform to that recommendation.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
Since commit cd4943bdbf, it is recommended to use ${i:PROFILE_DIR}
in paths to scripts, in order to make the paths absolute. It's not
required (yet) though.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
In order to avoid the overhead of executing 'sysctl --system', sysctl
settings from sysctl configuration files are now applied by directly
writing to /proc/sys.
Fixes#146
Resolves: rhbz#1663412
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
Previously it could (theoretically) happen that reading the original
value of a setting fails, but then a new value is successfully written.
This would result in Tuned not being able to restore the original value
of the setting.
Even though I don't know of a situation where this could happen in
practice, the code was logically wrong, so let's fix it.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
By limiting the number of bound methods and using regular functions
instead of methods where possible, we make the code easier to reason
about. For example, now we can be sure that _get_sysctl_path,
_read_sysctl and _write_sysctl won't touch attributes of SysctlPlugin.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
Set individual sysctl parameters by directly writing to /proc/sys in
order to avoid the overhead of executing the 'sysctl' program for each
setting.
We originally thought about rewriting the functionality using procfs.sysctl
from python-procfs, however I decided to implement it directly, because
it's trivial and because the error handling in procfs.sysctl is not
sufficient for our needs.
The list of deprecated sysctl settings was taken from the sysctl program
from procps-ng.
Fixes#146
Resolves: rhbz#1663412
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
kernel.sched_migration_cost_ns=5000000 was being defined here even though it was defined in the included latency-performance profile. Removed to eliminate redundancy.
The tests are now in a good shape, so we can now run pylint on them
without getting a huge list of errors. In fact, we should lint them
to decrease the likelihood they'll go stale again.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
Since python3.3, Mapping has been moved to collections.abc and starting
with python3.8, it will no longer be available as collections.Mapping.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
Tested on an AMD machine with acpi-cpufreq kernel driver providing
ondemand, conservative,... governors.
Without this line, one gets:
tuned.plugins.plugin_cpu: ignoring governor 'conservative' on cpu 'cpu7', it
is not supported
when switching to balanced profile.
With this line you see:
tuned.plugins.plugin_cpu: setting governor 'ondemand' on cpu 'cpu2'
Be careful: Order matters!
tuned seem to read profile config file line by line or section by section
and directly tries to apply things.
If order of these lines are exchanged:
[modules]
cpufreq_conservative=+r
[cpu]
governor=conservative
you fall back to previous error again. Therefore the newly introduced
[modules] section is not put at the end, but nearly at top.
This is an attempt to formalize the process of contributing to Tuned
and formulate what is expected from contributors. From now on, all
contributors are expected to follow the guidelines.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
This avoids wrong warning about a missing intel tool (x86_energy_perf_bias)
Remember arch and x86 CPU vendor to allow arch/vendor specific CPU tunings
in the future
The 'removable' attribute is a bytestring, so it will never be equal to
"0" in python3. Check equality with b"0" instead.
The patch was originally written by Tomáš Korbař.
Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
