Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Rumors are that it is not too much work to add ZFS support.
I am sure that UFS is hard due to SU and esp. SU+J. I am adding Kirk in case he is interested.
I promise to handle msdosfs if any of large fs (AKA UFS or ZFS) get the flag.
| sys/kern/vfs_syscalls.c | ||
|---|---|---|
| 3791 | The Linux man pages I looked at seem to indicate EINVAL should be returned if both NOREPLACE and EXCHANGE are set, since they're mutually exclusive. Do we need to be concerned about that level of compatibility? The below check that returns EEXIST will still avoid modifying the target file in that case. | |
| sys/kern/vfs_syscalls.c | ||
|---|---|---|
| 3791 | It's probably worth mirroring the Linux behaviour here. | |
The sys/kern/vfs_syscalls.c and sys/sys/fcntl.h changes LGTM with one small note
| sys/sys/fcntl.h | ||
|---|---|---|
| 263 | Perhaps /* Atomically exchange from and to */ | |
I've applied the patch to my tree and will give it a try in Halifax next week.
For reference, OpenZFS commit that added Linux support: https://github.com/openzfs/zfs/commit/dbf6108b4df92341eea40d0b41792ac16eabc514
Thanks to Air Canada giving me an unexpected few hours in YYZ I had a chance to try this out. One remaining issue w/ the tmpfs support - missing directory timestamp update.
I now have a kernel panic from an attempt to swap with source missing, e.g.
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
int main(void) {
close(open("target", O_CREAT | O_WRONLY, 0644));
renameat2(AT_FDCWD, "does_not_exist", AT_FDCWD, "target", RENAME_EXCHANGE);
}This thing does not even wrap renameat(2), neither it wraps renameat2(2).
Might be, but I do not have an example that I can thought of.
| lib/libsys/rename.2 | ||
|---|---|---|
| 151 | It is already documented below, in the renameat2() error section. It was done when renameat2() was added. | |
As a note, I think that WILLBEDIR is not too critical there. It basically allows to have slash after the file name to not choke the namei() parser. VWRITE vs. VWRITE | VACCESS seems to be irrelevant at all because we overwrite the same dirent.
This cannot be implemented in UFS since a requirement of its consistency model is that the on-disk image is always consistent. If the directory entries being swapped are in two different disk blocks, then there is no order of writing that will have a consistent filesystem. The only way to ensure consistency is to have a journal that can correct the unwritten directory name with its updated inode number.
In other words, it cannot be implemented for SU, but can for async and for SU+J, do you agree?
SUJ only journals things that SU is tracking. It is not a general journalling mechanism. So, if SU cannot do it, then SUJ cannot do it either. Since async makes no promises about consistency, it could implement anything it wants. But I would be loath to say that you have to run async in order to get a certain feature as I would not want to encourage use of async. My attitude is that UFS reliability has been a hallmark of its existence and I do not want to introduce a feature that removes its reliability. Especially a feature which is of such little use.
Having had overnight to think through how to implement AT_RENAME_EXCHANGE in UFS, it will require a journal. If the operation is single threaded or at least kept to a bounded number in parallel, the log can be kept in an area that fits in the disk block holding the superblock. The journal entry simply needs to keep entries for the two names being exchanged. Specifically, the inode number of their resident directory, their name, and the inode number that they should reference. A flag in the superblock can indicate that the journal needs to be checked. If so, fsck can look up each name and verify that it references the correct inode number correcting it if it is wrong.
Does this operation allow one or both of the names to reference a directory? If so implementing it is going to be a lot more complicated since it will need to deal with '..' resolution if the names are in different directories. The code implementing it is going to start looking like rename which is no small source of maintenance headaches over the years.
Since some time, all renames on the specific mount point are serialized. I added the mp->mnt_renamelock a year ago. So it is really not even a journal but just a storage for two dirents, and theirs locationss in the directory blocks. It can be done synchronously to avoid storing more that one exchange entry.
Yes, the EXCHANGE operation can exchange any dirents, so dotdot corrections are needed. It is implemented there for tmpfs.
I also thought about possible alternatives to the mini-journal. Could it help if we introduce another kind of the dirent type, something like ephemeral whiteout? Its property would be that it is ignored by directory lookups and getdirents() always. We could rename one of the dirent to the emphemeral whiteout first to keep the reference counting to ensure that no used resources could be considered free. Then do two renames including the whiteout rename, and then remove the original dirent.
I like the approach that you suggest. In thinking about the implementation it seems like the approach for exchange(a, b) would be:
lock directory(s) containing a & b so a & b names cannot be changed or deleted
rename a -> *b
rename b -> a
rename *b -> b
where *b is the "invisible" directory entry. The fsck recovery then is if *b exists and b does not exist, make *b visible as b. If b does exist, the *b needs to come back as a. The question then is, how do we know the name a? If the first rename were a -> *a then we need a way to know the b name.
Note that a and b may be located in different directories.
I think that all information we need should be put into the *b pseudo-entry. As such, it might be more of single-entry journal and not the entry. Then might be, it should be indeed located at some place pointed to by the superblock. It is enough to have only single entry there.
Peter finished testing the patch. Do anybody have more comments about it? I intend to commit it shortly.
No objection in principle, but I haven't reviewed it thoroughly and probably won't have time, and it would be preferable if someone could.
I'm curious why this functionality even exists in Linux and where it is used.
For UFS, yes, the *b entry cannot contain all the information because of a simple space argument. Having some block serving as a mini-journal somewhere could be fine, but proposing to fit the latter in the superblock's block looks a bit worrying (write errors, wear leveling, etc.?).
Do any file systems besides tmpfs support exchange?
What is the error returned by file systems that do not support exchange? It should be documented in rename(2).
Is exchange implemented by any system utility on Linux? If so, then it should be added to that utility in FreeBSD.
Do any file systems besides tmpfs support exchange?
OpenZFS supports it on Linux. I think it will be fairly easy to plumb it through.
No, because we do not have the flag before this patch.
And the patch only adds the tmpfs support.
What is the error returned by file systems that do not support exchange? It should be documented in rename(2).
The error is EOPNOTSUPP, and it was already documented when I added renameat2(2):
[EOPNOTSUPP] One of the flags specified is not supported by the
filesystem where the to-be renamed file is located.Is exchange implemented by any system utility on Linux? If so, then it should be added to that utility in FreeBSD.
Apparently there is at least the exch(1) utility on Linux: https://man7.org/linux/man-pages/man1/exch.1.html
Debian source code search shows that the flag is used by systemd (irrelevant for us) and Samba. So the flag addition is useful at least by bringing the VFS interface to the Samba expectations, which is IMO required.