Avoid unnecessary inter-processor interrupts by checking that writes are necessary

[zacikpa]

This PR adds a new option check_diff to the function commands.write_to_file. When set to True, the function checks whether the current value is equal to the one being written - if it is, the write is skipped.

The new option is disabled by default, but I enable it for plugin options where the write may cause an inter-processor interrupt even when the value does not change.

[jmencak]

Thank you for the PR. I'm wondering why do we need a new option for this and why this is not a default? cc @MarSik

Edit: Sorry, my bad. Looking at the code this seems fine. I thought we're adding a new option to tuned-main.conf

[jmencak]

Thank you for the PR, @zacikpa . Testing this right now. So far, I've noticed the following issues. When the same value is in the governor, we still write the same value. The check_diff seems to be False. Another issue I've noticed is that we'll need to change the TuneD configs. There are some aliases like "performance" when written to /sys/devices/system/cpu/cpuX/power/energy_perf_bias will be translated to 0. So our configuration, such as

energy_perf_bias=performance

will have to be changed to

energy_perf_bias=0

so that we take advantage of this new functionality.

Anyways, could you please look into the scaling_governor issue? Thank you.

[jmencak]

Also, running

x86_energy_perf_policy -r

causes quite a few IPIs. This is pretty "expensive" if we're just checking that x86_energy_perf_policy is supported by TuneD and then we never use it anyway. I wonder if we could just check for the presence of the file and whether it is executable?

[jmencak]

I've finished testing the code. In summary, I see these issues that I'd like to see fixed before merging:

• setting the same scaling_governor still triggers IPIs
• let's remove x86_energy_perf_policy -r check, which causes unnecessary IPIs

Issues that I do not expected fixed by this PR:

• [rtentsk] plug-in
• even reading EPB/EPP causes IPIs

As even reading

/sys/devices/system/cpu/cpu*/power/energy_perf_bias
/sys/devices/system/cpu/cpufreq/policy*/energy_performance_preference

can cause IPIs, this change could essentially make things worse IPI-wise when writing a different value of EPB/EPP. The first IPI would happen during the read, the second during the write. I wonder if it would make sense to exclude the diff_check (set it False) for these two knobs.

FWIW, reads/writes to EPP seem to be "expensive" in mostly the same way, whereas reads of EPB are less expensive than writes to EPB.

[zacikpa]

Hello @jmencak. I updated the PR so that:

• check_diff is True also for the scaling_governor,
• x86_energy_perf_policy is only checked for if we may actually need it, i.e., if we cannot set EPB via sysfs.

Regarding the reads: if reading EPP is as expensive as writing, then I'd say it doesn't make sense to check the value first.

[zacikpa]

@jmencak Regarding the aliases for EPB: can you please open a separate upstream or Jira issue for that?

[jmencak]

Thank you for the changes, Pavol. Great work! The code now does what I expect it to do. After testing, I can only see IPIs caused by the following options:

[rtentsk]
[cpu]
energy_perf_bias=<value>
energy_performance_preference=<value>

At this point this is expected and hard to avoid/work around.

The code changes look sane to me, but deferring to other reviewers for comment.

[yarda]

Thanks, the code LGTM. I have two minor comments that aren't blockers and could be handled later. I think this PR needs to be merged into FDP and we have still some time to fine tune it, so I will keep it open for now.

[jmencak]

I've retested with the latest code, still looks good.