[PATCH v5 00/19] vfs: add the ability to retry on ESTALE to several syscalls

[PATCH v5 00/19] vfs: add the ability to retry on ESTALE to several syscalls

[Jeff Layton]

This patchset is a respin of the one I sent on July 26th. The main
reason for the resend is to deal with some recent changes in namei.c
that created some merge conflicts.

This series depends on the "audit" series that I also sent on July 26th.
That set didn't need any changes, so I'm not planning to resend it.

This set is also available via the "estale" branch of my git tree:

    git://git.samba.org/jlayton/linux.git estale

I'd like to see this go in for 3.7 if at all possible.

The original cover letter text follows:

ESTALE errors are a source of pain for many users, primarily those who
are doing work on NFS. When userspace provides a path to a syscall, then
there's really little excuse for returning ESTALE. If userspace gave us
a path that we had to lookup in order to do the call, then it's not
particularly helpful to return ESTALE just because that path went stale
before we could do the actual operation.

We can and should do better here. The kernel should instead catch that
error and retry the lookup and call, while forcing a revalidation of all
dentries involved.

Unfortunately fixing this requires touching the syscalls themselves, or
at least their immediate helper functions. Not all syscalls can be
retried -- only those that take a pathname as an argument.

With this patchset, I've decided to take the relatively less
controversial approach of just having the kernel retry once when it gets
an ESTALE error. I still think that it's not as strong as it should be,
but it should improve the situation in many common cases.

I've also tried to engineer this in such a way that if we do decide that
we need to retry more than once, then it should be easy to change that
later. This should cover all of the syscalls in fs/stat.c and
fs/namei.c, and a few from fs/open.c.

Once these are merged, I'll look at adding similar handling to other
path-based syscalls in a later set. A quick look shows that we have
about 50-odd path-based syscalls that will need similar handling, so
this is just a start.

Jeff Layton (19):
  vfs: add a retry_estale helper function to handle retries on ESTALE
  vfs: add a kern_path_at function
  vfs: make fstatat retry on ESTALE errors from getattr call
  vfs: fix readlinkat to retry on ESTALE
  vfs: remove user_path_at_empty
  vfs: turn "empty" arg in getname_flags into a bool
  vfs: add new "reval" argument to kern_path_create
  vfs: fix mknodat to retry on ESTALE errors
  vfs: fix mkdir to retry on ESTALE errors
  vfs: fix symlinkat to retry on ESTALE errors
  vfs: fix linkat to retry on ESTALE errors
  vfs: make rmdir retry on ESTALE errors
  vfs: make do_unlinkat retry on ESTALE errors
  vfs: fix renameat to retry on ESTALE errors
  vfs: remove user_path_parent
  vfs: have do_sys_truncate retry once on an ESTALE error
  vfs: have faccessat retry once on an ESTALE error
  vfs: have chdir retry lookup and call once on ESTALE error
  vfs: make chroot retry once on ESTALE error

 drivers/base/devtmpfs.c |   7 +-
 fs/namei.c              | 357 ++++++++++++++++++++++++++++++------------------
 fs/open.c               | 234 ++++++++++++++++++-------------
 fs/stat.c               |  44 ++++--
 include/linux/fs.h      |  22 +++
 include/linux/namei.h   |   4 +-
 net/unix/af_unix.c      |   2 +-
 7 files changed, 422 insertions(+), 248 deletions(-)

-- 
1.7.11.2

[PATCH v5 01/19] vfs: add a retry_estale helper function to handle retries on ESTALE

[Jeff Layton]

This function is expected to be called from path-based syscalls to help
them decide whether to try the lookup and call again in the event that
they got an -ESTALE return back on an earier try.

Currently, we only retry the call once on an ESTALE error, but in the
event that we decide that that's not enough in the future, we should be
able to change the logic in this helper without too much effort.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 include/linux/fs.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index aa11047..b776a97 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2211,6 +2211,27 @@ extern int finish_open(struct file *file, struct dentry *dentry,
 			int *opened);
 extern int finish_no_open(struct file *file, struct dentry *dentry);
 
+/**
+ * retry_estale - determine whether the caller should retry an operation
+ *
+ * @error: the error we'll be returning
+ * @try: number of retries already performed
+ *
+ * Check to see if the error code was -ESTALE, and then determine whether
+ * to retry the call based on the number of retries so far. Currently, we only
+ * retry the call once.
+ *
+ * Returns true if the caller should try again.
+ */
+static inline bool
+retry_estale(const long error, const unsigned int try)
+{
+	if (likely(error != -ESTALE))
+		return false;
+
+	return !try;
+}
+
 /* fs/ioctl.c */
 
 extern int ioctl_preallocate(struct file *filp, void __user *argp);
-- 
1.7.11.2

[PATCH v5 02/19] vfs: add a kern_path_at function

[Jeff Layton]

This will function like user_path_at, but does not do the getname and
putname, leaving that to the caller.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c            | 27 +++++++++++++++++++--------
 include/linux/namei.h |  1 +
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index dc792e5..83a6f46 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2092,20 +2092,31 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
 	return __lookup_hash(&this, base, 0);
 }
 
+int kern_path_at(int dfd, const char *name, unsigned flags, struct path *path)
+{
+	struct nameidata nd;
+	int err;
+
+	BUG_ON(flags & LOOKUP_PARENT);
+
+	err = do_path_lookup(dfd, name, flags, &nd);
+	if (!err)
+		*path = nd.path;
+
+	return err;
+}
+
 int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
 		 struct path *path, int *empty)
 {
-	struct nameidata nd;
 	char *tmp = getname_flags(name, flags, empty);
-	int err = PTR_ERR(tmp);
-	if (!IS_ERR(tmp)) {
-
-		BUG_ON(flags & LOOKUP_PARENT);
+	int err;
 
-		err = do_path_lookup(dfd, tmp, flags, &nd);
+	if (IS_ERR(tmp)) {
+		err = PTR_ERR(tmp);
+	} else {
+		err = kern_path_at(dfd, tmp, flags, path);
 		putname(tmp);
-		if (!err)
-			*path = nd.path;
 	}
 	return err;
 }
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 4bf19d8..482f87f 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -55,6 +55,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 #define LOOKUP_ROOT		0x2000
 #define LOOKUP_EMPTY		0x4000
 
+extern int kern_path_at(int, const char *, unsigned, struct path *);
 extern int user_path_at(int, const char __user *, unsigned, struct path *);
 extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
 
-- 
1.7.11.2

[PATCH v5 03/19] vfs: make fstatat retry on ESTALE errors from getattr call

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c         |  2 +-
 fs/stat.c          | 21 ++++++++++++++++-----
 include/linux/fs.h |  1 +
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 83a6f46..e66161f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -117,7 +117,7 @@
  * POSIX.1 2.4: an empty pathname is invalid (ENOENT).
  * PATH_MAX includes the nul terminator --RR.
  */
-static char *getname_flags(const char __user *filename, int flags, int *empty)
+char *getname_flags(const char __user *filename, int flags, int *empty)
 {
 	char *result = __getname(), *err;
 	int len;
diff --git a/fs/stat.c b/fs/stat.c
index b6ff118..5afeb37 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -72,9 +72,11 @@ EXPORT_SYMBOL(vfs_fstat);
 int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat,
 		int flag)
 {
+	char *name;
 	struct path path;
 	int error = -EINVAL;
-	int lookup_flags = 0;
+	unsigned int try = 0;
+	unsigned int lookup_flags = 0;
 
 	if ((flag & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT |
 		      AT_EMPTY_PATH)) != 0)
@@ -85,12 +87,21 @@ int vfs_fstatat(int dfd, const char __user *filename, struct kstat *stat,
 	if (flag & AT_EMPTY_PATH)
 		lookup_flags |= LOOKUP_EMPTY;
 
-	error = user_path_at(dfd, filename, lookup_flags, &path);
-	if (error)
+	name = getname_flags(filename, lookup_flags, NULL);
+	if (IS_ERR(name)) {
+		error = PTR_ERR(name);
 		goto out;
+	}
+	do {
+		error = kern_path_at(dfd, name, lookup_flags, &path);
+		if (error)
+			break;
 
-	error = vfs_getattr(path.mnt, path.dentry, stat);
-	path_put(&path);
+		error = vfs_getattr(path.mnt, path.dentry, stat);
+		path_put(&path);
+		lookup_flags |= LOOKUP_REVAL;
+	} while (retry_estale(error, try++));
+	putname(name);
 out:
 	return error;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b776a97..f2e7371 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2201,6 +2201,7 @@ extern struct file *file_open_root(struct dentry *, struct vfsmount *,
 				   const char *, int);
 extern struct file * dentry_open(const struct path *, int, const struct cred *);
 extern int filp_close(struct file *, fl_owner_t id);
+extern char *getname_flags(const char __user *, int, int *);
 extern char * getname(const char __user *);
 enum {
 	FILE_CREATED = 1,
-- 
1.7.11.2

[PATCH v5 04/19] vfs: fix readlinkat to retry on ESTALE

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/stat.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/stat.c b/fs/stat.c
index 5afeb37..c9d88f7 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -307,14 +307,25 @@ SYSCALL_DEFINE4(readlinkat, int, dfd, const char __user *, pathname,
 	struct path path;
 	int error;
 	int empty = 0;
+	char *name;
+	unsigned int try = 0;
+	unsigned int lookup_flags = LOOKUP_EMPTY;
 
 	if (bufsiz <= 0)
 		return -EINVAL;
 
-	error = user_path_at_empty(dfd, pathname, LOOKUP_EMPTY, &path, &empty);
-	if (!error) {
-		struct inode *inode = path.dentry->d_inode;
+	name = getname_flags(pathname, lookup_flags, &empty);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
+
+	do {
+		struct inode *inode;
+
+		error = kern_path_at(dfd, name, lookup_flags, &path);
+		if (error)
+			break;
 
+		inode = path.dentry->d_inode;
 		error = empty ? -ENOENT : -EINVAL;
 		if (inode->i_op->readlink) {
 			error = security_inode_readlink(path.dentry);
@@ -325,7 +336,9 @@ SYSCALL_DEFINE4(readlinkat, int, dfd, const char __user *, pathname,
 			}
 		}
 		path_put(&path);
-	}
+		lookup_flags |= LOOKUP_REVAL;
+	} while (retry_estale(error, try++));
+	putname(name);
 	return error;
 }
 
-- 
1.7.11.2

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 05/19] vfs: remove user_path_at_empty

[Jeff Layton]

...there are no more callers.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/namei.c            | 12 +++---------
 include/linux/namei.h |  1 -
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index e66161f..dc58de9 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2106,10 +2106,10 @@ int kern_path_at(int dfd, const char *name, unsigned flags, struct path *path)
 	return err;
 }
 
-int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
-		 struct path *path, int *empty)
+int user_path_at(int dfd, const char __user *name, unsigned flags,
+		 struct path *path)
 {
-	char *tmp = getname_flags(name, flags, empty);
+	char *tmp = getname_flags(name, flags, NULL);
 	int err;
 
 	if (IS_ERR(tmp)) {
@@ -2121,12 +2121,6 @@ int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
 	return err;
 }
 
-int user_path_at(int dfd, const char __user *name, unsigned flags,
-		 struct path *path)
-{
-	return user_path_at_empty(dfd, name, flags, path, NULL);
-}
-
 static int user_path_parent(int dfd, const char __user *path,
 			struct nameidata *nd, char **name)
 {
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 482f87f..491fb48 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -57,7 +57,6 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
 
 extern int kern_path_at(int, const char *, unsigned, struct path *);
 extern int user_path_at(int, const char __user *, unsigned, struct path *);
-extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
 
 #define user_path(name, path) user_path_at(AT_FDCWD, name, LOOKUP_FOLLOW, path)
 #define user_lpath(name, path) user_path_at(AT_FDCWD, name, 0, path)
-- 
1.7.11.2

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 06/19] vfs: turn "empty" arg in getname_flags into a bool

[Jeff Layton]

...it's just used as a flag.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c         | 4 ++--
 fs/stat.c          | 2 +-
 include/linux/fs.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index dc58de9..f0463bc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -117,7 +117,7 @@
  * POSIX.1 2.4: an empty pathname is invalid (ENOENT).
  * PATH_MAX includes the nul terminator --RR.
  */
-char *getname_flags(const char __user *filename, int flags, int *empty)
+char *getname_flags(const char __user *filename, int flags, bool *empty)
 {
 	char *result = __getname(), *err;
 	int len;
@@ -133,7 +133,7 @@ char *getname_flags(const char __user *filename, int flags, int *empty)
 	/* The empty path is special. */
 	if (unlikely(!len)) {
 		if (empty)
-			*empty = 1;
+			*empty = true;
 		err = ERR_PTR(-ENOENT);
 		if (!(flags & LOOKUP_EMPTY))
 			goto error;
diff --git a/fs/stat.c b/fs/stat.c
index c9d88f7..4f8b6bc 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -306,7 +306,7 @@ SYSCALL_DEFINE4(readlinkat, int, dfd, const char __user *, pathname,
 {
 	struct path path;
 	int error;
-	int empty = 0;
+	bool empty = false;
 	char *name;
 	unsigned int try = 0;
 	unsigned int lookup_flags = LOOKUP_EMPTY;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f2e7371..55b3cc3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2201,7 +2201,7 @@ extern struct file *file_open_root(struct dentry *, struct vfsmount *,
 				   const char *, int);
 extern struct file * dentry_open(const struct path *, int, const struct cred *);
 extern int filp_close(struct file *, fl_owner_t id);
-extern char *getname_flags(const char __user *, int, int *);
+extern char *getname_flags(const char __user *, int, bool *);
 extern char * getname(const char __user *);
 enum {
 	FILE_CREATED = 1,
-- 
1.7.11.2

[PATCH v5 07/19] vfs: add new "reval" argument to kern_path_create

[Jeff Layton]

...for now, all of the callers pass in "false". Eventually, we'll set
that to "true" when we retry the lookup after getting back an ESTALE on
a call.

While we're at it, change the is_dir arg to a bool since that's how it's
used currently.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/base/devtmpfs.c |  7 ++++---
 fs/namei.c              | 12 +++++++++---
 include/linux/namei.h   |  2 +-
 net/unix/af_unix.c      |  2 +-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index deb4a45..2124437 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -148,7 +148,7 @@ static int dev_mkdir(const char *name, umode_t mode)
 	struct path path;
 	int err;
 
-	dentry = kern_path_create(AT_FDCWD, name, &path, 1);
+	dentry = kern_path_create(AT_FDCWD, name, &path, true, false);
 	if (IS_ERR(dentry))
 		return PTR_ERR(dentry);
 
@@ -193,10 +193,11 @@ static int handle_create(const char *nodename, umode_t mode, struct device *dev)
 	struct path path;
 	int err;
 
-	dentry = kern_path_create(AT_FDCWD, nodename, &path, 0);
+	dentry = kern_path_create(AT_FDCWD, nodename, &path, false, false);
 	if (dentry == ERR_PTR(-ENOENT)) {
 		create_path(nodename);
-		dentry = kern_path_create(AT_FDCWD, nodename, &path, 0);
+		dentry = kern_path_create(AT_FDCWD, nodename, &path,
+								false, false);
 	}
 	if (IS_ERR(dentry))
 		return PTR_ERR(dentry);
diff --git a/fs/namei.c b/fs/namei.c
index f0463bc..f04ba14 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2972,12 +2972,18 @@ struct file *do_file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 	return file;
 }
 
-struct dentry *kern_path_create(int dfd, const char *pathname, struct path *path, int is_dir)
+struct dentry *kern_path_create(int dfd, const char *pathname, struct path *path, bool is_dir, bool reval)
 {
 	struct dentry *dentry = ERR_PTR(-EEXIST);
 	struct nameidata nd;
 	int err2;
-	int error = do_path_lookup(dfd, pathname, LOOKUP_PARENT, &nd);
+	int error;
+	unsigned int lookup_flags = LOOKUP_PARENT;
+
+	if (reval)
+		lookup_flags |= LOOKUP_REVAL;
+
+	error = do_path_lookup(dfd, pathname, lookup_flags, &nd);
 	if (error)
 		return ERR_PTR(error);
 
@@ -3047,7 +3053,7 @@ struct dentry *user_path_create(int dfd, const char __user *pathname, struct pat
 	struct dentry *res;
 	if (IS_ERR(tmp))
 		return ERR_CAST(tmp);
-	res = kern_path_create(dfd, tmp, path, is_dir);
+	res = kern_path_create(dfd, tmp, path, (bool)is_dir, false);
 	putname(tmp);
 	return res;
 }
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 491fb48..24c4962 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -65,7 +65,7 @@ extern int user_path_at(int, const char __user *, unsigned, struct path *);
 
 extern int kern_path(const char *, unsigned, struct path *);
 
-extern struct dentry *kern_path_create(int, const char *, struct path *, int);
+extern struct dentry *kern_path_create(int, const char *, struct path *, bool, bool);
 extern struct dentry *user_path_create(int, const char __user *, struct path *, int);
 extern void done_path_create(struct path *, struct dentry *);
 extern struct dentry *kern_path_locked(const char *, struct path *);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e4768c1..3f61f4c 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -832,7 +832,7 @@ static int unix_mknod(const char *sun_path, umode_t mode, struct path *res)
 	 * Get the parent directory, calculate the hash for last
 	 * component.
 	 */
-	dentry = kern_path_create(AT_FDCWD, sun_path, &path, 0);
+	dentry = kern_path_create(AT_FDCWD, sun_path, &path, false, false);
 	err = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		return err;
-- 
1.7.11.2

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 08/19] vfs: fix mknodat to retry on ESTALE errors

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index f04ba14..0a0397f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3109,34 +3109,49 @@ SYSCALL_DEFINE4(mknodat, int, dfd, const char __user *, filename, umode_t, mode,
 	struct dentry *dentry;
 	struct path path;
 	int error;
+	char *name;
+	unsigned int try = 0;
 
 	error = may_mknod(mode);
 	if (error)
 		return error;
 
-	dentry = user_path_create(dfd, filename, &path, 0);
-	if (IS_ERR(dentry))
-		return PTR_ERR(dentry);
+	name = getname(filename);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
 
-	if (!IS_POSIXACL(path.dentry->d_inode))
-		mode &= ~current_umask();
-	error = security_path_mknod(&path, dentry, mode, dev);
-	if (error)
-		goto out;
-	switch (mode & S_IFMT) {
-		case 0: case S_IFREG:
-			error = vfs_create(path.dentry->d_inode,dentry,mode,true);
+	do {
+		dentry = kern_path_create(dfd, name, &path, false, try);
+		if (IS_ERR(dentry)) {
+			error = PTR_ERR(dentry);
 			break;
-		case S_IFCHR: case S_IFBLK:
-			error = vfs_mknod(path.dentry->d_inode,dentry,mode,
-					new_decode_dev(dev));
+		}
+
+		if (!IS_POSIXACL(path.dentry->d_inode))
+			mode &= ~current_umask();
+		error = security_path_mknod(&path, dentry, mode, dev);
+		if (error)
+			goto out;
+		switch (mode & S_IFMT) {
+		case 0:
+		case S_IFREG:
+			error = vfs_create(path.dentry->d_inode, dentry,
+					mode, true);
 			break;
-		case S_IFIFO: case S_IFSOCK:
-			error = vfs_mknod(path.dentry->d_inode,dentry,mode,0);
+		case S_IFCHR:
+		case S_IFBLK:
+			error = vfs_mknod(path.dentry->d_inode, dentry, mode,
+					new_decode_dev(dev));
 			break;
-	}
+		case S_IFIFO:
+		case S_IFSOCK:
+			error = vfs_mknod(path.dentry->d_inode, dentry,
+					mode, 0);
+		}
 out:
-	done_path_create(&path, dentry);
+		done_path_create(&path, dentry);
+	} while (retry_estale(error, try++));
+	putname(name);
 	return error;
 }
 
-- 
1.7.11.2

[PATCH v5 09/19] vfs: fix mkdir to retry on ESTALE errors

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 0a0397f..9fd5163 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3190,17 +3190,26 @@ SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
 	struct dentry *dentry;
 	struct path path;
 	int error;
+	char *name;
+	unsigned int try = 0;
 
-	dentry = user_path_create(dfd, pathname, &path, 1);
-	if (IS_ERR(dentry))
-		return PTR_ERR(dentry);
-
-	if (!IS_POSIXACL(path.dentry->d_inode))
-		mode &= ~current_umask();
-	error = security_path_mkdir(&path, dentry, mode);
-	if (!error)
-		error = vfs_mkdir(path.dentry->d_inode, dentry, mode);
-	done_path_create(&path, dentry);
+	name = getname(pathname);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
+	do {
+		dentry = kern_path_create(dfd, name, &path, true, try);
+		if (IS_ERR(dentry)) {
+			error = PTR_ERR(dentry);
+			break;
+		}
+		if (!IS_POSIXACL(path.dentry->d_inode))
+			mode &= ~current_umask();
+		error = security_path_mkdir(&path, dentry, mode);
+		if (!error)
+			error = vfs_mkdir(path.dentry->d_inode, dentry, mode);
+		done_path_create(&path, dentry);
+	} while (retry_estale(error, try++));
+	putname(name);
 	return error;
 }
 
-- 
1.7.11.2

[PATCH v5 10/19] vfs: fix symlinkat to retry on ESTALE errors

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 9fd5163..9986117 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3468,24 +3468,34 @@ SYSCALL_DEFINE3(symlinkat, const char __user *, oldname,
 		int, newdfd, const char __user *, newname)
 {
 	int error;
-	char *from;
+	char *from, *to;
 	struct dentry *dentry;
 	struct path path;
+	unsigned int try = 0;
 
 	from = getname(oldname);
 	if (IS_ERR(from))
 		return PTR_ERR(from);
 
-	dentry = user_path_create(newdfd, newname, &path, 0);
-	error = PTR_ERR(dentry);
-	if (IS_ERR(dentry))
-		goto out_putname;
+	to = getname(newname);
+	if (IS_ERR(to)) {
+		putname(from);
+		return PTR_ERR(to);
+	}
 
-	error = security_path_symlink(&path, dentry, from);
-	if (!error)
-		error = vfs_symlink(path.dentry->d_inode, dentry, from);
-	done_path_create(&path, dentry);
-out_putname:
+	do {
+		dentry = kern_path_create(newdfd, to, &path, 0, try);
+		if (IS_ERR(dentry)) {
+			error = PTR_ERR(dentry);
+			break;
+		}
+
+		error = security_path_symlink(&path, dentry, from);
+		if (!error)
+			error = vfs_symlink(path.dentry->d_inode, dentry, from);
+		done_path_create(&path, dentry);
+	} while (retry_estale(error, try++));
+	putname(to);
 	putname(from);
 	return error;
 }
-- 
1.7.11.2

[PATCH v5 11/19] vfs: fix linkat to retry on ESTALE errors

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 60 ++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 9986117..d6e9754 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3565,6 +3565,8 @@ SYSCALL_DEFINE5(linkat, int, olddfd, const char __user *, oldname,
 	struct path old_path, new_path;
 	int how = 0;
 	int error;
+	char *old, *new;
+	unsigned int try = 0;
 
 	if ((flags & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH)) != 0)
 		return -EINVAL;
@@ -3582,30 +3584,48 @@ SYSCALL_DEFINE5(linkat, int, olddfd, const char __user *, oldname,
 	if (flags & AT_SYMLINK_FOLLOW)
 		how |= LOOKUP_FOLLOW;
 
-	error = user_path_at(olddfd, oldname, how, &old_path);
-	if (error)
-		return error;
+	old = getname_flags(oldname, how, NULL);
+	if (IS_ERR(old))
+		return PTR_ERR(old);
 
-	new_dentry = user_path_create(newdfd, newname, &new_path, 0);
-	error = PTR_ERR(new_dentry);
-	if (IS_ERR(new_dentry))
-		goto out;
+	new = getname(newname);
+	if (IS_ERR(new)) {
+		putname(old);
+		return PTR_ERR(new);
+	}
 
-	error = -EXDEV;
-	if (old_path.mnt != new_path.mnt)
-		goto out_dput;
-	error = may_linkat(&old_path);
-	if (unlikely(error))
-		goto out_dput;
-	error = security_path_link(old_path.dentry, &new_path, new_dentry);
-	if (error)
-		goto out_dput;
-	error = vfs_link(old_path.dentry, new_path.dentry->d_inode, new_dentry);
+	do {
+		error = kern_path_at(olddfd, old, how, &old_path);
+		if (error)
+			break;
+
+		new_dentry = kern_path_create(newdfd, new, &new_path, 0, try);
+		error = PTR_ERR(new_dentry);
+		if (IS_ERR(new_dentry)) {
+			path_put(&old_path);
+			break;
+		}
+
+		error = -EXDEV;
+		if (old_path.mnt != new_path.mnt)
+			goto out_dput;
+		error = may_linkat(&old_path);
+		if (unlikely(error))
+			goto out_dput;
+		error = security_path_link(old_path.dentry, &new_path,
+						new_dentry);
+		if (error)
+			goto out_dput;
+		error = vfs_link(old_path.dentry, new_path.dentry->d_inode,
+						new_dentry);
 out_dput:
-	done_path_create(&new_path, new_dentry);
+		done_path_create(&new_path, new_dentry);
 out:
-	path_put(&old_path);
-
+		path_put(&old_path);
+		how |= LOOKUP_REVAL;
+	} while (retry_estale(error, try++));
+	putname(new);
+	putname(old);
 	return error;
 }
 
-- 
1.7.11.2

[PATCH v5 12/19] vfs: make rmdir retry on ESTALE errors

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 83 +++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index d6e9754..7eb59cc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3282,51 +3282,62 @@ out:
 static long do_rmdir(int dfd, const char __user *pathname)
 {
 	int error = 0;
-	char * name;
+	char *name;
 	struct dentry *dentry;
 	struct nameidata nd;
+	unsigned int try = 0;
+	unsigned int lookup_flags = LOOKUP_PARENT;
 
-	error = user_path_parent(dfd, pathname, &nd, &name);
-	if (error)
-		return error;
+	name = getname(pathname);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
 
-	switch(nd.last_type) {
-	case LAST_DOTDOT:
-		error = -ENOTEMPTY;
-		goto exit1;
-	case LAST_DOT:
-		error = -EINVAL;
-		goto exit1;
-	case LAST_ROOT:
-		error = -EBUSY;
-		goto exit1;
-	}
+	do {
+		error = do_path_lookup(dfd, name, lookup_flags, &nd);
+		if (error)
+			break;
 
-	nd.flags &= ~LOOKUP_PARENT;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto exit1;
+		switch (nd.last_type) {
+		case LAST_DOTDOT:
+			error = -ENOTEMPTY;
+			goto exit1;
+		case LAST_DOT:
+			error = -EINVAL;
+			goto exit1;
+		case LAST_ROOT:
+			error = -EBUSY;
+			goto exit1;
+		}
 
-	mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
-	dentry = lookup_hash(&nd);
-	error = PTR_ERR(dentry);
-	if (IS_ERR(dentry))
-		goto exit2;
-	if (!dentry->d_inode) {
-		error = -ENOENT;
-		goto exit3;
-	}
-	error = security_path_rmdir(&nd.path, dentry);
-	if (error)
-		goto exit3;
-	error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
+		nd.flags &= ~LOOKUP_PARENT;
+		error = mnt_want_write(nd.path.mnt);
+		if (error)
+			goto exit1;
+
+		mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex,
+					I_MUTEX_PARENT);
+		dentry = lookup_hash(&nd);
+		if (IS_ERR(dentry)) {
+			error = PTR_ERR(dentry);
+			goto exit2;
+		}
+		if (!dentry->d_inode) {
+			error = -ENOENT;
+			goto exit3;
+		}
+		error = security_path_rmdir(&nd.path, dentry);
+		if (error)
+			goto exit3;
+		error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
 exit3:
-	dput(dentry);
+		dput(dentry);
 exit2:
-	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
-	mnt_drop_write(nd.path.mnt);
+		mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
+		mnt_drop_write(nd.path.mnt);
 exit1:
-	path_put(&nd.path);
+		path_put(&nd.path);
+		lookup_flags |= LOOKUP_REVAL;
+	} while (retry_estale(error, try++));
 	putname(name);
 	return error;
 }
-- 
1.7.11.2

[PATCH v5 13/19] vfs: make do_unlinkat retry on ESTALE errors

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 7eb59cc..0c112c3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3392,8 +3392,14 @@ static long do_unlinkat(int dfd, const char __user *pathname)
 	struct dentry *dentry;
 	struct nameidata nd;
 	struct inode *inode = NULL;
+	unsigned int try = 0;
+	unsigned int lookup_flags = LOOKUP_PARENT;
 
-	error = user_path_parent(dfd, pathname, &nd, &name);
+	name = getname(pathname);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
+retry:
+	error = do_path_lookup(dfd, name, lookup_flags, &nd);
 	if (error)
 		return error;
 
@@ -3430,6 +3436,10 @@ exit2:
 	mnt_drop_write(nd.path.mnt);
 exit1:
 	path_put(&nd.path);
+	if (retry_estale(error, try++)) {
+		lookup_flags |= LOOKUP_REVAL;
+		goto retry;
+	}
 	putname(name);
 	return error;
 
-- 
1.7.11.2

[PATCH v5 14/19] vfs: fix renameat to retry on ESTALE errors

[Jeff Layton]

...as always, rename is the messiest of the bunch. We have to track
whether to retry or not via a separate flag since the error handling
is already quite complex.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 0c112c3..936591f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3817,12 +3817,25 @@ SYSCALL_DEFINE4(renameat, int, olddfd, const char __user *, oldname,
 	char *from;
 	char *to;
 	int error;
+	unsigned int try = 0;
+	bool should_retry = false;
+	unsigned int lookup_flags = LOOKUP_PARENT;
 
-	error = user_path_parent(olddfd, oldname, &oldnd, &from);
-	if (error)
+	from = getname(oldname);
+	if (IS_ERR(from))
+		return PTR_ERR(from);
+
+	to = getname(newname);
+	if (IS_ERR(to)) {
+		error = PTR_ERR(to);
 		goto exit;
+	}
+retry:
+	error = do_path_lookup(olddfd, from, lookup_flags, &oldnd);
+	if (error)
+		goto exit0;
 
-	error = user_path_parent(newdfd, newname, &newnd, &to);
+	error = do_path_lookup(newdfd, to, lookup_flags, &newnd);
 	if (error)
 		goto exit1;
 
@@ -3891,13 +3904,21 @@ exit4:
 exit3:
 	unlock_rename(new_dir, old_dir);
 	mnt_drop_write(oldnd.path.mnt);
+	if (retry_estale(error, try++))
+		should_retry = true;
 exit2:
 	path_put(&newnd.path);
-	putname(to);
 exit1:
 	path_put(&oldnd.path);
-	putname(from);
+	if (should_retry) {
+		should_retry = false;
+		lookup_flags |= LOOKUP_REVAL;
+		goto retry;
+	}
+exit0:
+	putname(to);
 exit:
+	putname(from);
 	return error;
 }
 
-- 
1.7.11.2

[PATCH v5 15/19] vfs: remove user_path_parent

[Jeff Layton]

...there are no more callers.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/namei.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 936591f..ca1496a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2121,24 +2121,6 @@ int user_path_at(int dfd, const char __user *name, unsigned flags,
 	return err;
 }
 
-static int user_path_parent(int dfd, const char __user *path,
-			struct nameidata *nd, char **name)
-{
-	char *s = getname(path);
-	int error;
-
-	if (IS_ERR(s))
-		return PTR_ERR(s);
-
-	error = do_path_lookup(dfd, s, LOOKUP_PARENT, nd);
-	if (error)
-		putname(s);
-	else
-		*name = s;
-
-	return error;
-}
-
 /*
  * It's inline, so penalty for filesystems that don't use sticky bit is
  * minimal.
-- 
1.7.11.2

[PATCH v5 16/19] vfs: have do_sys_truncate retry once on an ESTALE error

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/open.c | 92 +++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 7babeed..0df0c5d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -66,62 +66,72 @@ static long do_sys_truncate(const char __user *pathname, loff_t length)
 	struct path path;
 	struct inode *inode;
 	int error;
+	unsigned int lookup_flags = LOOKUP_FOLLOW;
+	unsigned int try = 0;
+	char *name;
 
-	error = -EINVAL;
 	if (length < 0)	/* sorry, but loff_t says... */
-		goto out;
+		return -EINVAL;
 
-	error = user_path(pathname, &path);
-	if (error)
-		goto out;
-	inode = path.dentry->d_inode;
+	name = getname_flags(pathname, lookup_flags, NULL);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
 
-	/* For directories it's -EISDIR, for other non-regulars - -EINVAL */
-	error = -EISDIR;
-	if (S_ISDIR(inode->i_mode))
-		goto dput_and_out;
+	do {
+		error = kern_path_at(AT_FDCWD, name, lookup_flags, &path);
+		if (error)
+			break;
+		inode = path.dentry->d_inode;
 
-	error = -EINVAL;
-	if (!S_ISREG(inode->i_mode))
-		goto dput_and_out;
+		/* For dirs, -EISDIR. For other non-regulars, -EINVAL */
+		error = -EISDIR;
+		if (S_ISDIR(inode->i_mode))
+			goto dput_and_out;
 
-	error = mnt_want_write(path.mnt);
-	if (error)
-		goto dput_and_out;
+		error = -EINVAL;
+		if (!S_ISREG(inode->i_mode))
+			goto dput_and_out;
 
-	error = inode_permission(inode, MAY_WRITE);
-	if (error)
-		goto mnt_drop_write_and_out;
+		error = mnt_want_write(path.mnt);
+		if (error)
+			goto dput_and_out;
 
-	error = -EPERM;
-	if (IS_APPEND(inode))
-		goto mnt_drop_write_and_out;
+		error = inode_permission(inode, MAY_WRITE);
+		if (error)
+			goto mnt_drop_write_and_out;
 
-	error = get_write_access(inode);
-	if (error)
-		goto mnt_drop_write_and_out;
+		error = -EPERM;
+		if (IS_APPEND(inode))
+			goto mnt_drop_write_and_out;
 
-	/*
-	 * Make sure that there are no leases.  get_write_access() protects
-	 * against the truncate racing with a lease-granting setlease().
-	 */
-	error = break_lease(inode, O_WRONLY);
-	if (error)
-		goto put_write_and_out;
+		error = get_write_access(inode);
+		if (error)
+			goto mnt_drop_write_and_out;
 
-	error = locks_verify_truncate(inode, NULL, length);
-	if (!error)
-		error = security_path_truncate(&path);
-	if (!error)
-		error = do_truncate(path.dentry, length, 0, NULL);
+		/*
+		 * Make sure that there are no leases. get_write_access()
+		 * protects against the truncate racing with a lease-granting
+		 * setlease().
+		 */
+		error = break_lease(inode, O_WRONLY);
+		if (error)
+			goto put_write_and_out;
+
+		error = locks_verify_truncate(inode, NULL, length);
+		if (!error)
+			error = security_path_truncate(&path);
+		if (!error)
+			error = do_truncate(path.dentry, length, 0, NULL);
 
 put_write_and_out:
-	put_write_access(inode);
+		put_write_access(inode);
 mnt_drop_write_and_out:
-	mnt_drop_write(path.mnt);
+		mnt_drop_write(path.mnt);
 dput_and_out:
-	path_put(&path);
-out:
+		path_put(&path);
+		lookup_flags |= LOOKUP_REVAL;
+	} while (retry_estale(error, try++));
+	putname(name);
 	return error;
 }
 
-- 
1.7.11.2

[PATCH v5 17/19] vfs: have faccessat retry once on an ESTALE error

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/open.c | 70 ++++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 0df0c5d..147b232 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -318,6 +318,9 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
 	struct path path;
 	struct inode *inode;
 	int res;
+	unsigned int lookup_flags = LOOKUP_FOLLOW;
+	unsigned int try = 0;
+	char *name;
 
 	if (mode & ~S_IRWXO)	/* where's F_OK, X_OK, W_OK, R_OK? */
 		return -EINVAL;
@@ -339,44 +342,51 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
 				override_cred->cap_permitted;
 	}
 
+	name = getname_flags(filename, lookup_flags, NULL);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
+
 	old_cred = override_creds(override_cred);
 
-	res = user_path_at(dfd, filename, LOOKUP_FOLLOW, &path);
-	if (res)
-		goto out;
+	do {
+		res = kern_path_at(dfd, name, lookup_flags, &path);
+		if (res)
+			break;
 
-	inode = path.dentry->d_inode;
+		inode = path.dentry->d_inode;
 
-	if ((mode & MAY_EXEC) && S_ISREG(inode->i_mode)) {
+		if ((mode & MAY_EXEC) && S_ISREG(inode->i_mode)) {
+			/*
+			 * MAY_EXEC on regular files is denied if the fs is
+			 * mounted with the "noexec" flag.
+			 */
+			res = -EACCES;
+			if (path.mnt->mnt_flags & MNT_NOEXEC)
+				goto out_path_release;
+		}
+
+		res = inode_permission(inode, mode | MAY_ACCESS);
+		/* SuS v2 requires we report a read only fs too */
+		if (res || !(mode & S_IWOTH) || special_file(inode->i_mode))
+			goto out_path_release;
 		/*
-		 * MAY_EXEC on regular files is denied if the fs is mounted
-		 * with the "noexec" flag.
+		 * This is a rare case where using __mnt_is_readonly()
+		 * is OK without a mnt_want/drop_write() pair.  Since
+		 * no actual write to the fs is performed here, we do
+		 * not need to telegraph to that to anyone.
+		 *
+		 * By doing this, we accept that this access is
+		 * inherently racy and know that the fs may change
+		 * state before we even see this result.
 		 */
-		res = -EACCES;
-		if (path.mnt->mnt_flags & MNT_NOEXEC)
-			goto out_path_release;
-	}
-
-	res = inode_permission(inode, mode | MAY_ACCESS);
-	/* SuS v2 requires we report a read only fs too */
-	if (res || !(mode & S_IWOTH) || special_file(inode->i_mode))
-		goto out_path_release;
-	/*
-	 * This is a rare case where using __mnt_is_readonly()
-	 * is OK without a mnt_want/drop_write() pair.  Since
-	 * no actual write to the fs is performed here, we do
-	 * not need to telegraph to that to anyone.
-	 *
-	 * By doing this, we accept that this access is
-	 * inherently racy and know that the fs may change
-	 * state before we even see this result.
-	 */
-	if (__mnt_is_readonly(path.mnt))
-		res = -EROFS;
+		if (__mnt_is_readonly(path.mnt))
+			res = -EROFS;
 
 out_path_release:
-	path_put(&path);
-out:
+		path_put(&path);
+		lookup_flags |= LOOKUP_REVAL;
+	} while (retry_estale(res, try++));
+	putname(name);
 	revert_creds(old_cred);
 	put_cred(override_cred);
 	return res;
-- 
1.7.11.2

[PATCH v5 18/19] vfs: have chdir retry lookup and call once on ESTALE error

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/open.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 147b232..59a7e9d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -401,20 +401,27 @@ SYSCALL_DEFINE1(chdir, const char __user *, filename)
 {
 	struct path path;
 	int error;
+	int lookup_flags = LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
+	unsigned int try = 0;
+	char *name;
 
-	error = user_path_dir(filename, &path);
-	if (error)
-		goto out;
-
-	error = inode_permission(path.dentry->d_inode, MAY_EXEC | MAY_CHDIR);
-	if (error)
-		goto dput_and_out;
+	name = getname_flags(filename, lookup_flags, NULL);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
 
-	set_fs_pwd(current->fs, &path);
+	do {
+		error = kern_path_at(AT_FDCWD, name, lookup_flags, &path);
+		if (error)
+			break;
 
-dput_and_out:
-	path_put(&path);
-out:
+		error = inode_permission(path.dentry->d_inode,
+					 MAY_EXEC | MAY_CHDIR);
+		if (!error)
+			set_fs_pwd(current->fs, &path);
+		path_put(&path);
+		lookup_flags |= LOOKUP_REVAL;
+	} while (retry_estale(error, try++));
+	putname(name);
 	return error;
 }
 
-- 
1.7.11.2

[PATCH v5 19/19] vfs: make chroot retry once on ESTALE error

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/open.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 59a7e9d..991760f 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -455,27 +455,38 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename)
 {
 	struct path path;
 	int error;
+	int lookup_flags = LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
+	unsigned int try = 0;
+	char *name;
 
-	error = user_path_dir(filename, &path);
-	if (error)
-		goto out;
+	name = getname_flags(filename, lookup_flags, NULL);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
 
-	error = inode_permission(path.dentry->d_inode, MAY_EXEC | MAY_CHDIR);
-	if (error)
-		goto dput_and_out;
+	do {
+		error = kern_path_at(AT_FDCWD, name, lookup_flags, &path);
+		if (error)
+			break;
 
-	error = -EPERM;
-	if (!capable(CAP_SYS_CHROOT))
-		goto dput_and_out;
-	error = security_path_chroot(&path);
-	if (error)
-		goto dput_and_out;
+		error = inode_permission(path.dentry->d_inode,
+					 MAY_EXEC | MAY_CHDIR);
+		if (error)
+			goto dput_and_out;
 
-	set_fs_root(current->fs, &path);
-	error = 0;
+		error = -EPERM;
+		if (!capable(CAP_SYS_CHROOT))
+			goto dput_and_out;
+		error = security_path_chroot(&path);
+		if (error)
+			goto dput_and_out;
+
+		set_fs_root(current->fs, &path);
+		error = 0;
 dput_and_out:
-	path_put(&path);
-out:
+		path_put(&path);
+		lookup_flags |= LOOKUP_REVAL;
+	} while (retry_estale(error, try++));
+	putname(name);
 	return error;
 }
 
-- 
1.7.11.2

Re: [PATCH v5 00/19] vfs: add the ability to retry on ESTALE to several syscalls

[Namjae Jeon]

Hi Jeff.

I still found ESTALE error although patching these patch-set.
Is test method correct that I try to run estale_test on each nfs
server and client at the same time ?

./estale_test
chmod: Stale NFS[  281.720000] ##### send signal from USER, SIG : 2,
estale_test(107)->estale_test(102) sys_kill
[  281.728000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(103) sys_kill
[  281.736000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(104) sys_kill
[  281.744000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(105) sys_kill
[  281.752000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(106) sys_kill
[  281.760000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(107) sys_kill
[  281.768000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(108) sys_kill
[  281.780000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(109) sys_kill
[  281.788000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(110) sys_kill
[  281.796000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(111) sys_kill
[  281.804000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(112) sys_kill
[  281.812000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(113) sys_kill
[  281.820000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(114) sys_kill
[  281.828000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(115) sys_kill
[  281.840000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(116) sys_kill
[  281.848000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(117) sys_kill
[  281.856000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(118) sys_kill
[  281.864000] ##### send signal from USER, SIG : 15,
estale_test(102)->estale_test(119) sys_kill
 file handle
VDLinux#> chdir: Stale NFS[  282.664000] ##### send signal from USER,
SIG : 2, estale_test(120)->???(102) sys_kill
 file handle

Thanks.

2012/8/8, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> This patchset is a respin of the one I sent on July 26th. The main
> reason for the resend is to deal with some recent changes in namei.c
> that created some merge conflicts.
>
> This series depends on the "audit" series that I also sent on July 26th.
> That set didn't need any changes, so I'm not planning to resend it.
>
> This set is also available via the "estale" branch of my git tree:
>
>     git://git.samba.org/jlayton/linux.git estale
>
> I'd like to see this go in for 3.7 if at all possible.
>
> The original cover letter text follows:
>
> ESTALE errors are a source of pain for many users, primarily those who
> are doing work on NFS. When userspace provides a path to a syscall, then
> there's really little excuse for returning ESTALE. If userspace gave us
> a path that we had to lookup in order to do the call, then it's not
> particularly helpful to return ESTALE just because that path went stale
> before we could do the actual operation.
>
> We can and should do better here. The kernel should instead catch that
> error and retry the lookup and call, while forcing a revalidation of all
> dentries involved.
>
> Unfortunately fixing this requires touching the syscalls themselves, or
> at least their immediate helper functions. Not all syscalls can be
> retried -- only those that take a pathname as an argument.
>
> With this patchset, I've decided to take the relatively less
> controversial approach of just having the kernel retry once when it gets
> an ESTALE error. I still think that it's not as strong as it should be,
> but it should improve the situation in many common cases.
>
> I've also tried to engineer this in such a way that if we do decide that
> we need to retry more than once, then it should be easy to change that
> later. This should cover all of the syscalls in fs/stat.c and
> fs/namei.c, and a few from fs/open.c.
>
> Once these are merged, I'll look at adding similar handling to other
> path-based syscalls in a later set. A quick look shows that we have
> about 50-odd path-based syscalls that will need similar handling, so
> this is just a start.
>
> Jeff Layton (19):
>   vfs: add a retry_estale helper function to handle retries on ESTALE
>   vfs: add a kern_path_at function
>   vfs: make fstatat retry on ESTALE errors from getattr call
>   vfs: fix readlinkat to retry on ESTALE
>   vfs: remove user_path_at_empty
>   vfs: turn "empty" arg in getname_flags into a bool
>   vfs: add new "reval" argument to kern_path_create
>   vfs: fix mknodat to retry on ESTALE errors
>   vfs: fix mkdir to retry on ESTALE errors
>   vfs: fix symlinkat to retry on ESTALE errors
>   vfs: fix linkat to retry on ESTALE errors
>   vfs: make rmdir retry on ESTALE errors
>   vfs: make do_unlinkat retry on ESTALE errors
>   vfs: fix renameat to retry on ESTALE errors
>   vfs: remove user_path_parent
>   vfs: have do_sys_truncate retry once on an ESTALE error
>   vfs: have faccessat retry once on an ESTALE error
>   vfs: have chdir retry lookup and call once on ESTALE error
>   vfs: make chroot retry once on ESTALE error
>
>  drivers/base/devtmpfs.c |   7 +-
>  fs/namei.c              | 357
> ++++++++++++++++++++++++++++++------------------
>  fs/open.c               | 234 ++++++++++++++++++-------------
>  fs/stat.c               |  44 ++++--
>  include/linux/fs.h      |  22 +++
>  include/linux/namei.h   |   4 +-
>  net/unix/af_unix.c      |   2 +-
>  7 files changed, 422 insertions(+), 248 deletions(-)
>
> --
> 1.7.11.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 00/19] vfs: add the ability to retry on ESTALE to several syscalls

[Jeff Layton]

On Thu, 9 Aug 2012 20:57:14 +0900
Namjae Jeon <linkinjeon-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Hi Jeff.
> 
> I still found ESTALE error although patching these patch-set.
> Is test method correct that I try to run estale_test on each nfs
> server and client at the same time ?
> 
> ./estale_test
> chmod: Stale NFS[  281.720000] ##### send signal from USER, SIG : 2,
> estale_test(107)->estale_test(102) sys_kill
> [  281.728000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(103) sys_kill
> [  281.736000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(104) sys_kill
> [  281.744000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(105) sys_kill
> [  281.752000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(106) sys_kill
> [  281.760000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(107) sys_kill
> [  281.768000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(108) sys_kill
> [  281.780000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(109) sys_kill
> [  281.788000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(110) sys_kill
> [  281.796000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(111) sys_kill
> [  281.804000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(112) sys_kill
> [  281.812000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(113) sys_kill
> [  281.820000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(114) sys_kill
> [  281.828000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(115) sys_kill
> [  281.840000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(116) sys_kill
> [  281.848000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(117) sys_kill
> [  281.856000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(118) sys_kill
> [  281.864000] ##### send signal from USER, SIG : 15,
> estale_test(102)->estale_test(119) sys_kill
>  file handle
> VDLinux#> chdir: Stale NFS[  282.664000] ##### send signal from USER,
> SIG : 2, estale_test(120)->???(102) sys_kill
>  file handle
> 
> Thanks.
> 

I guess you didn't read my response earlier? I'll re-post it here...

> It's a bit labor intensive, I'm afraid...
>
> Attached is a cleaned-up copy of the test program that Peter wrote to
> test his original patchset. The basic idea is to run this on both the
> client and server at the same time so they race against each other. He
> was able to run it overnight when testing with his patchset.
>
> With this patchset, that doesn't work since we're only retrying the
> lookup and call once. So, what I've been doing is modifying the program
> so that it just runs one test at a time, and sniffing traffic to see
> whether the lookups and calls are retried after an ESTALE return from
> the server. 


So, ESTALE errors are still expected when running that test. This
patchset only fixes a very specific set of circumstances where an entry
goes stale once between the lookup and the actual operation(s).
Anything outside of that, and it won't help.

That test is very aggressive, and can cause it to race multiple times.
You actually have to sniff traffic and look to see if the lookup and
call were reattempted after the ESTALE error.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html