Re: [PATCH-v4 1/7] vfs: split update_time() into update_time() and write_time()

Theodore Ts'o
2014-11-27T15:33:23Z

Christoph, can you take a quick look at this? I'm not sure I got the
xfs inode transaction logging correct.

Thanks!!

- Ted

commit cd58addfa340c9cf88b1f9b2d31a42e2e65c7252
Author: Theodore Ts'o <tytso@mit.edu>
Date: Thu Nov 27 10:14:27 2014 -0500

vfs: split update_time() into update_time() and write_time()

In preparation for adding support for the lazytime mount option, we
need to be able to separate out the update_time() and write_time()
inode operations. Previously, only btrfs and xfs uses update_time().

With this patch, btrfs only needs write_time(), and xfs uses
update_time() to synchronize its on-disk and in-memory timestamps.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: xfs@oss.sgi.com
Cc: linux-btrfs@vger.kernel.org
Acked-by: David Sterba <dsterba@suse.cz>

diff --git a/include/linux/fs.h b/include/linux/fs.h
index f4b0ecd..befd5d2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1545,7 +1545,8 @@ struct inode_operations {
int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
u64 len);
int (*is_readonly)(struct inode *);
- int (*update_time)(struct inode *, struct timespec *, int);
+ void (*update_time)(struct inode *);
+ int (*write_time)(struct inode *);
int (*atomic_open)(struct inode *, struct dentry *,
struct file *, unsigned open_flag,
umode_t create_mode, int *opened);
diff --git a/fs/inode.c b/fs/inode.c
index 53f0173..94bc908 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1506,9 +1506,6 @@ static int update_time(struct inode *inode, struct timespec *time, int flags)
if (ret)
return ret;
}
- if (inode->i_op->update_time)
- return inode->i_op->update_time(inode, time, flags);
-
if (flags & S_ATIME)
inode->i_atime = *time;
if (flags & S_VERSION)
@@ -1517,6 +1514,10 @@ static int update_time(struct inode *inode, struct timespec *time, int flags)
inode->i_ctime = *time;
if (flags & S_MTIME)
inode->i_mtime = *time;
+ if (inode->i_op->update_time)
+ inode->i_op->update_time(inode);
+ if (inode->i_op->write_time)
+ return inode->i_op->write_time(inode);
mark_inode_dirty_sync(inode);
return 0;
}
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index ec6dcdc..b69493d 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -983,42 +983,42 @@ xfs_vn_setattr(
return error;
}

-STATIC int
+STATIC void
xfs_vn_update_time(
- struct inode *inode,
- struct timespec *now,
- int flags)
+ struct inode *inode)
+{
+ struct xfs_inode *ip = XFS_I(inode);
+
+ trace_xfs_update_time(ip);
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ ip->i_d.di_ctime.t_sec = (__int32_t) inode->i_ctime.tv_sec;
+ ip->i_d.di_ctime.t_nsec = (__int32_t) inode->i_ctime.tv_nsec;
+
+ ip->i_d.di_mtime.t_sec = (__int32_t)inode->i_mtime.tv_sec;
+ ip->i_d.di_mtime.t_nsec = (__int32_t) inode->i_mtime.tv_nsec;
+
+ ip->i_d.di_atime.t_sec = (__int32_t) inode->i_atime.tv_sec;
+ ip->i_d.di_atime.t_nsec = (__int32_t) inode->i_atime.tv_nsec;
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+}
+
+STATIC int
+xfs_vn_write_time(
+ struct inode *inode)
{
struct xfs_inode *ip = XFS_I(inode);
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
int error;

- trace_xfs_update_time(ip);
-
+ trace_xfs_write_time(ip);
tp = xfs_trans_alloc(mp, XFS_TRANS_FSYNC_TS);
error = xfs_trans_reserve(tp, &M_RES(mp)->tr_fsyncts, 0, 0);
if (error) {
xfs_trans_cancel(tp, 0);
return error;
}
-
xfs_ilock(ip, XFS_ILOCK_EXCL);
- if (flags & S_CTIME) {
- inode->i_ctime = *now;
- ip->i_d.di_ctime.t_sec = (__int32_t)now->tv_sec;
- ip->i_d.di_ctime.t_nsec = (__int32_t)now->tv_nsec;
- }
- if (flags & S_MTIME) {
- inode->i_mtime = *now;
- ip->i_d.di_mtime.t_sec = (__int32_t)now->tv_sec;
- ip->i_d.di_mtime.t_nsec = (__int32_t)now->tv_nsec;
- }
- if (flags & S_ATIME) {
- inode->i_atime = *now;
- ip->i_d.di_atime.t_sec = (__int32_t)now->tv_sec;
- ip->i_d.di_atime.t_nsec = (__int32_t)now->tv_nsec;
- }
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
return xfs_trans_commit(tp, 0);
@@ -1130,6 +1130,7 @@ static const struct inode_operations xfs_inode_operations = {
.listxattr = xfs_vn_listxattr,
.fiemap = xfs_vn_fiemap,
.update_time = xfs_vn_update_time,
+ .write_time = xfs_vn_write_time,
};

static const struct inode_operations xfs_dir_inode_operations = {
@@ -1157,6 +1158,7 @@ static const struct inode_operations xfs_dir_inode_operations = {
.removexattr = generic_removexattr,
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
+ .write_time = xfs_vn_write_time,
.tmpfile = xfs_vn_tmpfile,
};

@@ -1185,6 +1187,7 @@ static const struct inode_operations xfs_dir_ci_inode_operations = {
.removexattr = generic_removexattr,
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
+ .write_time = xfs_vn_write_time,
.tmpfile = xfs_vn_tmpfile,
};

@@ -1199,6 +1202,7 @@ static const struct inode_operations xfs_symlink_inode_operations = {
.removexattr = generic_removexattr,
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
+ .write_time = xfs_vn_write_time,
};

STATIC void
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 51372e3..09d261c 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -677,6 +677,7 @@ DEFINE_INODE_EVENT(xfs_file_fsync);
DEFINE_INODE_EVENT(xfs_destroy_inode);
DEFINE_INODE_EVENT(xfs_evict_inode);
DEFINE_INODE_EVENT(xfs_update_time);
+DEFINE_INODE_EVENT(xfs_write_time);

DEFINE_INODE_EVENT(xfs_dquot_dqalloc);
DEFINE_INODE_EVENT(xfs_dquot_dqdetach);
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index b30753c..ee94a66 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -62,7 +62,8 @@ prototypes:
ssize_t (*listxattr) (struct dentry *, char *, size_t);
int (*removexattr) (struct dentry *, const char *);
int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start, u64 len);
- void (*update_time)(struct inode *, struct timespec *, int);
+ void (*update_time)(struct inode *);
+ void (*write_time)(struct inode *);
int (*atomic_open)(struct inode *, struct dentry *,
struct file *, unsigned open_flag,
umode_t create_mode, int *opened);
@@ -95,6 +96,7 @@ listxattr: no
removexattr: yes
fiemap: no
update_time: no
+write_time: no
atomic_open: yes
tmpfile: no
dentry_open: no
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index bd46a22..a81a0652 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5563,26 +5563,8 @@ static int btrfs_is_readonly(struct inode *inode)
return 0;
}

-/*
- * This is a copy of file_update_time. We need this so we can return error on
- * ENOSPC for updating the inode in the case of file write and mmap writes.
- */
-static int btrfs_update_time(struct inode *inode, struct timespec *now,
- int flags)
+static int btrfs_write_time(struct inode *inode)
{
- struct btrfs_root *root = BTRFS_I(inode)->root;
-
- if (btrfs_root_readonly(root))
- return -EROFS;
-
- if (flags & S_VERSION)
- inode_inc_iversion(inode);
- if (flags & S_CTIME)
- inode->i_ctime = *now;
- if (flags & S_MTIME)
- inode->i_mtime = *now;
- if (flags & S_ATIME)
- inode->i_atime = *now;
return btrfs_dirty_inode(inode);
}

@@ -9471,7 +9453,7 @@ static const struct inode_operations btrfs_dir_inode_operations = {
.get_acl = btrfs_get_acl,
.set_acl = btrfs_set_acl,
.is_readonly = btrfs_is_readonly,
- .update_time = btrfs_update_time,
+ .write_time = btrfs_write_time,
.tmpfile = btrfs_tmpfile,
};
static const struct inode_operations btrfs_dir_ro_inode_operations = {
@@ -9480,7 +9462,7 @@ static const struct inode_operations btrfs_dir_ro_inode_operations = {
.get_acl = btrfs_get_acl,
.set_acl = btrfs_set_acl,
.is_readonly = btrfs_is_readonly,
- .update_time = btrfs_update_time,
+ .write_time = btrfs_write_time,
};

static const struct file_operations btrfs_dir_file_operations = {
@@ -9551,7 +9533,7 @@ static const struct inode_operations btrfs_file_inode_operations = {
.get_acl = btrfs_get_acl,
.set_acl = btrfs_set_acl,
.is_readonly = btrfs_is_readonly,
- .update_time = btrfs_update_time,
+ .write_time = btrfs_write_time,
};
static const struct inode_operations btrfs_special_inode_operations = {
.getattr = btrfs_getattr,
@@ -9564,7 +9546,7 @@ static const struct inode_operations btrfs_special_inode_operations = {
.get_acl = btrfs_get_acl,
.set_acl = btrfs_set_acl,
.is_readonly = btrfs_is_readonly,
- .update_time = btrfs_update_time,
+ .write_time = btrfs_write_time,
};
static const struct inode_operations btrfs_symlink_inode_operations = {
.readlink = generic_readlink,
@@ -9578,7 +9560,7 @@ static const struct inode_operations btrfs_symlink_inode_operations = {
.listxattr = btrfs_listxattr,
.removexattr = btrfs_removexattr,
.is_readonly = btrfs_is_readonly,
- .update_time = btrfs_update_time,
+ .write_time = btrfs_write_time,
};

const struct dentry_operations btrfs_dentry_operations = {