why does ext4_sync_parent() bother with d_find_any_alias() at all?

The caller has a dentry for that inode, after all...
Am I missing something there? IOW, is there anything wrong with something like (untested) patch below?

index 924726dcc85f..c635cd732c2e 100644
@@ -43,16 +43,14 @@
  * the parent directory's parent as well, and so on recursively, if
  * they are also freshly created.
  */
-static int ext4_sync_parent(struct inode *inode)
+static int ext4_sync_parent(struct inode *inode, struct dentry *dentry)
 {
-	struct dentry *dentry, *next;
+	struct dentry *next;
 	int ret = 0;
 
 	if (!ext4_test_inode_state(inode, EXT4_STATE_NEWENTRY))
 		return 0;
-	dentry = d_find_any_alias(inode);
-	if (!dentry)
-		return 0;
+	dget(dentry);
 	while (ext4_test_inode_state(inode, EXT4_STATE_NEWENTRY)) {
 		ext4_clear_inode_state(inode, EXT4_STATE_NEWENTRY);
 

@@ -99,7 +97,7 @@ static int ext4_fsync_nojournal(struct file *file, loff_t start, loff_t end,
 	if (ret)
 		return ret;
 
-	ret = ext4_sync_parent(inode);
+	ret = ext4_sync_parent(inode, file->f_path.dentry);
 
 	if (test_opt(inode->i_sb, BARRIER))
 		*needs_barrier = true;

FSVO "reliably"; apply memory pressure and all promises are gone. As the matter of fact, have another thread link() into a different directory and unlink() the result before your fsync(), and you'll end up syncing a directory that has no links to the file in question.

Quite possibly returned by d_find_any_alias().

What warranties is the entire thing trying to provide? Anything that relies upon the order of dentries in the alias list is going to be quite brittle...

Originally, this was added not to provide *guarantees*, but just to make things better in no-journal mode, where the parent directory might not get written until 30 seconds later without a journal.

commit 14ece1028b3ed53ffec1b1213ffc6acaf79ad77c

Author: Frank Mayhar <[email protected]>
Date:   Mon May 17 08:00:00 2010 -0400

ext4: Make fsync sync new parent directories in no-journal mode

Add a new ext4 state to tell us when a file has been newly created; use that state in ext4_sync_file in no-journal mode to tell us when we need to sync the parent directory as well as the inode and data itself. This fixes a problem in which a panic or power failure may lose the entire file even when using fsync, since the parent directory entry is lost.

Addresses-Google-Bug: #2480057

The use case that we were optimizing for was for newly created files, and if the common an atomic write pattern wan't:

    fd = open("dir/foo.new");
    write(fd);
    link("dir/foo.new", "dir/foo");
    fsync(fd);

... but rather:

    fd = open("dir/foo.new");
    write(fd);
    rename("dir/foo.new", "dir/foo");
    fsync(fd);

So I'd say that what we are trying to do is to provide a best effort attempt to avoid data loss in the face of crashes where (a) there is only a single directory involved (e.g., where the application program is renaming "foo.new" to "foo" in the original directory where "foo.new" was created), and where there aren't other processes trying to play fancy games with a newly created file.

This isn't documented as a guarantee; we're just trying to provide a better experience for application programmers which don't realize that POSIX requires that you fsync the directory if you really want strong guarantees, on the theory that the application programmers vastly outnumber the number of file system developers ---- and application programmers tend do the darnest things :-)

In any case, because of this background, which an AI review bot would have no hope of understanding, I think your patch is fine. :-)

- Ted

P.S. We also have a similar, but unrelated "save the clueless application programmer" hack where if the user does:

    fd = open("dir/foo.new");
    write(fd);
    close(fd)
    rename("dir/foo.new", "dir/foo");

... on the rename(), if foo.new was a newly created file, we will initiate an asynchronous writeback. This was a compromise where we didn't want to impose a performance tax where it wasn't necessary, but it was truly *shocking* how common the above pattern, sans any fsync at all existed in many GNOME and KDE libraries, as well as the TuxRacer game, where it would write the top-ten scoreboard, and then close the 3D MESA library shortly thereafter, at which point the proprietary Nvidia driver would crash the system, and users who lost
their 10 ten score were very sad. :-) This resulted in the whole
O_PONIES discussion / compromise in 2009:

    https://blahg.josefsipek.net/?p=364

Again, all of this is best efforts, and the philosophy is to work around buggy userspace while minimizing performance overhead --- and especially, *not* penalizing programs that do things correctly where they are supposed to.