[RFC][PATCH 0/2] Updating overlay inode i_mtime for nfsd

[RFC][PATCH 0/2] Updating overlay inode i_mtime for nfsd

[Amir Goldstein]

Jeff, Bruce,

These RFC patches fix an issue I found when running nfstest_posix
over NFS exported overlayfs.

I am posting these separately from (soon to be posted) overlayfs NFS
export support patches to get your inputs about the preferred way for
a fix.

The 1st patch is a proposal to fix overlay inode i_mtime on inode
modification. This fix is incomplete w.r.t nfstest_posix test failures.
The 2nd patch more or less reverts the 1st patch in favor of updating
overlay inode i_mtime only when nfsd would care. At least as far as
nfstest_posix is concerned this fix is complete.

I lean towards the 2nd fix, but wasn't sure, so posting both suggestions.
Another option would be to call vfs_getattr() from lease_get_mtime(),
which will get the correct i_mtime for overlayfs (upper inode i_mtime),
but wasn't sure if this is a viable option for nfsd.

Would appreciate your thoughts about the issue and about the proposed fix.

Thanks,
Amir.

Amir Goldstein (2):
  vfs: update overlay inode times on write
  vfs: update overlay inode times on lease_get_mtime()

 fs/inode.c    | 15 ++++++++++++---
 fs/internal.h |  1 +
 fs/locks.c    | 12 +++++++++++-
 3 files changed, 24 insertions(+), 4 deletions(-)

-- 
2.7.4

[RFC][PATCH 0/2] Updating overlay inode i_mtime for nfsd

[Amir Goldstein]

Jeff, Bruce,

These RFC patches fix an issue I found when running nfstest_posix
over NFS exported overlayfs.

I am posting these separately from (soon to be posted) overlayfs NFS
export support patches to get your inputs about the preferred way for
a fix.

The 1st patch is a proposal to fix overlay inode i_mtime on inode
modification. This fix is incomplete w.r.t nfstest_posix test failures.
The 2nd patch more or less reverts the 1st patch in favor of updating
overlay inode i_mtime only when nfsd would care. At least as far as
nfstest_posix is concerned this fix is complete.

I lean towards the 2nd fix, but wasn't sure, so posting both suggestions.
Another option would be to call vfs_getattr() from lease_get_mtime(),
which will get the correct i_mtime for overlayfs (upper inode i_mtime),
but wasn't sure if this is a viable option for nfsd.

Would appreciate your thoughts about the issue and about the proposed fix.

Thanks,
Amir.

Amir Goldstein (2):
  vfs: update overlay inode times on write
  vfs: update overlay inode times on lease_get_mtime()

 fs/inode.c    | 15 ++++++++++++---
 fs/internal.h |  1 +
 fs/locks.c    | 12 +++++++++++-
 3 files changed, 24 insertions(+), 4 deletions(-)

-- 
2.7.4

[RFC][PATCH 1/2] vfs: update overlay inode times on write

[Amir Goldstein]

Currently with overlayfs, the real upper inode's i_mtime is updated on
write, but not overlay vfs inode. The vfs inode's i_mtime is used by nfsd
to check if nfs client cache is stale, so updating the overlay vfs inode
i_mtime on write is required for overlayfs NFS export support.

The non uptodate mtime issue was found and verified with the
nfstest_posix test when run over NFS exported overlayfs:

 $ nfstest_posix --runtest=write
 ...
 FAIL: write - file st_mtime should be updated

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/inode.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 03102d6ef044..a252256f4e51 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1567,8 +1567,7 @@ EXPORT_SYMBOL(bmap);
 /*
  * Update times in overlayed inode from underlying real inode
  */
-static void update_ovl_inode_times(struct dentry *dentry, struct inode *inode,
-			       bool rcu)
+static void update_ovl_d_inode_times(struct dentry *dentry, bool rcu)
 {
 	struct dentry *upperdentry;
 
@@ -1585,6 +1584,7 @@ static void update_ovl_inode_times(struct dentry *dentry, struct inode *inode,
 	 * stale mtime/ctime.
 	 */
 	if (upperdentry) {
+		struct inode *inode = d_inode(dentry);
 		struct inode *realinode = d_inode(upperdentry);
 
 		if ((!timespec_equal(&inode->i_mtime, &realinode->i_mtime) ||
@@ -1607,7 +1607,8 @@ static int relatime_need_update(const struct path *path, struct inode *inode,
 	if (!(path->mnt->mnt_flags & MNT_RELATIME))
 		return 1;
 
-	update_ovl_inode_times(path->dentry, inode, rcu);
+	update_ovl_d_inode_times(path->dentry, rcu);
+
 	/*
 	 * Is mtime younger than atime? If yes, update atime:
 	 */
@@ -1876,6 +1877,8 @@ int file_update_time(struct file *file)
 	ret = update_time(inode, &now, sync_it);
 	__mnt_drop_write_file(file);
 
+	update_ovl_d_inode_times(file->f_path.dentry, false);
+
 	return ret;
 }
 EXPORT_SYMBOL(file_update_time);
-- 
2.7.4

[RFC][PATCH 2/2] vfs: update overlay inode times on lease_get_mtime()

[Amir Goldstein]

Instead of looking for all the places that update inode's m_time,
update overlay inode i_mtime just before nfsd needs to read it.

The non uptodate mtime issue was found and verified with the
nfstest_posix test when run over NFS exported overlayfs:

 $ nfstest_posix --runtest=link,write
 ...
 FAIL: link - parent directory st_mtime should be updated
 FAIL: write - file st_mtime should be updated

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/inode.c    | 10 ++++++++--
 fs/internal.h |  1 +
 fs/locks.c    | 12 +++++++++++-
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index a252256f4e51..ceb5f5cb9c79 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1595,6 +1595,14 @@ static void update_ovl_d_inode_times(struct dentry *dentry, bool rcu)
 	}
 }
 
+void update_ovl_inode_times(struct inode *inode)
+{
+	struct dentry *alias = d_find_any_alias(inode);
+
+	if (alias)
+		update_ovl_d_inode_times(alias, false);
+}
+
 /*
  * With relative atime, only update atime if the previous atime is
  * earlier than either the ctime or mtime or if at least a day has
@@ -1877,8 +1885,6 @@ int file_update_time(struct file *file)
 	ret = update_time(inode, &now, sync_it);
 	__mnt_drop_write_file(file);
 
-	update_ovl_d_inode_times(file->f_path.dentry, false);
-
 	return ret;
 }
 EXPORT_SYMBOL(file_update_time);
diff --git a/fs/internal.h b/fs/internal.h
index df262f41a0ef..a631fd49a1ee 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -122,6 +122,7 @@ extern long prune_icache_sb(struct super_block *sb, struct shrink_control *sc);
 extern void inode_add_lru(struct inode *inode);
 extern int dentry_needs_remove_privs(struct dentry *dentry);
 
+extern void update_ovl_inode_times(struct inode *inode);
 extern bool __atime_needs_update(const struct path *, struct inode *, bool);
 static inline bool atime_needs_update_rcu(const struct path *path,
 					  struct inode *inode)
diff --git a/fs/locks.c b/fs/locks.c
index 21b4dfa289ee..e2f604839aaf 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -127,6 +127,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/hashtable.h>
 #include <linux/percpu.h>
+#include "internal.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/filelock.h>
@@ -1553,6 +1554,15 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
 
 EXPORT_SYMBOL(__break_lease);
 
+static struct timespec inode_mtime(struct inode *inode)
+{
+	/* TODO: use another SB_ flag and/or pass dentry to lease_get_mtime() */
+	if (unlikely(inode->i_sb->s_flags & MS_NOREMOTELOCK))
+		update_ovl_inode_times(inode);
+
+	return inode->i_mtime;
+}
+
 /**
  *	lease_get_mtime - get the last modified time of an inode
  *	@inode: the inode
@@ -1581,7 +1591,7 @@ void lease_get_mtime(struct inode *inode, struct timespec *time)
 	if (has_lease)
 		*time = current_time(inode);
 	else
-		*time = inode->i_mtime;
+		*time = inode_mtime(inode);
 }
 
 EXPORT_SYMBOL(lease_get_mtime);
-- 
2.7.4

Re: [RFC][PATCH 0/2] Updating overlay inode i_mtime for nfsd

[Amir Goldstein]

On Wed, Jan 3, 2018 at 5:11 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Jeff, Bruce,
>
> These RFC patches fix an issue I found when running nfstest_posix
> over NFS exported overlayfs.
>

Wrong version posted. Sorry for the noise.

Amir.