* [PATCH] orangefs: stop setting atime on inode dirty
2017-10-30 21:54 ` Dave Chinner
@ 2017-11-07 20:01 ` Martin Brandenburg
2017-11-07 20:23 ` [RFC] don't update atime on file write Mike Marshall
1 sibling, 0 replies; 4+ messages in thread
From: Martin Brandenburg @ 2017-11-07 20:01 UTC (permalink / raw)
To: hubcap, david, linux-fsdevel, linux-kernel, devel; +Cc: Martin Brandenburg
The previous code path was to mark the inode dirty, let
orangefs_inode_dirty set a flag in our private inode, then later during
inode release call orangefs_flush_inode which notices the flag and
writes the atime out.
The code path worked almost identically for mtime, ctime, and mode
except that those flags are set explicitly and not as side effects of
dirty.
Now orangefs_flush_inode is removed. Marking an inode dirty does not
imply an atime update. Any place where flags were set before is now
an explicit call to orangefs_inode_setattr. Since OrangeFS does not
utilize inode writeback, the attribute change should be written out
immediately.
Fixes generic/120.
In namei.c, there are several places where the directory mtime and ctime
are set, but only the mtime is sent to the server. These don't seem
right, but I've left them as is for now.
Signed-off-by: Martin Brandenburg <martin@omnibond.com>
---
fs/orangefs/acl.c | 10 +++---
fs/orangefs/dir.c | 1 -
fs/orangefs/file.c | 16 +++++----
fs/orangefs/inode.c | 17 +++++++++
fs/orangefs/namei.c | 21 ++++++++---
fs/orangefs/orangefs-kernel.h | 31 ++--------------
fs/orangefs/orangefs-utils.c | 83 +------------------------------------------
fs/orangefs/super.c | 13 -------
fs/orangefs/symlink.c | 1 +
9 files changed, 52 insertions(+), 141 deletions(-)
diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
index 9108ef433e6d..9f9231e64375 100644
--- a/fs/orangefs/acl.c
+++ b/fs/orangefs/acl.c
@@ -154,13 +154,11 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
int orangefs_init_acl(struct inode *inode, struct inode *dir)
{
- struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
struct posix_acl *default_acl, *acl;
umode_t mode = inode->i_mode;
+ struct iattr iattr;
int error = 0;
- ClearModeFlag(orangefs_inode);
-
error = posix_acl_create(dir, &mode, &default_acl, &acl);
if (error)
return error;
@@ -179,9 +177,11 @@ int orangefs_init_acl(struct inode *inode, struct inode *dir)
/* If mode of the inode was changed, then do a forcible ->setattr */
if (mode != inode->i_mode) {
- SetModeFlag(orangefs_inode);
+ memset(&iattr, 0, sizeof iattr);
inode->i_mode = mode;
- orangefs_flush_inode(inode);
+ iattr.ia_mode = mode;
+ iattr.ia_valid |= ATTR_MODE;
+ orangefs_inode_setattr(inode, &iattr);
}
return error;
diff --git a/fs/orangefs/dir.c b/fs/orangefs/dir.c
index d327cbd17756..76e17cddbe42 100644
--- a/fs/orangefs/dir.c
+++ b/fs/orangefs/dir.c
@@ -385,7 +385,6 @@ static int orangefs_dir_release(struct inode *inode, struct file *file)
{
struct orangefs_dir *od = file->private_data;
struct orangefs_dir_part *part = od->part;
- orangefs_flush_inode(inode);
while (part) {
struct orangefs_dir_part *next = part->next;
vfree(part);
diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index 336ecbf8c268..b822d09186f0 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -382,9 +382,15 @@ static ssize_t do_readv_writev(enum ORANGEFS_io_type type, struct file *file,
if (type == ORANGEFS_IO_READ) {
file_accessed(file);
} else {
- SetMtimeFlag(orangefs_inode);
- inode->i_mtime = current_time(inode);
- mark_inode_dirty_sync(inode);
+ file_update_time(file);
+ /*
+ * Must invalidate to ensure write loop doesn't
+ * prevent kernel from reading updated
+ * attribute. Size probably changed because of
+ * the write, and other clients could update
+ * any other attribute.
+ */
+ orangefs_inode->getattr_time = jiffies - 1;
}
}
@@ -614,8 +620,6 @@ static int orangefs_file_release(struct inode *inode, struct file *file)
"orangefs_file_release: called on %pD\n",
file);
- orangefs_flush_inode(inode);
-
/*
* remove all associated inode pages from the page cache and
* readahead cache (if any); this forces an expensive refresh of
@@ -665,8 +669,6 @@ static int orangefs_fsync(struct file *file,
ret);
op_release(new_op);
-
- orangefs_flush_inode(file_inode(file));
return ret;
}
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 9428ea0aac16..fa8a11888d46 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -289,6 +289,22 @@ int orangefs_permission(struct inode *inode, int mask)
return generic_permission(inode, mask);
}
+int orangefs_update_time(struct inode *inode, struct timespec *time, int flags)
+{
+ struct iattr iattr;
+ gossip_debug(GOSSIP_INODE_DEBUG, "orangefs_update_time: %pU\n",
+ get_khandle_from_ino(inode));
+ generic_update_time(inode, time, flags);
+ memset(&iattr, 0, sizeof iattr);
+ if (flags & S_ATIME)
+ iattr.ia_valid |= ATTR_ATIME;
+ if (flags & S_CTIME)
+ iattr.ia_valid |= ATTR_CTIME;
+ if (flags & S_MTIME)
+ iattr.ia_valid |= ATTR_MTIME;
+ return orangefs_inode_setattr(inode, &iattr);
+}
+
/* ORANGEDS2 implementation of VFS inode operations for files */
const struct inode_operations orangefs_file_inode_operations = {
.get_acl = orangefs_get_acl,
@@ -297,6 +313,7 @@ const struct inode_operations orangefs_file_inode_operations = {
.getattr = orangefs_getattr,
.listxattr = orangefs_listxattr,
.permission = orangefs_permission,
+ .update_time = orangefs_update_time,
};
static int orangefs_init_iops(struct inode *inode)
diff --git a/fs/orangefs/namei.c b/fs/orangefs/namei.c
index 478e88bd7f9d..d798b4cd1b23 100644
--- a/fs/orangefs/namei.c
+++ b/fs/orangefs/namei.c
@@ -22,6 +22,7 @@ static int orangefs_create(struct inode *dir,
struct orangefs_inode_s *parent = ORANGEFS_I(dir);
struct orangefs_kernel_op_s *new_op;
struct inode *inode;
+ struct iattr iattr;
int ret;
gossip_debug(GOSSIP_NAME_DEBUG, "%s: %pd\n",
@@ -81,8 +82,10 @@ static int orangefs_create(struct inode *dir,
__func__,
dentry);
- SetMtimeFlag(parent);
dir->i_mtime = dir->i_ctime = current_time(dir);
+ memset(&iattr, 0, sizeof iattr);
+ iattr.ia_valid |= ATTR_MTIME;
+ orangefs_inode_setattr(dir, &iattr);
mark_inode_dirty_sync(dir);
ret = 0;
out:
@@ -220,6 +223,7 @@ static int orangefs_unlink(struct inode *dir, struct dentry *dentry)
struct inode *inode = dentry->d_inode;
struct orangefs_inode_s *parent = ORANGEFS_I(dir);
struct orangefs_kernel_op_s *new_op;
+ struct iattr iattr;
int ret;
gossip_debug(GOSSIP_NAME_DEBUG,
@@ -252,8 +256,10 @@ static int orangefs_unlink(struct inode *dir, struct dentry *dentry)
if (!ret) {
drop_nlink(inode);
- SetMtimeFlag(parent);
dir->i_mtime = dir->i_ctime = current_time(dir);
+ memset(&iattr, 0, sizeof iattr);
+ iattr.ia_valid |= ATTR_MTIME;
+ orangefs_inode_setattr(dir, &iattr);
mark_inode_dirty_sync(dir);
}
return ret;
@@ -266,6 +272,7 @@ static int orangefs_symlink(struct inode *dir,
struct orangefs_inode_s *parent = ORANGEFS_I(dir);
struct orangefs_kernel_op_s *new_op;
struct inode *inode;
+ struct iattr iattr;
int mode = 755;
int ret;
@@ -330,8 +337,10 @@ static int orangefs_symlink(struct inode *dir,
get_khandle_from_ino(inode),
dentry);
- SetMtimeFlag(parent);
dir->i_mtime = dir->i_ctime = current_time(dir);
+ memset(&iattr, 0, sizeof iattr);
+ iattr.ia_valid |= ATTR_MTIME;
+ orangefs_inode_setattr(dir, &iattr);
mark_inode_dirty_sync(dir);
ret = 0;
out:
@@ -344,6 +353,7 @@ static int orangefs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode
struct orangefs_inode_s *parent = ORANGEFS_I(dir);
struct orangefs_kernel_op_s *new_op;
struct inode *inode;
+ struct iattr iattr;
int ret;
new_op = op_alloc(ORANGEFS_VFS_OP_MKDIR);
@@ -399,8 +409,10 @@ static int orangefs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode
* NOTE: we have no good way to keep nlink consistent for directories
* across clients; keep constant at 1.
*/
- SetMtimeFlag(parent);
dir->i_mtime = dir->i_ctime = current_time(dir);
+ memset(&iattr, 0, sizeof iattr);
+ iattr.ia_valid |= ATTR_MTIME;
+ orangefs_inode_setattr(dir, &iattr);
mark_inode_dirty_sync(dir);
out:
op_release(new_op);
@@ -469,4 +481,5 @@ const struct inode_operations orangefs_dir_inode_operations = {
.getattr = orangefs_getattr,
.listxattr = orangefs_listxattr,
.permission = orangefs_permission,
+ .update_time = orangefs_update_time,
};
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index ea0ce507a6ab..ff56074e729c 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -208,37 +208,10 @@ struct orangefs_inode_s {
struct inode vfs_inode;
sector_t last_failed_block_index_read;
- /*
- * State of in-memory attributes not yet flushed to disk associated
- * with this object
- */
- unsigned long pinode_flags;
-
unsigned long getattr_time;
u32 getattr_mask;
};
-#define P_ATIME_FLAG 0
-#define P_MTIME_FLAG 1
-#define P_CTIME_FLAG 2
-#define P_MODE_FLAG 3
-
-#define ClearAtimeFlag(pinode) clear_bit(P_ATIME_FLAG, &(pinode)->pinode_flags)
-#define SetAtimeFlag(pinode) set_bit(P_ATIME_FLAG, &(pinode)->pinode_flags)
-#define AtimeFlag(pinode) test_bit(P_ATIME_FLAG, &(pinode)->pinode_flags)
-
-#define ClearMtimeFlag(pinode) clear_bit(P_MTIME_FLAG, &(pinode)->pinode_flags)
-#define SetMtimeFlag(pinode) set_bit(P_MTIME_FLAG, &(pinode)->pinode_flags)
-#define MtimeFlag(pinode) test_bit(P_MTIME_FLAG, &(pinode)->pinode_flags)
-
-#define ClearCtimeFlag(pinode) clear_bit(P_CTIME_FLAG, &(pinode)->pinode_flags)
-#define SetCtimeFlag(pinode) set_bit(P_CTIME_FLAG, &(pinode)->pinode_flags)
-#define CtimeFlag(pinode) test_bit(P_CTIME_FLAG, &(pinode)->pinode_flags)
-
-#define ClearModeFlag(pinode) clear_bit(P_MODE_FLAG, &(pinode)->pinode_flags)
-#define SetModeFlag(pinode) set_bit(P_MODE_FLAG, &(pinode)->pinode_flags)
-#define ModeFlag(pinode) test_bit(P_MODE_FLAG, &(pinode)->pinode_flags)
-
/* per superblock private orangefs info */
struct orangefs_sb_info_s {
struct orangefs_khandle root_khandle;
@@ -441,6 +414,8 @@ int orangefs_getattr(const struct path *path, struct kstat *stat,
int orangefs_permission(struct inode *inode, int mask);
+int orangefs_update_time(struct inode *, struct timespec *, int);
+
/*
* defined in xattr.c
*/
@@ -483,8 +458,6 @@ bool __is_daemon_in_service(void);
*/
__s32 fsid_of_op(struct orangefs_kernel_op_s *op);
-int orangefs_flush_inode(struct inode *inode);
-
ssize_t orangefs_inode_getxattr(struct inode *inode,
const char *name,
void *buffer,
diff --git a/fs/orangefs/orangefs-utils.c b/fs/orangefs/orangefs-utils.c
index aab6f1842963..70608fe2689f 100644
--- a/fs/orangefs/orangefs-utils.c
+++ b/fs/orangefs/orangefs-utils.c
@@ -436,89 +436,8 @@ int orangefs_inode_setattr(struct inode *inode, struct iattr *iattr)
op_release(new_op);
- /*
- * successful setattr should clear the atime, mtime and
- * ctime flags.
- */
- if (ret == 0) {
- ClearAtimeFlag(orangefs_inode);
- ClearMtimeFlag(orangefs_inode);
- ClearCtimeFlag(orangefs_inode);
- ClearModeFlag(orangefs_inode);
+ if (ret == 0)
orangefs_inode->getattr_time = jiffies - 1;
- }
-
- return ret;
-}
-
-int orangefs_flush_inode(struct inode *inode)
-{
- /*
- * If it is a dirty inode, this function gets called.
- * Gather all the information that needs to be setattr'ed
- * Right now, this will only be used for mode, atime, mtime
- * and/or ctime.
- */
- struct iattr wbattr;
- int ret;
- int mtime_flag;
- int ctime_flag;
- int atime_flag;
- int mode_flag;
- struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
-
- memset(&wbattr, 0, sizeof(wbattr));
-
- /*
- * check inode flags up front, and clear them if they are set. This
- * will prevent multiple processes from all trying to flush the same
- * inode if they call close() simultaneously
- */
- mtime_flag = MtimeFlag(orangefs_inode);
- ClearMtimeFlag(orangefs_inode);
- ctime_flag = CtimeFlag(orangefs_inode);
- ClearCtimeFlag(orangefs_inode);
- atime_flag = AtimeFlag(orangefs_inode);
- ClearAtimeFlag(orangefs_inode);
- mode_flag = ModeFlag(orangefs_inode);
- ClearModeFlag(orangefs_inode);
-
- /* -- Lazy atime,mtime and ctime update --
- * Note: all times are dictated by server in the new scheme
- * and not by the clients
- *
- * Also mode updates are being handled now..
- */
-
- if (mtime_flag)
- wbattr.ia_valid |= ATTR_MTIME;
- if (ctime_flag)
- wbattr.ia_valid |= ATTR_CTIME;
- if (atime_flag)
- wbattr.ia_valid |= ATTR_ATIME;
-
- if (mode_flag) {
- wbattr.ia_mode = inode->i_mode;
- wbattr.ia_valid |= ATTR_MODE;
- }
-
- gossip_debug(GOSSIP_UTILS_DEBUG,
- "*********** orangefs_flush_inode: %pU "
- "(ia_valid %d)\n",
- get_khandle_from_ino(inode),
- wbattr.ia_valid);
- if (wbattr.ia_valid == 0) {
- gossip_debug(GOSSIP_UTILS_DEBUG,
- "orangefs_flush_inode skipping setattr()\n");
- return 0;
- }
-
- gossip_debug(GOSSIP_UTILS_DEBUG,
- "orangefs_flush_inode (%pU) writing mode %o\n",
- get_khandle_from_ino(inode),
- inode->i_mode);
-
- ret = orangefs_inode_setattr(inode, &wbattr);
return ret;
}
diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c
index 47f3fb9cbec4..eeb4181ee5b7 100644
--- a/fs/orangefs/super.c
+++ b/fs/orangefs/super.c
@@ -118,7 +118,6 @@ static struct inode *orangefs_alloc_inode(struct super_block *sb)
orangefs_inode->refn.fs_id = ORANGEFS_FS_ID_NULL;
orangefs_inode->last_failed_block_index_read = 0;
memset(orangefs_inode->link_target, 0, sizeof(orangefs_inode->link_target));
- orangefs_inode->pinode_flags = 0;
gossip_debug(GOSSIP_SUPER_DEBUG,
"orangefs_alloc_inode: allocated %p\n",
@@ -298,21 +297,9 @@ void fsid_key_table_finalize(void)
{
}
-/* Called whenever the VFS dirties the inode in response to atime updates */
-static void orangefs_dirty_inode(struct inode *inode, int flags)
-{
- struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
-
- gossip_debug(GOSSIP_SUPER_DEBUG,
- "orangefs_dirty_inode: %pU\n",
- get_khandle_from_ino(inode));
- SetAtimeFlag(orangefs_inode);
-}
-
static const struct super_operations orangefs_s_ops = {
.alloc_inode = orangefs_alloc_inode,
.destroy_inode = orangefs_destroy_inode,
- .dirty_inode = orangefs_dirty_inode,
.drop_inode = generic_delete_inode,
.statfs = orangefs_statfs,
.remount_fs = orangefs_remount_fs,
diff --git a/fs/orangefs/symlink.c b/fs/orangefs/symlink.c
index 02b1bbdbcc42..43b778308db3 100644
--- a/fs/orangefs/symlink.c
+++ b/fs/orangefs/symlink.c
@@ -14,4 +14,5 @@ const struct inode_operations orangefs_symlink_inode_operations = {
.getattr = orangefs_getattr,
.listxattr = orangefs_listxattr,
.permission = orangefs_permission,
+ .update_time = orangefs_update_time,
};
--
2.15.0
^ permalink raw reply related [flat|nested] 4+ messages in thread