11 ms·

Without the fsync() before rename(), on system crash, you can end up with the rename having been executed but the data of the new file not yet written to stable

by AnssiH 4y ago

Without the fsync() before rename(), on system crash, you can end up with the rename having been executed but the data of the new file not yet written to stable storage, losing the data. ext4 on Linux (since 2009) special-cases rename() when overwriting an existing file so that it works safely even without fsync() (https://lwn.net/Articles/322823/ https://lwn.net/Articles/322823/), but that is not guaranteed by all other implementations and filesystems. The sync() at the end is indeed not needed for the atomicity, it just allows you to know that after its completion the rename will not "roll back" anymore on a crash. IIRC you can also use fsync() on the parent directory to achieve this, avoiding sync() / syncfs().

AdamJacobMuller 4y ago
> ext4 on Linux (since 2009) special-cases rename This is interesting. The linked git entry (https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?id=dbc85aa9f11d8c13c15527d43a3def8d7beffdc8 https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.g...) from the LWN article says "Notice: this object is not reachable from any branch." Did this never get merged because I definitely saw this issue in production well after 2009. I guess it either got changed, or, a different patch applied but perhaps this https://github.com/torvalds/linux/blob/master/fs/ext4/namei.c#L3765-L3766 https://github.com/torvalds/linux/blob/master/fs/ext4/namei.... does it?
- AnssiH 4y ago
  The patch just got rebased, here's the one that was actually applied in master for v2.6.30: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?id=8750c6d5fcbd3342b3d908d157f81d345c5325a7 https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.g... And yes, the code you highlighted is exactly this special-case in its current form. The mount option "noauto_da_alloc" can be used to disable these software-not-calling-fsync safety features.

by AnssiH 4y ago

AdamJacobMuller 4y ago

> ext4 on Linux (since 2009) special-cases rename This is interesting. The linked git entry (https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?id=dbc85aa9f11d8c13c15527d43a3def8d7beffdc8 https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.g...) from the LWN article says "Notice: this object is not reachable from any branch." Did this never get merged because I definitely saw this issue in production well after 2009. I guess it either got changed, or, a different patch applied but perhaps this https://github.com/torvalds/linux/blob/master/fs/ext4/namei.c#L3765-L3766 https://github.com/torvalds/linux/blob/master/fs/ext4/namei.... does it?

AnssiH 4y ago
The patch just got rebased, here's the one that was actually applied in master for v2.6.30: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/commit/?id=8750c6d5fcbd3342b3d908d157f81d345c5325a7 https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.g... And yes, the code you highlighted is exactly this special-case in its current form. The mount option "noauto_da_alloc" can be used to disable these software-not-calling-fsync safety features.