Page MenuHomeFreeBSD

Reduce disk write load in /usr/libexec/save-entropy

Authored by delphij on Nov 30 2019, 9:15 AM.



The save-entropy script rotates entropy files like logs,
but the entropy data do not really need to be kept in a particular
order, and replacing the oldest file is sufficient.

The proposed change replaces the rotation with a scan in the
[1..entropy_save_num) space that finds the first empty slot,
or the slot of oldest file, and overwrites the file in that slot.

This also fixes an issue that prevents save-entropy from saving
any entropy when there is one non-regular file in any slot as a
side effect.

PR: 134225

Test Plan

Test the script with empty, more than entropy_save_num,
sparse slot in /var/db/entropy as well as covering one to many slots
with directories for the non-regular file case.

Diff Detail

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This revision is now accepted and ready to land.Nov 30 2019, 9:21 AM
cem requested changes to this revision.Nov 30 2019, 5:03 PM

Mostly lgtm! Thanks for taking this 2009 bug. I may not understand correctly, but I believe the loop condition tagged above is off by one.

68 ↗(On Diff #65067)

We are in entropy_dir from here on

85 ↗(On Diff #65067)


In the byzantine case where [1, n-1] exist and are special files, but n is either empty or a regular file, save_file will not be initialized and the logic below (part 3) will not work, I think.

86 ↗(On Diff #65067)

${entropy_dir}/ can be elided here and below.

130 ↗(On Diff #65067)

“.” is still entropy_dir, although I am fine with making it explicit here.

This revision now requires changes to proceed.Nov 30 2019, 5:03 PM

Thanks for the review, please take another look.

85 ↗(On Diff #65067)

Good catch, the original intention was to have scan to up to (n-1), and have it handled in the second pass, but with the -f bypass, this would cause the script to erroneously skip when [1..n-1] are all non-regular files, even when n was not.

delphij marked an inline comment as done.

Fix issues pointed out by cem@

(Note that the new revision also changed permission from 400 r-------- to 600 rw-------, and umask from 377 to 177, because the non-root user needs write permission to modify the inode).

This revision is now accepted and ready to land.Nov 30 2019, 7:51 PM