Page MenuHomeFreeBSD

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

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

Details

Summary

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

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

Event Timeline

delphij created this revision.Sat, Nov 30, 9:15 AM
markm accepted this revision.Sat, Nov 30, 9:21 AM
markm added a subscriber: markm.

LGTM

This revision is now accepted and ready to land.Sat, Nov 30, 9:21 AM
cem requested changes to this revision.Sat, Nov 30, 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.

libexec/save-entropy/save-entropy.sh
68 ↗(On Diff #65067)

We are in entropy_dir from here on

85 ↗(On Diff #65067)

-le?

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.Sat, Nov 30, 5:03 PM
delphij marked 4 inline comments as done.Sat, Nov 30, 6:27 PM

Thanks for the review, please take another look.

libexec/save-entropy/save-entropy.sh
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 updated this revision to Diff 65075.Sat, Nov 30, 6:27 PM
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).

cem accepted this revision.Sat, Nov 30, 7:51 PM

LGTM, thanks!

This revision is now accepted and ready to land.Sat, Nov 30, 7:51 PM
This revision was automatically updated to reflect the committed changes.