Am 20.02.2016 um 03:31 schrieb Eric van Gyzen: > On 2/19/16 6:40 PM, Bryan Drewery wrote: >> On 2/19/2016 12:42 AM, Stefan Esser wrote: >>> Author: se >>> Date: Fri Feb 19 08:42:13 2016 >>> New Revision: 295800 >>> URL: https://svnweb.freebsd.org/changeset/base/295800 >>> >>> Log: >>> Remove O_SYNC from the options passed to dbmopen(). >> Uh, this is a full revert of r293312's changes to cap_mkdb which were >> made for good reason. So this seems simply wrong without a better fix. >> >>> The output file is created as a temporary file that is moved >>> over the >>> existing file after completion. Thus there is no need to immediately >>> flush all created db records to the temporary file. >> This is not right either. Depending on the use of soft updates / >> journaling the data and metadata (file name / rename) may be written at >> different times. It is entirely possible to get a renamed file with no >> or junk content without an fsync. That's exactly what r293312 mentions >> in its commit message.
I had performed multiple tests and found, that the file was always created identical whether with or without O_SYNC. But I've got to admit, that I did not check the SVN log. After reading the commit log entry for r293312, I understand there is the risk of an empty db file if a system crash happens before all data and metadata has been flushed. I had assumed that one of the guarantees soft-updates makes is that data and meta-data operations are ordered relative to each other. I'm not sure, whether this is a hypothetical case, or whether such a case had been observed in reality, but I understand that you don't want to take risk when operating on critical system files. O_SYNC was first added to updates of the password db, and I think that was the file with the biggest risk of accidental damage due to a crash. But services_mkdb and cap_mkdb are only invoked by the administrator after the text files are modified and it is highly unlikely (but of course not impossible) that a crash occurs at just that moment. > dwmalone@ plans to put the fsync() in the db close method, which makes a > lot of sense, and would fix this in a better way. > > https://reviews.freebsd.org/D5186 > > This commit probably should have waited for D5186 to be committed, but > at least that seems imminent. Yes, I mentioned review D5186 in the commit log and was aware, that there was consensus that dwmalone's change to hash.c is about to be committed. I was afraid that D5186 could be committed without the removal of O_SYNC and that D5186 might be merged to 10.3, but that cap_mkdb kept O_SYNC and would be slow throughout the lifetime of 10.3. So the commit was expected to be followed by the fsync() patch in hash.c very soon and I had mentioned, that the MFC of the mkdb patch should be performed in conjunction (or after) the MFC of D5186. I think it is important that the mkdb utilities do not take tens of seconds and up to several minutes in 10.3 ... Regards, STefan _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
