Bug 187172

Summary: truncated configuration files on power loss or hard crash
Product: [Frameworks and Libraries] frameworks-kconfig Reporter: Martin Steigerwald <Martin>
Component: generalAssignee: Matthew Dawson <matthew>
Status: REOPENED ---    
Severity: normal CC: arekm, aseigo, christoph, jdmulloy, kai, kdelibs-bugs, l.lunak, lure, mcamen, mcepl, mpyne, ms-kdebugs, octoploid, tytso
Priority: NOR    
Version: 6.3.0   
Target Milestone: ---   
Platform: Debian testing   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Martin Steigerwald 2009-03-14 20:40:34 UTC
Version:            (using KDE 3.5.10)
OS:                Linux
Installed from:    Debian testing/unstable Packages

This is no crash, but IMHO it is more severe than a bug, cause data loss is involved. Leaving as bug nonetheless.

I put this on ksycoca, since it has to do with configuration files, but also other files in general. This might not the best assignment. Feel free to change as necessary. It happend to me with BasKet as well, so its not just configuration files. This issue seems to be pretty generic. But now start:

It happens with KDE configuration files and also in several other KDE applications - I should file different bug reports for them probably - that files are truncated on power loss or hard crash. This happens especially when power loss or crash is during desktop initialization.

I had this with on Linux kernel 2.6.28, 2.6.27.7 and after I tested also some earlier kernels and an XFS filesystem.

http://oss.sgi.com/archives/xfs/2008-12/msg00540.html

But it is not related to XFS only. It has been reported for Ext4 as well:

http://oss.sgi.com/archives/xfs/2009-03/msg00072.html

which links to

http://lwn.net/Articles/322823/

which is currently subscriber only. The posting contains a link that works. LWN might not be to happy about this. I just use the official link here.

Heise mentions it too:
http://www.heise.de/english/newsticker/news/134483

and mentions a Ubuntu bug report which is about KDE 4 on Ext4:

https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/317781

Conclusion of various filesystem developers including seems to be what  Theodore Ts'o posted as a response to this bug report:

https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/317781/comments/45

Following this modern filesystems do not make any guarentees as to when data lands on the disk. Especially when delayed allocation is used.

Thus applications should use fsync / fdatasync to make sure data is stored on disk.

What I still wonder tough: It does appear to me that KDE writes to many configuration files on desktop initialization. Exactly why I don't get tough. It seems to write certain files almost periodically without me making changes to the configuration.

Since this does not only happen with configuration files, maybe it should be wise to make a superbug that links to sub bugs for different applications. I could add BasKet there and after looking at my reports possibly other KDE applications.
Comment 1 Martin Steigerwald 2009-03-14 20:47:22 UTC
Problem appears partly that application writers relied on the following behavior of ext3, citing Theodore Ts'o:

"So, why wasn't this a problem before in the past? Well, ext3 by default has a commit interval of 5 seconds, and has data=ordered. What does this mean? Well, every 5 seconds, the ext3 journal is committed; this means that any changes in since the last commit are now guaranteed to survive an unclean shutdown. The journalling mode data=ordered means that only metadata is written in the journal, but data is ordered; this means that before the commit takes place, any data blocks are associated with inodes that are about to be committed in that transaction will be forced out to disk. This is primarily done for security reasons; if this was not done, then any newly allocated blocks might still contain previous data belonging to some other file or user [...]"

I recommend reading his complete comment:

https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/317781/comments/45
Comment 2 Martin Steigerwald 2009-03-14 20:54:50 UTC
Sorry for another comment, but I think it should be mentioned here that the issue appears to be related to delayed allocation which has been implemented in ext4. XFS used delayed allocation even before ext4 had it.
Comment 3 Martin Steigerwald 2009-03-14 22:53:11 UTC
Thanks Pino for reassigning it. Didn't see "general" in the bug report wizard.

I want to add some additional info from Eric Sandeen about KDE_EXTRA_FSYNC environment variable. I set this in my .zshrc and see how it works. I think I would need to switch the notebook off hardly in order to see whether it helps. Not today anymore tough.

---------------------------------------------
LWN article: ext4 and data loss
Eric Sandeen sandeen at sandeen.net 
 Sat Mar 14 15:02:00 CDT 2009 

Perhaps you can try this and add info if you like.

There is an environment variable KDE_EXTRA_FSYNC, put in with this
changelog:

Do not paranoidly sync every time, it causes I/O performance problems
for some users. People who still want it for whatever reason like using
XFS can set $KDE_EXTRA_FSYNC to 1."

This is not "extra" - it is necessary if you actually want the data to
be on-disk.

See also: http://lists.kde.org/?l=kde-devel&m=120880682813170&w=2

(note however that xfs is _not_ "zeroing just to be sure...")

http://api.kde.org/4.x-api/kdelibs-apidocs/kde3support/html/k3tempfile_8cpp-source.html

 bool
 K3TempFile::sync()
    int result = 0;

    if (mStream)
    {
       do {
          result = fflush(mStream); // We need to flush first otherwise
fsync may not have our data
       }
       while ((result == -1) && (errno == EINTR));

       if (result)
       {
          kWarning() << "K3TempFile: Error trying to flush " << mTmpName
 " << strerror(errno);
          mError = errno;
       }
    }

    if (mFd >= 0)
    {
       if( qgetenv( "KDE_EXTRA_FSYNC" ) == "1" )
       {
          result = FDATASYNC(mFd);
          if (result)
          {
             kWarning() << "K3TempFile: Error trying to sync " <<
mTmpName << ": " << strerror(errno);
             mError = errno;
          }
       }
    }

    return (mError == 0);
 }

So somebody knew it was the right thing to do, but then turned it off,
probably because of ext3's painful behavior on fsync (see also: firefox
"bug" from a while ago)

what's interesting is that the sync() method isn't automatically called
from the other methods, near as I can tell, so it's up to the api user
to invoke it; and even then it only does fflush() not fsync() by
default, which doesn't actually push things to disk.

I'd suggest turning on this KDE_EXTRA_FSYNC and see how things go from
there on.

Although, the API refs say that this is deprecated in favor of
KTemporaryFile, and I find no reference whatsoever to "sync" of any kind
in ktemporaryfile.cpp.

-Eric
---------------------------------------------


Link to the post:

http://oss.sgi.com/pipermail/xfs/2009-March/040628.html


And another information from him:

---------------------------------------------
... getting OT sorry but in case anyone else is interested,
http://api.kde.org/4.x-api/kdelibs-apidocs/kdecore/html/classKSaveFile.html
also seems relevant.

I'd like to know just what KDE is doing here, so digging a little.

-Eric
---------------------------------------------


Link to post:
http://oss.sgi.com/pipermail/xfs/2009-March/040629.html
Comment 4 Michael Pyne 2009-03-14 23:20:02 UTC
Well since we're posting hyperlinks I'll go ahead and link to
http://mjg59.livejournal.com/108257.html which actually mostly describes how I
feel as well.

Basically either way we're screwed, either we have horrible performance on
ext3, which most Linux users use, or we lose data on high-performance
filesystems like xfs (and now ext4) which people apparently shouldn't be using
for desktops.

However it looks like the EXTRA_FSYNC stuff only applies to kde3support code,
it appears to me that it should be ported over to the current KDE 4 equivalent
code (KTemporaryFile and KSaveFile at least, and probably a couple of KConfig
backends).

And maybe Solid can detect what type of filesystem a file is on and set
EXTRA_FSYNC either on or off by default.

But I disagree with just defaulting it on because right now that is going to
give undesirable behavior to most Linux users, and no one is forcing people to use xfs or ext4... :(
Comment 5 Kai Krakow 2009-03-15 21:44:55 UTC
> But I disagree with just defaulting it on because right now that is going to
> give undesirable behavior to most Linux users, and no one is forcing people to
> use xfs or ext4... :(

Well I think this is not exactly true. From all what I read we could now say that Ext3 has the "broken behaviour". You could have said "no one is forcing people to use ext3" when it was introduced. But anyone used it! Why? Because it was told to be good, and well tested, and based on "the best filesystem ever". Bla bla. It then came as default with every distribution. Well, there are some exceptions: SuSE used ReiserFS - and they were hated for it sometimes. Probably due to some moral conflicts they switched to Ext3.

The same will happen to Ext4 now: Any distribution will use it as default (e.g. forthcoming Ubuntu) or already use it (Fedora). How many people choose another filesystem during installation? When someone does, he is a pro. Probably no newbie will even know about this possibility. And a newbie using this "all-fantastic-because-it's-based-on-ext3-which-is-superior-because-it's-based-on-ext2" filesystem (even not knowing about these problems) will suddenly say: "Shitty KDE - looses my settings on every crash." Or "Shitty linux...". Just because Theodore Tso's filesystems and distributors put application writers into a dilemma now. New installations will have Ext4, olders stay with Ext3. What to do?

The best way would be to detect what filesystem is run and use the right method to store/exchange files. So go with fsync() on Ext4 and XFS and non-journalled filesystems. And don't use fsync() on the others (or even don't use on Ext3?). This list should probably be evaluated more (Reiser4, ReiserFS, NFS, JFS...).
Comment 6 Theodore Ts'o 2009-03-16 07:41:33 UTC
Please see: http://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync

Running fsync() isn't that bad even on ext3; just do it on a separate thread and don't do it while running your main UI thread --- that was the cause of the FireFox 3.0 problems.
Comment 7 Martin Steigerwald 2009-03-16 10:49:36 UTC
This appears to have several aspects:

1) fsync is not called. A behavior which also on Ext3 is not really safe.

2) KDE seems to be writing out configuration files regularily. I found that with kdeglobals, but also with other files. Is this actually needed in all of the time? Is there room for batching or saving less often? Note, this would not remedy aspect 1.

3) Especially during starting / desktop initialization KDE massively seems to rewrite configuration files. With 2.6.28 and 2.6.27 and also some earlier kernels I have able to produce *massive* data loss when switching of the machine shortly after desktop initialization. Losts of files were truncated. Why KDE appears to massively rewrite configuration files during startup? Shouldn't it just *read* those?

4) Not only configuration files are affected. At least BasKet and also KWallet is affected to.

I think this is showing a *fundamental* problem in how applications write files. 

It gets even worse with Mac OS X, which doesn't guarentee data is on disk with fsync(), but I consider this a bug even if POSIX doesn't. POSIX does allow fsync() to be a NULL function. Thus KDE on Mac OS X needs to have special measures.

Some general overview of all of this is in the "Eat My Data" presentation:

http://www.flamingsport.com/talks/2007/06/eat_my_data.odp

I'd like to point out some more information from Theodore Ts'o. First off

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/317781/comments/45
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/317781/comments/50

on why this is more of an issue with ext4 and XFS and any filesystem that does delayed allocation.

And then about different implementations on saving files:

-------------------------------------------------
OK, so let me explain what's going on a bit more explicitly. There are application programmers who are rewriting application files like this:
1.a) open and read file ~/.kde/foo/bar/baz
 1.b) fd = open("~/.kde/foo/bar/baz", O_WRONLY|O_TRUNC|O_CREAT) --- this truncates the file
 1.c) write(fd, buf-of-new-contents-of-file, size-of-new-contents-of-file)
 1.d) close(fd)
Slightly more sophisticated application writers will do this:
2.a) open and read file ~/.kde/foo/bar/baz
 2.b) fd = open("~/.kde/foo/bar/baz.new", O_WRONLY|O_TRUNC|O_CREAT)
 2.c) write(fd, buf-of-new-contents-of-file, size-of-new-contents-of-file)
 2.d) close(fd)
 2.e) rename("~/.kde/foo/bar/baz.new", "~/.kde/foo/bar/baz")
What emacs (and very sophisticated, careful application writers) will do is this:
3.a) open and read file ~/.kde/foo/bar/baz
 3.b) fd = open("~/.kde/foo/bar/baz.new", O_WRONLY|O_TRUNC|O_CREAT)
 3.c) write(fd, buf-of-new-contents-of-file, size-of-new-contents-of-file)
 3.d) fsync(fd) --- and check the error return from the fsync
 3.e) close(fd)
 3.f) rename("~/.kde/foo/bar/baz", "~/.kde/foo/bar/baz~") --- this is optional
 3.g) rename("~/.kde/foo/bar/baz.new", "~/.kde/foo/bar/baz")
The fact that series (1) and (2) works at all is an accident. Ext3 in its default configuration happens to have the property that 5 seconds after (1) and (2) completes, the data is safely on disk. (3) is the ***only*** thing which is guaranteed not to lose data. For example, if you are using laptop mode, the 5 seconds is extended to 30 seconds.
Now the one downside with (3) is that fsync() is a heavyweight operation. If your application is stupid, and has hundreds of dot files in your home directory, each one taking up a 4k disk block even though it is only storing 4 to 12 bytes of data in each singleton dot file, and you have to repeat (3) for each of your one hundred dot files --- and worse yet, your application for some stupid, unknown reason is writing all of these hundred+ dot files every few seconds, then (3) will be very painful. But it is painful because the application is stupidly written --- not for any fundamental filesystem fault.
[...]
-------------------------------------------------
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/317781/comments/54

I do not know how to best tackle this. But making sure data is safe - and either an older or the current version is on disk, instead of having lost not even the current but also the previous version - is really important to make users happy.

Maybe KDE should kdelibs should provide some standard functions not only for configuration files but also for applications which want to save files, but then applications use libxml and whatnot. So this may require further bug reports. I am thinking of adding bug reports for KWallet and BasKet at least.
Comment 8 Martin Steigerwald 2009-03-16 10:58:48 UTC
Some hints for users wanting to preserve data now - before this issues are resolved properly:

# KDE Sync
# http://oss.sgi.com/pipermail/xfs/2009-March/040628.html
export KDE_EXTRA_FSYNC=1

in your shell startup file. Hopefully that really does work. I did not yet test it by hardly switching of the computer.


For hibernate via hibernate script and TuxOnIce which with some combination of Linux Kernel and X.org driver crashed on some occassions - it appeared to be a hard lock during switching from X11 to console:

shambhala:~> cat /etc/acpi/hibernate-extra.sh
#!/bin/sh

sync
/etc/acpi/hibernate.sh

(TuxOnIce does sync filesystems, but this came to late - the lockup was before that)


I also did this in times when I had those crashes - after having had lost files for several times:
martin@shambhala:~> fcrontab -l
10:53:00 listing martin's fcrontab
@ 12h /usr/bin/rdiff-backup ~/.kde/ ~/Backup/KDE-Konfiguration ; sync
Comment 9 Martin Steigerwald 2009-03-16 15:19:22 UTC
As often there are different viewpoints:

Matthew Garrett differentiates between

1) open("foo", O_TRUNC), write(), close()
2) open("foo.tmp"), write(), close() and then rename("foo.tmp", "foo")

and thinks that its okay when the first approach fails, but the second should work. Especially as its not the same as

3) open("foo.tmp"), write(), fsync(), close() and then rename("foo.tmp", "foo")

The application would be fine if either the old or the new state is there after a crash - which it tries to achieve by the second approach. But the application doesn't not require ultimately that the new data is there like the third approach - aside from on disk caches and on controller caches - does guarantee.

And so it would be fine to have the rename() happen after the date of the file to be renamend is written. Thats how ext4 in 2.6.30 will be doing it. And it will also skip delayed allocation when a truncated file is closed.

Users can mount ext4 with nodelalloc as a work-around until then.

XFS only skips delayed allocation when a truncated file is closed, but not when a renamed file is closed.

See more:

ext4, application expectations and power management
http://mjg59.livejournal.com/108257.html

Well actually I can understand both viewpoints.
Comment 10 Oswald Buddenhagen 2009-03-17 00:17:04 UTC
speaking as someone who has dealt with this in the past and just read "the post" ...
ted makes some rather unrealistic assumptions and thus comes to useless conclusions by all practical means. most of my own concerns are already raised in the comments over there, so i won't bother.
it's a no-win situation. we can make KDE_EXTRA_FSYNC work again and even add some heuristics to auto-enable it with some configurations, but that's an utter crutch.
the threaded solution is insanely complex; we'd end up with an own daemon which maintains consistency between processes.
Comment 11 Michael Pyne 2009-03-17 00:57:45 UTC
In response to comment 7:

1: there's reasons that KDE doesn't use fsync as much: see bug 110318 (which is what I assume Oswald is referring to).

2: much related is the fact that no, KDE probably doesn't need to write to disk nearly so much as it does in KConfig, which is probably not too difficult to add a flag that only writes to disk when a KConfig object is destroyed if the configuration has changed.

3: this is simply #2 over again, as KDE loads up the process creation/destruction sequence will involve lots of KConfig objects being created and destroyed.  The configuration is not being "re"written, it's just that the loaded config gets saved back to disk.

This kind of thing should be fixed if only to cope with the upcoming SSDs.

With all that said, expecting all applications to use fsync() or fdatasync() (twice!) simply to safely use write()/rename() when mere atomicity of file renaming is needed is madness.
Comment 12 Martin Steigerwald 2009-03-17 09:19:06 UTC
Well as said I understand both ways of seeing things. One is the viewpoint of a filesystem developer and the other one the viewpoint of an application developer.

And this said, ext4 will sport some patches that alleviate the issue in linux kernel 2.6.30 which are backported by Ubuntu and will probably be backported by other distros. Also btrfs will get a patch for the rename case for 2.6.30. But other filesystems may not.

Since there is the risk of massive data loss being involved when using such a filesystem which doesn't skip delayed allocation after close after rename, I think it is still wise to think of ways how the probability of this happening can at least be reduced in KDE as well. I had occassions were my KDE desktop looked like I did a rm -rf ~/.kde in the meantime, cause so much configuration was lost.

I think to stop writing unchanged files again and again would be a good start.

About the fsync() / fsyncdata() case there IMHO at least should be some consensus between application and filesystem developers. For the benefit of users who are happy when their data is kept safe even on power losses, crashes and whatever. So either its filesystem being adapted or applications or both. And sure IMHO it doesn't have to be stricly POSIX standard. But if not adhering to POSIX, this would mean that data losses could possibly still happen on other systems such as Linux. And then there is Mac OS X where fsync() is a no-op. So from that viewpoint, using fsync() and on Mac OS X that ioctl for FULL DATA SYNC would still be the safest method for now when its about keeping users data safe.
Comment 13 Michael Pyne 2009-03-17 23:18:34 UTC
> I think to stop writing unchanged files again and again would be a good
> start.

Well I've gone and looked at the KConfig implementation and the destructor doesn't call sync unless the config object is dirty.

This obviously doesn't stop paranoid applications from manually syncing, who would have to be fixed but for the most part this shouldn't be an issue after all, I guess the best way to tell would be to instrument KConfig during a KDE startup to see if any apps actually are trying to write out to disk during startup.
Comment 14 Theodore Ts'o 2009-03-17 23:57:29 UTC
>And then there is Mac OS X where fsync() is a no-op.

For the record, fsync() is not a no-op on MacOS X.   It will cause data to be sent to the hard drive, but it won't send a low-level flush-the-data-to-the-spinning-platters command.   So if the power is lost, there might be data in the track buffer which hasn't been fully pushed out to the iron oxide.  But fsync() does do *something*; it does send the data to the hard drive.

Note by the way, that if you don't use the "barrier=1" mount option, ext3 actually doesn't send low-level flush commands to the hard drive either.   Ext4 does use "barrier=1" by default, but Andrew Morton refused the patch for ext3 because of the performance impact, and his belief that it's relatively hard to hit the failure case.  (Of course, Chris Mason then created a shell script which loaded the system hard enough to corrupt an ext3 file system; but akpm still hasn't taken the patch.)    I didn't care enough to push the ext3 patch through akpm's resistance, since ext4 is faster than ext3 even with barriers enabled, and I'm using ext4 on all of _my_ systems.  :-)
Comment 15 Kai Krakow 2009-03-18 00:02:18 UTC
It looks like kconfigini.cpp is just doing in-place edits instead of create-write-close-rename semantics:

http://websvn.kde.org/trunk/KDE/kdelibs/kdecore/config/kconfigini.cpp?revision=882430&view=markup

Or does the magic happen somewhere else in the KConfig hierarchie?

If not, no wonder why so many people loose configs: Not only KConfig rewrites many files during boot, it also replaces contents in-place instead creating a tmp file and renaming to ensure atomicity. The Ext4 fixes in 2.6.30 won't help this...
Comment 16 Kai Krakow 2009-03-18 00:26:43 UTC
(In reply to comment #13)
> > I think to stop writing unchanged files again and again would be a good
> > start.
> 
> Well I've gone and looked at the KConfig implementation and the destructor
> doesn't call sync unless the config object is dirty.

Still sync() doesn't do an atomic replace when using KConfigIni backend - as far as I realized. Also the method name could be misleading in context of this discussion: It doesn't do anything like fsync(), it just opens a file, truncates, writes entries, and closes the file. No fsync, just left with a closed file handle. No chance to call fsync() on the handle from outside.

But my suspision is that fsync() is not needed anyway if the replace would be atomic - and my current understandings of the code don't reveal it could be atomic.

> This obviously doesn't stop paranoid applications from manually syncing, who
> would have to be fixed but for the most part this shouldn't be an issue after
> all, I guess the best way to tell would be to instrument KConfig during a KDE
> startup to see if any apps actually are trying to write out to disk during
> startup.

Does the dirty flag get set if you replace config data with itself? Means: There was a write request but no actual change in data? I didn't dig that deep into the code - prefering going to bed now. ;-)
Comment 17 Michael Pyne 2009-03-18 00:31:22 UTC
Kai: Indeed, the backend now seems to use a truncate/write combo in the event the file already exists, so that's definitely a bug, I'll see if I can't get that fixed and backported so that the changes in 2.6.30 will actually have some good effect for KDE 4.
Comment 18 Theodore Ts'o 2009-03-18 01:09:37 UTC
>Not only KConfig rewrites many files during boot, it also replaces contents >in-place instead creating a tmp file and renaming to ensure atomicity. The Ext4 >fixes in 2.6.30 won't help this...

Actually, the ext4 fixes includes an attempt to try to provide some safety for (broken) applications/libraries for open-truncate-write-close (aka replace-via-truncate).  It is no worse than ext3; but if a file system commit happens after the truncate, and a crash happens after that commit, but before the close and the next commit, you will lose data.

Note, by the way, that btrfs has added a workaround for open-write-close-rename (aka replace-via-rename), but it does _not_ have one for replace-via-truncate.   XFS has an implied fsync after replace-via-truncate, but it does _not_ have one for replace-via-rename.   Ext4 has both workarounds; I took the idea of the workaround for replace-via-truncate from XFS, and adding a workaround for replace-via-rename was my idea, which btrfs later copied, and which I had coded up and queued for 2.6.30 *before* this whole controversy got slashdotted.   One of the things I plan to talk about at the upcoming Linux Filesystem and Storage workshop is some kind of standard about what workarounds (if any) filesystems "should" implement, and if we are going to create a workaround for replace-via-rename, whether it should be done in the VFS layer and make it an officially supported Linux feature that applications should be able to depend upon.

There _will_ be controversy about this in the Linux file system community; I suggested adding the rename workaround to Eric Sandeen, who is also an XFS developer, and he was not psyched about adding it to XFS, since it seems like Yet Another Kludge to work around broken applications.   The bottom line is that the only safe thing on all operating systems and all file systems really is open-write-close-fsync-rename.   We'll see how my other fellow file system developers react about this at the upcoming Linux Filesystem and Storage Workshop.
Comment 19 Oswald Buddenhagen 2009-03-18 01:23:37 UTC
kai & mpyne: it takes a bit more than a glimpse to form a qualified opinion about some piece of code. kconfig does *not* replace by truncating unless absolutely necessary for file ownership reasons.

the dirty flag is indeed set even if data does not actually change. this is no problem for kconfigxt apps, but "conventional" apps will typically rewrite their configs needlessly.
Comment 20 Michael Pyne 2009-03-18 02:48:24 UTC
> kai & mpyne: it takes a bit more than a glimpse to form a qualified opinion
> about some piece of code. kconfig does *not* replace by truncating unless
> absolutely necessary for file ownership reasons.

I'll quote the code I'm referring to: (kdelibs/config/kconfigini.cpp)

    // Open existing file. *DON'T* create it if it suddenly does not exist!
#ifdef Q_OS_UNIX
    int fd = KDE_open(QFile::encodeName(filePath()), O_WRONLY | O_TRUNC);
<snip>
    writeEntries(locale, f, writeMap);
    f.close();
    fclose(fp);
#else

Now I understand that the intention is to avoid creating the file if it was deleted in the interim but I think in the interests of data consistency it would be better here to check for existence directly before doing the rename() and accepting that there will be a race.

I have guesses as to why we are trying to avoid re-creating a unlinked file but since it's not in the code it would probably be easier if you said why it's necessary.
Comment 21 Oswald Buddenhagen 2009-03-18 10:18:40 UTC
yes, thanks for the quote. i wrote that code, and rewrote it after it got broken during the kde4 port.
now, how about looking just a few lines up to actually understand how things work, instead of essentially spreading FUD about our own technology?
Comment 22 Kai Krakow 2009-03-18 10:48:33 UTC
(In reply to comment #19)
> kai & mpyne: it takes a bit more than a glimpse to form a qualified opinion
> about some piece of code. kconfig does *not* replace by truncating unless
> absolutely necessary for file ownership reasons.

Sorry if I understood the internal workings of this code wrong. This is why I had the question if there's some magic happening outside this backend method. I will have a deeper look. By no means I wanted to feed any fud.

I just wanted to put in some ideas where the problem may be, and as you wrote:

> the dirty flag is indeed set even if data does not actually change.

There is actually a problem. I think getting this fixed will proably already help the data-loss problem a lot. And fixing this would be wise for SSD users, too. In the end that is the purpose of having a general backend: To solve common problems there and not in the application using the backend. Or did I get this wrong?
Comment 23 Kai Krakow 2009-03-18 11:02:19 UTC
Oswald, I assume this is where the hidden magic takes places?

> @@ KConfigIniBackend::writeConfig @@
> return file.finalize();

Meaning: create, write, close, replace semantic?
Comment 24 Michael Pyne 2009-03-19 00:16:36 UTC
> now, how about looking just a few lines up to actually understand how things
> work, instead of essentially spreading FUD about our own technology?

Well I think the point is that it was obvious I wasn't understanding it, otherwise I wouldn't have asked.  You may also take it as a given that I actually read at least the function involved, so telling me to go read a few lines up was not helpful.

So instead I went and asked around and for future posterity the answer is that the case in question is only run if the config file is somehow not owned by the user.  The assumption is that the user will not have write permissions to re-create a new file and therefore the file is truncated and re-written.  This is not a normal condition for operation therefore the correct KSaveFile case is normally run.

I would argue that checking only if uid == owner id is incorrect since the user may still have permission to perform a rename (since that is based on the underlying directory permissions) but I understand that the truncate/write case would not be experienced except in abnormal circumstances.
Comment 25 Martin Steigerwald 2009-03-26 10:11:45 UTC
Two posts from Linus about the topic in a discussion on LKML - Linus wants data being written before metadata, anything else seems totally insane to him:

http://lkml.org/lkml/2009/3/24/415
http://lkml.org/lkml/2009/3/24/460


Daniel Philipps, developer of Tux3 filesystem, wants at to make sure that renames come after file being written:
http://mailman.tux3.org/pipermail/tux3/2009-March/000829.html


I tend to think that it really would be better when filesystems would default to data before metadata and be tunable for high performance workloads on servers with controllers that are backed by NVRAM. But I am not that sure on this either. POSIX or not. The risk of silent data loss is quite severe and it seems fsync()-ing doesn't give quite the same semantics than the rename approach which just should guarentee atomicity.


But however thats decided: I guess its best to split this bug. There are two issues:

1) Config files being truncated after crash or hard crash.

2) At least some KDE applications seem to needlessly write out config and other files. 

Problem 1 should stay here IMHO and then it could be decided whether to do something about it on the KDE side or set it to won't-fix.

I can do a bug report about problem 2. According to comment #16 from Kai Krakow its seem not yet completely clear to me whether KConfig always writes out config files that have unchanged or whether it does not and its some applications that do it. My observation was lots of truncated files on KDE startup and to me it appears that a lot of config files have been rewritten needlessly.

When its global to KConfig the bug report should go to kdelibs/general I think. But when its not, there should be individual bug reports regarding affected applications and probably a container bug referring them all.

Could somebody who has more experience with reading KDE code than I, like Oswald, who has written it, comment on whether writing out unchanged configuration files is caused by KConfig in general or by single applications?

Then I could starting making one or more bug reports for the second issue. Having this one fixed at least reduces the probability of data loss quite a lot. That would be at least my expectation.
Comment 26 Oswald Buddenhagen 2009-03-26 11:10:13 UTC
re 1), it's cool that obviously several top kernel devs agree with the need for proper barrier semantics (though i wonder why you had to make that "inflammatory" blog post then, ted? ;)

re 2), i remembered that we actually tried to tackle this issue in may/june last year - the related thread is at http://lists.kde.org/?l=kde-core-devel&m=121154771109985&w=2 and we got some hacking done at akademy. as the issue apparently persists, somebody (with lots of time ;) needs to verify that our autotests cover all relevant cases (and pass in the first place, heh). if everything is right in kconfig, the issue needs to be looked for further up - maybe make a diff between a pre- and post-startup state of ~/.kde and see whether there is some pattern in the modifications.
Comment 27 Martin Steigerwald 2009-03-26 11:20:39 UTC
I want to add that I only ever had this with KDE 3.5.10 yet. I have KDE 4.2.1 on one laptop. This one crashed hard while unplugging a Silicon Image PCMCIA eSATA adaptor from DeLOCK twice, but I didn't see (!) configuration losses. I am using XFS from Linux Kernel 2.6.28.8 on it.

However the reporter of 

https://bugs.launchpad.net/bugs/317781

used Ubuntu Jaunty which appears to have KDE 4.2.1:

http://packages.ubuntu.com/search?keywords=kdebase&searchon=names&suite=jaunty&section=all
Comment 28 Theodore Ts'o 2009-03-26 17:10:21 UTC
Well, if everyone on the Linux side agrees that there should implied (asynchronous) fsync() semantics for replace-via-rename, then we can do something that works on for all file systems, at least on Linux.   It still won't help KDE on other operating systems (but maybe I shouldn't care about whether or not OpenSolaris has these semantics or not).   

My position has always been that I would implement workaround patches for ext4 ("be liberal in what you accept"), but that for people who want to make their programs portable and safe, they should do the fsync ("be conservative in what you send").    OTOH, if people want to promulgate programs that only work safely on Linux and/or only on ext3/ext4, ultimately, I can't stop them.

So at the moment XFS doesn't have the workaround for replace-via-rename, but if we (as a Linux file system developers consensus) end up agreeing that the implied fsync() should be present for rename, I suspect XFS will also add the workaround, perhaps under protest.   But again, this isn't going to help KDE on non-Linux systems; I don't know if you care or not.
Comment 29 Aaron J. Seigo 2009-03-26 17:50:35 UTC
Ossi: if you can decide how you want KConfigBackend to look like that would be great so that we can get on with writing other config system drivers for KConfig. then we can very easily provide a backend, for instance, that uses TDB/LDB and eliminate this problem completely for those who wish to go that route while picking up some free LDAP and perhaps even Samba related integration in the process.

obviously this is "fixing" the problem in a completely different area, but i think it shows how useful having a completed and published KConfigBackend class would be.

however, I'm not going to bother touching KConfigBackend unless you are on board as I don't need that frustration again. will you have time/motivation to work on that class? if not, are you willing/able to let someone else work on it without blocking their efforts?
Comment 30 Oswald Buddenhagen 2009-03-26 17:50:41 UTC
well, yes - you should not care, ted. :D
and we already did - at least in kde3. as usual, our windows porting heroes broke it. will have to fix it again some day ... :-/

fwiw, i talked about that problem with the xfs@linux chief guy some years ago. he said they wanted to to implement such barrier semantics. no idea whether they actually did.

as far as i'm concerned, implementing these semantics is not a workaround - it is the *right* thing to do. obviously, the posix semantics are optimized for performance. on enterprise systems, where uptime is at a premium and actually lost data can be restored from very frequent backups, having only metadata consistency guarantees is the acceptable lowest common denominator. desktop system have typically pretty much inverse requirements, and it would be perfectly logical to set other standards. and with sane support at the block layer (so "write that before that" doesn't imply "write everything before writing that"), the performance penalty should be pretty acceptable.
Comment 31 Oswald Buddenhagen 2009-03-26 18:25:51 UTC
hey aaron. this camouflaged thread hijacking is a rather clever way to nag me. or maybe not. :-P
the backend api "only" needs to comply with some quality standards, providing maximal possible encapsulation and at the same time allowing for everything that needs to be possible. see, just the usual stuff. :-D
to get the ball rolling, somebody needs to gather the requirements (from both the (advanced) user's and the backend implementor's perspective).
but this really isn't the report to discuss this ... you know how to reach me. :)
Comment 32 Oswald Buddenhagen 2009-03-28 18:10:54 UTC
SVN commit 946094 by ossi:

don't dirty the config when overwriting with same value, take 2

it helps enormously when the set dirty flag is not part of the
definition of "same". thx to Maksim

CCBUG: 187172


 M  +1 -1      config/kconfig.cpp  
 M  +6 -5      config/kconfigdata.h  
 M  +25 -0     tests/kconfigtest.cpp  
 M  +1 -0      tests/kconfigtest.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=946094
Comment 33 Kai Krakow 2009-03-28 20:57:20 UTC
Ah very nice Oswald, thanks for working on it. Commit 946094 does not seem to be the complete patch. Where is take 1 so I can put it into my KDE tree for testing?
Comment 34 Oswald Buddenhagen 2009-03-28 21:00:48 UTC
take one was in august 2008. ;)
Comment 35 Kai Krakow 2009-03-29 13:23:39 UTC
(In reply to comment #34)
> take one was in august 2008. ;)

So take 1 is probably already in my installation (KDE 4.2.1 on gentoo)...
Comment 36 Martin Steigerwald 2009-03-30 22:45:08 UTC
Thanks a lot, Oswald! So you fixed it before I opened a separate bug report for this one ;-). I think this will already help a lot.

Would Take 2 be a backport candidate for KDE 4.2.3?
Comment 37 Oswald Buddenhagen 2009-03-31 11:34:08 UTC
it has been backported "long" ago.
Comment 38 Oswald Buddenhagen 2009-04-04 14:48:58 UTC
SVN commit 949054 by ossi:

re-introduce KDE_EXTRA_FSYNC for the poor XFS/Ext4/... users

setting this env variable to anything (before kde starts!) will avoid
getting those pesky 0-byte sized files after system crashes - at a
potentially non-trivial performance cost.

BUG: 187172


 M  +1 -0      ConfigureChecks.cmake  
 M  +1 -0      config.h.cmake  
 M  +31 -0     kdecore/io/ksavefile.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=949054
Comment 39 Kai Krakow 2009-04-04 20:10:41 UTC
(In reply to comment #38)
> re-introduce KDE_EXTRA_FSYNC for the poor XFS/Ext4/... users
> 
> setting this env variable to anything (before kde starts!) will avoid
> getting those pesky 0-byte sized files after system crashes - at a
> potentially non-trivial performance cost.

The performance impact should not be that bad since you fixed rewriting config files which were not really dirty. That fix probably also makes the extra fsync patch a little less needed for xfs/ext4/... users.

BTW: Did that don't-rewrite-non-dirty-configs patch (SVN 946094, comment #32) into KDE 4.2.2? Or do I need to ask my distributor to back-port the patch?
Comment 40 Pino Toscano 2009-04-14 22:33:23 UTC
*** Bug 189644 has been marked as a duplicate of this bug. ***
Comment 41 Oswald Buddenhagen 2009-05-31 10:21:04 UTC
*** Bug 187032 has been marked as a duplicate of this bug. ***
Comment 42 Markus Trippelsdorf 2013-07-11 17:52:31 UTC
Sorry to resurrect this bug, but I've got hit by the same issue in the last few days
during bisection of a kernel hang. During the bisection I was loosing my KDE settings
bit by bit with every reboot. First my window-rules disappeared, then my desktop background
changed to default, then my taskbar moved from top to the bottom, etc.

I tried to set KDE_EXTRA_FSYNC=1, but that doesn't affect config file handling at all.

So I wrote the following patch:

--- kconfigini.cpp      2013-06-28 19:03:40.859337527 +0200
+++ /root/kconfigini.cpp        2013-07-11 19:47:35.557230235 +0200
@@ -37,6 +37,14 @@
 #include <qdebug.h>
 #include <qmetaobject.h>
 #include <qregexp.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#ifdef HAVE_FDATASYNC
+#  define FDATASYNC fdatasync
+#else
+#  define FDATASYNC fsync
+#endif
 
 extern bool kde_kiosk_exception;
 
@@ -441,7 +449,13 @@
         file.setTextModeEnabled(true); // to get eol translation
         writeEntries(locale, file, writeMap);
 
-        if (!file.flush()) {
+        if (file.flush()) {
+            forever {
+                if (!FDATASYNC(file.handle()))
+                    break;
+                }
+           }
+       } else {
             // Couldn't write. Disk full?
             kWarning() << "Couldn't write" << filePath() << ". Disk full?";
             file.abort();

and it does seem to fix the issue.
Comment 43 Martin Steigerwald 2018-09-05 09:30:53 UTC
Markus, did you commit this patch? The bug is marked as FIXED, so I bet so, but as I still have KDE_EXTRA_FSYNC set in ~/.zshrc I ask:

Has this issue really been fixed so that the default behavior is safe?

fsync() originally was omitted due to Ext3 having been painfully slow. But nowadays at least for desktops I´d expect people use Ext4, XFS or BTRFS and no Ext3 anymore.
Comment 44 Martin Steigerwald 2023-07-19 07:20:11 UTC
Updating the version field to frameworks.

I am reopening this, since I can not confirm that the patch of Markus or something similar went in.

I checked kconfig.git master branch as of d5940e22aaf651fe6643f159c94d2b24096e1d8e from 19th of July.

No mention of "fdatasync" or "fsync" searched independently of lower and upper case anywhere in the code.

These days it really does not make any sense to use Ext3 anymore which has been very slow on fsync()¹. So while Ext4 fixes the truncation and rename delayed allocation case, XFS fixes the truncation delayed allocation case and BTRFS fixes the rename delayedd allocation case, it still does make sense that the application does the right thing and synchronize configuration files to disk properly.

In case this has been fixed at a different place or in a different way that above search did not uncover, please close again.

[1] https://blahg.josefsipek.net/?p=364
Comment 45 Christoph Cullmann 2024-06-20 19:44:14 UTC
I don't think we have datasync calls there.
Comment 46 Martin Steigerwald 2024-06-21 08:31:36 UTC
Nowadays where Ext3 should not be in wide use anymore, if at all, and nowadays where many people use flash based storage, I am strongly for just making sure to sync where needed.

That written, I did not see any truncated config file since ages. I still have "KDE_EXTRA_FSYNC=1" set, but whether this actually really makes a difference, given some of the comments above, I do not know. Anyway, I'd just sync by default. It really can't be a performance issue anymore – except maybe on embedded device with some slow SD card –, especially as unchanged files are not rewritten anymore.
Comment 47 Kai Krakow 2024-06-21 09:26:31 UTC
(In reply to Martin Steigerwald from comment #46)
> Nowadays where Ext3 should not be in wide use anymore, if at all, and
> nowadays where many people use flash based storage, I am strongly for just
> making sure to sync where needed.
> 
> That written, I did not see any truncated config file since ages. I still
> have "KDE_EXTRA_FSYNC=1" set, but whether this actually really makes a
> difference, given some of the comments above, I do not know. Anyway, I'd
> just sync by default. It really can't be a performance issue anymore –
> except maybe on embedded device with some slow SD card –, especially as
> unchanged files are not rewritten anymore.

If I remember correctly, you may be using btrfs. On btrfs, this is actually not needed, it works fine without fsync and is actually faster that way no matter the storage technology. I'm running with `KDE_EXTRA_FSYNC=0` since years on btrfs and never had a problem.

I suggest that the behavior is kept to optionally disable fsync/fdatasync, but it should befault to "on".

The problem is with file systems which do not commit metadata and data properly in a single transaction or in expected order, e.g. xfs does this and writes data lazily or delayed. In that case, xfs ensures that you do not see stale or old data after a crash by zeroing or truncating the affected files. I think ext3/ext4 does something similar unless you use its full journal mode (which is a lot slower, ofc), where xfs probably behaves like ext4 with "journal=writeback".

Especially with btrfs, the argument is not whether it's on flash or spinning disks: fsync/fdatasync can significantly slow the system down because btrfs may have a rather long list of outstanding transactions it would have to write then (blocking) until reaching the transaction affected by the sync. This can take a long time and blocks the system even on flash disks.

So if this discussion is about whether we should remove `KDE_EXTRA_FSYNC`, I'd rather not have it force-enabled, especially because KDE seems to be very busy with its config files and reads and writes them a lot (similar to the cache directory). IMHO, the variable could be changed to require `KDE_DISABLE_EXTRA_FSYNC=this_is_dangerous_and_I_know_what_I_am_doing`. This may make it more obvious to people who blindly follow some internet guides to "make things faster" that they may be doing something harmful.
Comment 48 Martin Steigerwald 2024-06-21 11:41:20 UTC
(In reply to Kai Krakow from comment #47)
> (In reply to Martin Steigerwald from comment #46)
[…]
> So if this discussion is about whether we should remove `KDE_EXTRA_FSYNC`,
> I'd rather not have it force-enabled, especially because KDE seems to be
> very busy with its config files and reads and writes them a lot (similar to
> the cache directory). IMHO, the variable could be changed to require
> `KDE_DISABLE_EXTRA_FSYNC=this_is_dangerous_and_I_know_what_I_am_doing`. This
> may make it more obvious to people who blindly follow some internet guides
> to "make things faster" that they may be doing something harmful.

Well for special cases there could be an option to switch off the syncing. But the default nowadays should really be on the safe side. From what I read from you, Kai, we can agree on this much, can we? I am okay with renaming the variable to the non default case like KDE_DISABLE_EXTRA_FSYNC and have it off by default. Not sure whether any "iknowwhatiamdoing" is needed. Its not exposed in a configuration GUI to begin with.

Is it really that much of an issue with BTRFS? Especially since I switched to laptop with NVME SSD I am not aware of any issues here.

That written, I have no direct comparison with home directory being on XFS, I bet it would feel faster, but I am not actually waiting on filesystem writes to begin with. At least as long as Baloo only indexes filenames and Akonadi indexer is disabled (waiting for "make indexing great again"). On another system I do have Baloo completely enabled, as there are way less files. Anyway, this is getting of topic as well I am looking forward to try with BCacheFS in the future which with it frontend / backend architecture may solve any performance issues with syncing in a filesystem of the new generation (with snapshots and so on).