pwd_mkdb opens files with O_SYNC instead of calling fsync

[dwmalone]

Summary

The right fix for this shouldn't be the use of O_SYNC, but calling
fsync before the files are moved into place (plus calling fsync on
the directory, which is a good idea).

In fact, pwd_mkdb calls the close method on the db, which the man
page for dbopen claims should flush the file. However, looking at
the sync and close methods in lib/libc/db/hash, it seems the code
for flushing is missing, which is probably the real bug here.

[se]

I had assumed that buffers are flushed before the file is closed (there is a comment to that effect in hash.c:hdestroy() but I have not looked at this code in detail).

In my tests, the db file came out identical with or without O_SYNC.

BTW: There is one further command that uses dbopen(..., O_SYNC) and can be sped up by a significant factor (25.5 seconds --> 0.6 seconds in my tests with the full termcap file) and that command is cap_mkdb.

ARC does not like me and since its a trivial change I just point you at src/usr.bin/cap_mkdb/cap_mkdb.c line 122. Please update the diff to include the removal of O_SYNC from that file to.

[bapt]

looks ok to me, and from my testing it does the job. @garga can you confirm that is also fixes the issue you found?

[vangyzen]

lib/libc/db/hash/hash.c

426 ↗

(On Diff #12999)

This is good, but you might consider calling _fsync() at the end of flush_meta(), instead of duplicating the call in these two places.

[garga]

I've committed it to pfSense and we will run batch scripts to validate if the original issue is not back. I'll return ASAP

[garga]

After over 1000 power cycles we were not able to reproduce original bug.

[vangyzen]

Please get this into 10.3.

[dwmalone]

Great - I'll commit it in the next day or two, and ask for permission to MFC.

[AMDmi3]

Both fsync and close may return an error (EIO). I think it'd be nice to check for it here as well.