[PATCH v2 01/14] VFS: revise and expand documentation for atomic_open.

[NeilBrown <neilb@ownmail.net>]

From: NeilBrown <neil@brown.name>

atomic_open is a complex operation which different filesystems implement
quite differently.  The available documentation doesn't give clear
guidance on how it should be implemented.

nfsd has a particular need to open only regular files, but to get
precise information about what was found if it wasn't a regular file.
This is slightly different to the syscall calling needs.  In particular
it suggests that __O_REGULAR shouldn't always result in -EFTYPE.

In any case that does involve creating open state, using
finish_no_open() is simplest as it reduces the need to check
__O_REGULAR, O_DIRECTORY, O_NOFOLLOW.

So refresh the documentation to give guidance on the choice between
finish_no_open, finish_open, and an error.  Efficiency always wins, but
when that isn't an issue, prefer finish_no_open().

Also clarify the required behaviour when __O_REGULAR is given.  This
should return -EISDIR if a directory is found as nfsd needs this.  If a
symlink is found then __O_REGULAR does NOT apply: O_NOFOLLOW must be
used to decided if it is safe to not return the looked-up dentry.

Signed-off-by: NeilBrown <neil@brown.name>
---
 Documentation/filesystems/vfs.rst | 67 ++++++++++++++++++++++++++-----
 fs/namei.c                        |  3 ++
 2 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index d3a93eec3945..00ada8cc85ae 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -599,17 +599,62 @@ otherwise noted.
 
 ``atomic_open``
 	called on the last component of an open.  Using this optional
-	method the filesystem can look up, possibly create and open the
-	file in one atomic operation.  If it wants to leave actual
-	opening to the caller (e.g. if the file turned out to be a
-	symlink, device, or just something filesystem won't do atomic
-	open for), it may signal this by returning finish_no_open(file,
-	dentry).  This method is only called if the last component is
-	negative or needs lookup.  Cached positive dentries are still
-	handled by f_op->open().  If the file was created, FMODE_CREATED
-	flag should be set in file->f_mode.  In case of O_EXCL the
-	method must only succeed if the file didn't exist and hence
-	FMODE_CREATED shall always be set on success.
+	method the filesystem can look up, create, truncate, and open
+	the file in one atomic operation.  This is needed if the
+	filesystem content can be changed asynchronously and
+	specifically if a negative dentry is not a guarantee that the
+	object doesn't exist.  It is also useful if it is possible to
+	perform combinations of revalidate, lookup, create, open, and
+	truncate more efficiently what with a sequence of individual
+	operations.
+
+	If the object found is not a file or directory, or if
+	lookup/create succeeded without establishing any "open" state,
+	then finish_no_open() should be called to confirm that the
+	dentry is ready to be handled by normal VFS processing.
+	FMODE_CREATED should be set in the "file" if the object was
+	created, and this will prevent further access permission checks,
+	or handling of O_TRUNC and O_EXCL.
+
+	If the lookup/create operation established some open state for a
+	file or directory, the open should be completed by calling
+	finish_open().  Passing NULL as the "open" function to
+	finish_open() is unlikely to be useful as that assumes that no
+	open state has been established.
+
+	atomic_open() may generate errors related to O_DIRECTORY,
+	__O_REGULAR, O_EXCL, O_NOFOLLOW but is not required to as the
+	caller will check those against the resulting dentry and
+	generate any error needed, possibly closing the file if it was
+	opened by finish_open().  atomic_open() is encouraged to handle
+	these flags only when doing so is more efficient than not.
+
+	If __O_REGULAR is handled, it should generate -EISDIR if the
+	name is known to be a directory or -EFTYPE if it is some other
+	non-regular file other than a symbolic link.  Handling of a
+	symbolic link should be guided by O_NOFOLLOW, not __O_REGULAR:
+	-ELOOP can be return if O_NOFOLLOW is set, otherwise the symlink
+	should be returned through finish_no_open().
+
+	The focus for atomic_open() is to provide the correct dentry and
+	to set FMODE_CREATED as accurately as possible.  If O_EXCL was
+	set, FMODE_CREATED should only be set if this operation
+	certainly created the object.  If O_EXCL was not set,
+	FMODE_CREATE should be set if it is possible that this operation
+	created the object.
+
+	This method is only called if the last component is negative or
+	needs lookup.  Cached positive dentries are still handled by
+	f_op->open().
+
+	If the dentry provided is negative (not in-lookup) and O_CREAT
+	isn't set, then there is no guarantee of exclusive access to the
+	dentry - another thread might call ->atomic_open() on the same
+	dentry at the same time.  If needed a filesystem can ensure this
+	doesn't happen by returning 0 from ->d_revalidate when that is
+	called with LOOKUP_OPEN on a negative dentry.  This will ensure
+	that ->atomic_open() only receives an in-lookup dentry, which
+	always ensures exclusive access.
 
 ``tmpfile``
 	called in the end of O_TMPFILE open().  Optional, equivalent to
diff --git a/fs/namei.c b/fs/namei.c
index d95249dd527c..0f69abb3743b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -5007,6 +5007,9 @@ static struct file *path_openat(struct nameidata *nd,
 		error = -EINVAL;
 	}
 	fput_close(file);
+	if (error == -EISDIR &&
+	    (op->open_flag & __O_REGULAR))
+		error = -EFTYPE;
 	if (error == -EOPENSTALE) {
 		if (flags & LOOKUP_RCU)
 			error = -ECHILD;

base-commit: 9189e6a6f89e32d3a604b221ea64e67e1a35957c
-- 
2.50.0.107.gf914562f5916.dirty