Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1545898 > unrolled thread

[RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    [RFC PATCH v1 02/30] ecryptfs: remove unnecessary i_version bump Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    [RFC PATCH v1 06/30] jfs: remove initialization of i_version counter Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    [RFC PATCH v1 12/30] fat: convert to new i_version API Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    [RFC PATCH v1 07/30] nilfs2: remove inode->i_version initialization Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    [RFC PATCH v1 10/30] ntfs: remove i_version handling Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    [RFC PATCH v1 03/30] ceph: remove the bump of i_version Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    [RFC PATCH v1 15/30] btrfs: convert to new i_version API Jeff Layton <jlayton@redhat.com> - 2016-12-21 18:20 +0100
    Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization Christoph Hellwig <hch@infradead.org> - 2016-12-22 09:50 +0100
      Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and  optimization Jeff Layton <jlayton@redhat.com> - 2016-12-22 15:50 +0100

#1545898 — [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

tl;dr: I think we can greatly reduce the cost of the inode->i_version
counter, by exploiting the fact that we don't need to increment it
if no one is looking at it. We can also clean up the code to prepare
to eventually expose this value via statx().

The inode->i_version field is supposed to be a value that changes
whenever there is any data or metadata change to the inode. Some
filesystems use it internally to detect directory changes during
readdir. knfsd will use it if the filesystem has MS_I_VERSION
set. IMA will also use it (though it's not clear to me that that
works 100% -- no check for MS_I_VERSION there).

Only btrfs, ext4, and xfs implement it for data changes. Because of
this, these filesystems must log the inode to disk whenever the
i_version counter changes. That has a non-zero performance impact,
especially on write-heavy workloads, because we end up dirtying the
inode metadata on every write, not just when the times change. [1]

It turns out though that none of these users of i_version require that
i_version change on every change to the file. The only real requirement
is that it be different if _something_ changed since the last time we
queried for it. [2]

So, if we simply keep track of when something queries the value, we
can avoid bumping the counter and that metadata update.

This patchset implements this:

It starts with some small cleanup patches to just remove any mention of
the i_version counter in filesystems that don't actually use it.

Then, we add a new set of static inlines for managing the counter. The
initial version should work identically to what we have now. Then, all
of the remaining filesystems that use i_version are converted to the new
inlines.

Once that's in place, we switch to a new implementation that allows us
to track readers of i_version counter, and only bump it when it's
necessary or convenient (i.e. we're going to disk anyway).

The final patch switches from a scheme that uses the i_lock to serialize
the counter updates during write to an atomic64_t. That's a wash
performance-wise in my testing, but I like not having to take the i_lock
down where it could end up nested inside other locks.

With this, we reduce inode metadata updates across all 3 filesystems
down to roughly the frequency of the timestamp granularity, particularly
when it's not being queried (the vastly common case).

The pessimal workload here is 1 byte writes, and it helps that
significantly. Of course, that's not a real-world workload.

A tiobench-example.fio workload also shows some modest performance
gains, and I've gotten mails from the kernel test robot that show some
significant performance gains on some microbenchmarks (case-msync-mt in
the vm-scalability testsuite to be specific).

I'm happy to run other workloads if anyone can suggest them.

At this point, the patchset works and does what it's expected to do in
my own testing. It seems like it's at least a modest performance win
across all 3 major disk-based filesystems. It may also encourage others
to implement i_version as well since it reduces that cost.

Is this an avenue that's worthwhile to pursue?

Note that I think we may have other changes coming in the future that
will make this sort of cleanup necessary anyway. I'd like to plug in the
Ceph change attribute here eventually, and that will require something
like this anyway.

Thoughts, comments and suggestions are welcome...

---

[1]: On ext4 it must be turned on with the i_version mount option,
     mostly due to fears of incurring this impact, AFAICT.

[2]: NFS also recommends that it appear to increase in value over time, so
     that clients can discard metadata updates that are older than ones
     they've already seen.

Jeff Layton (30):
  lustre: don't set f_version in ll_readdir
  ecryptfs: remove unnecessary i_version bump
  ceph: remove the bump of i_version
  f2fs: don't bother setting i_version
  hpfs: don't bother with the i_version counter
  jfs: remove initialization of i_version counter
  nilfs2: remove inode->i_version initialization
  orangefs: remove initialization of i_version
  reiserfs: remove unneeded i_version bump
  ntfs: remove i_version handling
  fs: new API for handling i_version
  fat: convert to new i_version API
  affs: convert to new i_version API
  afs: convert to new i_version API
  btrfs: convert to new i_version API
  exofs: switch to new i_version API
  ext2: convert to new i_version API
  ext4: convert to new i_version API
  nfs: convert to new i_version API
  nfsd: convert to new i_version API
  ocfs2: convert to new i_version API
  ufs: use new i_version API
  xfs: convert to new i_version API
  IMA: switch IMA over to new i_version API
  fs: add a "force" parameter to inode_inc_iversion
  fs: only set S_VERSION when updating times if it has been queried
  xfs: avoid setting XFS_ILOG_CORE if i_version doesn't need
    incrementing
  btrfs: only dirty the inode in btrfs_update_time if something was
    changed
  fs: track whether the i_version has been queried with an i_state flag
  fs: convert i_version counter over to an atomic64_t

 drivers/staging/lustre/lustre/llite/dir.c |   3 -
 fs/affs/amigaffs.c                        |   4 +-
 fs/affs/dir.c                             |   4 +-
 fs/affs/super.c                           |   2 +-
 fs/afs/fsclient.c                         |   2 +-
 fs/afs/inode.c                            |   4 +-
 fs/btrfs/delayed-inode.c                  |   4 +-
 fs/btrfs/file.c                           |   4 +-
 fs/btrfs/inode.c                          |  41 ++++----
 fs/btrfs/ioctl.c                          |   4 +-
 fs/btrfs/tree-log.c                       |   2 +-
 fs/btrfs/xattr.c                          |   2 +-
 fs/ceph/inode.c                           |   1 -
 fs/ecryptfs/inode.c                       |   1 -
 fs/exofs/dir.c                            |   8 +-
 fs/exofs/super.c                          |   2 +-
 fs/ext2/dir.c                             |   8 +-
 fs/ext2/super.c                           |   4 +-
 fs/ext4/dir.c                             |   8 +-
 fs/ext4/inline.c                          |   6 +-
 fs/ext4/inode.c                           |  16 ++--
 fs/ext4/ioctl.c                           |   2 +-
 fs/ext4/namei.c                           |   8 +-
 fs/ext4/super.c                           |   2 +-
 fs/f2fs/super.c                           |   1 -
 fs/fat/dir.c                              |   2 +-
 fs/fat/inode.c                            |   8 +-
 fs/fat/namei_msdos.c                      |   6 +-
 fs/fat/namei_vfat.c                       |  20 ++--
 fs/hpfs/dir.c                             |   1 -
 fs/hpfs/dnode.c                           |   2 -
 fs/hpfs/super.c                           |   1 -
 fs/inode.c                                |   9 +-
 fs/jfs/super.c                            |   1 -
 fs/nfs/delegation.c                       |   2 +-
 fs/nfs/fscache-index.c                    |   4 +-
 fs/nfs/inode.c                            |  16 ++--
 fs/nfs/nfs4proc.c                         |   4 +-
 fs/nfs/nfstrace.h                         |   4 +-
 fs/nfs/write.c                            |   2 +-
 fs/nfsd/nfs3xdr.c                         |   2 +-
 fs/nfsd/nfs4xdr.c                         |   2 +-
 fs/nfsd/nfsfh.h                           |   2 +-
 fs/nilfs2/super.c                         |   1 -
 fs/ntfs/inode.c                           |   9 --
 fs/ntfs/mft.c                             |   6 --
 fs/ocfs2/dir.c                            |  14 +--
 fs/ocfs2/inode.c                          |   2 +-
 fs/ocfs2/namei.c                          |   2 +-
 fs/ocfs2/quota_global.c                   |   2 +-
 fs/orangefs/super.c                       |   2 -
 fs/reiserfs/super.c                       |   1 -
 fs/ufs/dir.c                              |   8 +-
 fs/ufs/inode.c                            |   2 +-
 fs/ufs/super.c                            |   2 +-
 fs/xfs/libxfs/xfs_inode_buf.c             |   4 +-
 fs/xfs/xfs_icache.c                       |   4 +-
 fs/xfs/xfs_inode.c                        |   2 +-
 fs/xfs/xfs_inode_item.c                   |   2 +-
 fs/xfs/xfs_trans_inode.c                  |  12 +--
 include/linux/fs.h                        | 151 ++++++++++++++++++++++++++++--
 security/integrity/ima/ima_api.c          |   2 +-
 security/integrity/ima/ima_main.c         |   2 +-
 63 files changed, 288 insertions(+), 173 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1545899 — [RFC PATCH v1 02/30] ecryptfs: remove unnecessary i_version bump

ecryptfs bumps the i_version counter when a new inode is
instantiated, but never touches it again.

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

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index e7413f82d27b..9493e8614de1 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -64,7 +64,6 @@ static int ecryptfs_inode_set(struct inode *inode, void *opaque)
 	/* i_size will be overwritten for encrypted regular files */
 	fsstack_copy_inode_size(inode, lower_inode);
 	inode->i_ino = lower_inode->i_ino;
-	inode->i_version++;
 	inode->i_mapping->a_ops = &ecryptfs_aops;
 
 	if (S_ISLNK(inode->i_mode))
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1545901 — [RFC PATCH v1 06/30] jfs: remove initialization of i_version counter

It's bumped on quota writes but is otherwise untouched.

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

diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 85671f7f8518..2739600f826e 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -829,7 +829,6 @@ static ssize_t jfs_quota_write(struct super_block *sb, int type,
 	}
 	if (inode->i_size < off+len-towrite)
 		i_size_write(inode, off+len-towrite);
-	inode->i_version++;
 	inode->i_mtime = inode->i_ctime = current_time(inode);
 	mark_inode_dirty(inode);
 	inode_unlock(inode);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1545902 — [RFC PATCH v1 12/30] fat: convert to new i_version API

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/fat/dir.c         |  2 +-
 fs/fat/inode.c       |  8 ++++----
 fs/fat/namei_msdos.c |  6 +++---
 fs/fat/namei_vfat.c  | 20 ++++++++++----------
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 81cecbe6d7cf..76a4cace7543 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1056,7 +1056,7 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
 	brelse(bh);
 	if (err)
 		return err;
-	dir->i_version++;
+	inode_inc_iversion_locked(dir);
 
 	if (nr_slots) {
 		/*
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 338d2f73eb29..9a8a22111031 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -507,7 +507,7 @@ int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de)
 	MSDOS_I(inode)->i_pos = 0;
 	inode->i_uid = sbi->options.fs_uid;
 	inode->i_gid = sbi->options.fs_gid;
-	inode->i_version++;
+	inode_inc_iversion_locked(inode);
 	inode->i_generation = get_seconds();
 
 	if ((de->attr & ATTR_DIR) && !IS_FREE(de->name)) {
@@ -590,7 +590,7 @@ struct inode *fat_build_inode(struct super_block *sb,
 		goto out;
 	}
 	inode->i_ino = iunique(sb, MSDOS_ROOT_INO);
-	inode->i_version = 1;
+	inode_set_iversion(inode, 1);
 	err = fat_fill_inode(inode, de);
 	if (err) {
 		iput(inode);
@@ -1367,7 +1367,7 @@ static int fat_read_root(struct inode *inode)
 	MSDOS_I(inode)->i_pos = MSDOS_ROOT_INO;
 	inode->i_uid = sbi->options.fs_uid;
 	inode->i_gid = sbi->options.fs_gid;
-	inode->i_version++;
+	inode_inc_iversion_locked(inode);
 	inode->i_generation = 0;
 	inode->i_mode = fat_make_mode(sbi, ATTR_DIR, S_IRWXUGO);
 	inode->i_op = sbi->dir_ops;
@@ -1817,7 +1817,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	if (!root_inode)
 		goto out_fail;
 	root_inode->i_ino = MSDOS_ROOT_INO;
-	root_inode->i_version = 1;
+	inode_set_iversion(root_inode, 1);
 	error = fat_read_root(root_inode);
 	if (error < 0) {
 		iput(root_inode);
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index 7d6a105d601b..459fe1d9b578 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -480,7 +480,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 			} else
 				mark_inode_dirty(old_inode);
 
-			old_dir->i_version++;
+			inode_inc_iversion_locked(old_dir);
 			old_dir->i_ctime = old_dir->i_mtime = current_time(old_dir);
 			if (IS_DIRSYNC(old_dir))
 				(void)fat_sync_inode(old_dir);
@@ -508,7 +508,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 			goto out;
 		new_i_pos = sinfo.i_pos;
 	}
-	new_dir->i_version++;
+	inode_inc_iversion_locked(new_dir);
 
 	fat_detach(old_inode);
 	fat_attach(old_inode, new_i_pos);
@@ -540,7 +540,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 	old_sinfo.bh = NULL;
 	if (err)
 		goto error_dotdot;
-	old_dir->i_version++;
+	inode_inc_iversion_locked(old_dir);
 	old_dir->i_ctime = old_dir->i_mtime = ts;
 	if (IS_DIRSYNC(old_dir))
 		(void)fat_sync_inode(old_dir);
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 6a7152d0c250..d7db82751a98 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -44,7 +44,7 @@ static int vfat_revalidate_shortname(struct dentry *dentry)
 {
 	int ret = 1;
 	spin_lock(&dentry->d_lock);
-	if (vfat_d_version(dentry) != d_inode(dentry->d_parent)->i_version)
+	if (inode_cmp_iversion(d_inode(dentry->d_parent), vfat_d_version(dentry)))
 		ret = 0;
 	spin_unlock(&dentry->d_lock);
 	return ret;
@@ -770,7 +770,7 @@ static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
 	if (!inode)
-		vfat_d_version_set(dentry, dir->i_version);
+		vfat_d_version_set(dentry, inode_get_iversion(dir));
 	return d_splice_alias(inode, dentry);
 error:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
@@ -792,7 +792,7 @@ static int vfat_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	err = vfat_add_entry(dir, &dentry->d_name, 0, 0, &ts, &sinfo);
 	if (err)
 		goto out;
-	dir->i_version++;
+	inode_inc_iversion_locked(dir);
 
 	inode = fat_build_inode(sb, sinfo.de, sinfo.i_pos);
 	brelse(sinfo.bh);
@@ -800,7 +800,7 @@ static int vfat_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 		err = PTR_ERR(inode);
 		goto out;
 	}
-	inode->i_version++;
+	inode_inc_iversion_locked(inode);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = ts;
 	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
 
@@ -834,7 +834,7 @@ static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
 	clear_nlink(inode);
 	inode->i_mtime = inode->i_atime = current_time(inode);
 	fat_detach(inode);
-	vfat_d_version_set(dentry, dir->i_version);
+	vfat_d_version_set(dentry, inode_get_iversion(dir));
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
 
@@ -860,7 +860,7 @@ static int vfat_unlink(struct inode *dir, struct dentry *dentry)
 	clear_nlink(inode);
 	inode->i_mtime = inode->i_atime = current_time(inode);
 	fat_detach(inode);
-	vfat_d_version_set(dentry, dir->i_version);
+	vfat_d_version_set(dentry, inode_get_iversion(dir));
 out:
 	mutex_unlock(&MSDOS_SB(sb)->s_lock);
 
@@ -886,7 +886,7 @@ static int vfat_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	err = vfat_add_entry(dir, &dentry->d_name, 1, cluster, &ts, &sinfo);
 	if (err)
 		goto out_free;
-	dir->i_version++;
+	inode_inc_iversion_locked(dir);
 	inc_nlink(dir);
 
 	inode = fat_build_inode(sb, sinfo.de, sinfo.i_pos);
@@ -896,7 +896,7 @@ static int vfat_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 		/* the directory was completed, just return a error */
 		goto out;
 	}
-	inode->i_version++;
+	inode_inc_iversion_locked(inode);
 	set_nlink(inode, 2);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = ts;
 	/* timestamp is already written, so mark_inode_dirty() is unneeded. */
@@ -962,7 +962,7 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto out;
 		new_i_pos = sinfo.i_pos;
 	}
-	new_dir->i_version++;
+	inode_inc_iversion_locked(new_dir);
 
 	fat_detach(old_inode);
 	fat_attach(old_inode, new_i_pos);
@@ -990,7 +990,7 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 	old_sinfo.bh = NULL;
 	if (err)
 		goto error_dotdot;
-	old_dir->i_version++;
+	inode_inc_iversion_locked(old_dir);
 	old_dir->i_ctime = old_dir->i_mtime = ts;
 	if (IS_DIRSYNC(old_dir))
 		(void)fat_sync_inode(old_dir);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1545903 — [RFC PATCH v1 07/30] nilfs2: remove inode->i_version initialization

It's never used in nilfs2.

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

diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 12eeae62a2b1..d157f6f96d9a 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -160,7 +160,6 @@ struct inode *nilfs_alloc_inode(struct super_block *sb)
 	ii->i_bh = NULL;
 	ii->i_state = 0;
 	ii->i_cno = 0;
-	ii->vfs_inode.i_version = 1;
 	nilfs_mapping_init(&ii->i_btnode_cache, &ii->vfs_inode);
 	return &ii->vfs_inode;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1545904 — [RFC PATCH v1 10/30] ntfs: remove i_version handling

Nothing uses this, and the i_version is never incremented on ntfs.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/ntfs/inode.c | 9 ---------
 fs/ntfs/mft.c   | 6 ------
 2 files changed, 15 deletions(-)

diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 7c410f879412..1c1ee489284b 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -560,13 +560,6 @@ static int ntfs_read_locked_inode(struct inode *vi)
 	ntfs_debug("Entering for i_ino 0x%lx.", vi->i_ino);
 
 	/* Setup the generic vfs inode parts now. */
-
-	/*
-	 * This is for checking whether an inode has changed w.r.t. a file so
-	 * that the file can be updated if necessary (compare with f_version).
-	 */
-	vi->i_version = 1;
-
 	vi->i_uid = vol->uid;
 	vi->i_gid = vol->gid;
 	vi->i_mode = 0;
@@ -1240,7 +1233,6 @@ static int ntfs_read_locked_attr_inode(struct inode *base_vi, struct inode *vi)
 	base_ni = NTFS_I(base_vi);
 
 	/* Just mirror the values from the base inode. */
-	vi->i_version	= base_vi->i_version;
 	vi->i_uid	= base_vi->i_uid;
 	vi->i_gid	= base_vi->i_gid;
 	set_nlink(vi, base_vi->i_nlink);
@@ -1507,7 +1499,6 @@ static int ntfs_read_locked_index_inode(struct inode *base_vi, struct inode *vi)
 	ni	= NTFS_I(vi);
 	base_ni = NTFS_I(base_vi);
 	/* Just mirror the values from the base inode. */
-	vi->i_version	= base_vi->i_version;
 	vi->i_uid	= base_vi->i_uid;
 	vi->i_gid	= base_vi->i_gid;
 	set_nlink(vi, base_vi->i_nlink);
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index b6f402194f02..165133321577 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -2641,12 +2641,6 @@ ntfs_inode *ntfs_mft_record_alloc(ntfs_volume *vol, const int mode,
 			goto undo_mftbmp_alloc;
 		}
 		vi->i_ino = bit;
-		/*
-		 * This is for checking whether an inode has changed w.r.t. a
-		 * file so that the file can be updated if necessary (compare
-		 * with f_version).
-		 */
-		vi->i_version = 1;
 
 		/* The owner and group come from the ntfs volume. */
 		vi->i_uid = vol->uid;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1545905 — [RFC PATCH v1 03/30] ceph: remove the bump of i_version

Eventually, we'll want to wire it up to use the change attribute that
the cluster tracks instead, but for now this is unneeded.

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

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 398e5328b309..c3406acb36e7 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -796,7 +796,6 @@ static int fill_inode(struct inode *inode, struct page *locked_page,
 
 	/* update inode */
 	ci->i_version = le64_to_cpu(info->version);
-	inode->i_version++;
 	inode->i_rdev = le32_to_cpu(info->rdev);
 	inode->i_blkbits = fls(le32_to_cpu(info->layout.fl_stripe_unit)) - 1;
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1545906 — [RFC PATCH v1 15/30] btrfs: convert to new i_version API

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/btrfs/delayed-inode.c | 4 ++--
 fs/btrfs/inode.c         | 4 ++--
 fs/btrfs/tree-log.c      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index 80982a83c9fd..5e1015705c8b 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -1744,7 +1744,7 @@ static void fill_stack_inode_item(struct btrfs_trans_handle *trans,
 	btrfs_set_stack_inode_nbytes(inode_item, inode_get_bytes(inode));
 	btrfs_set_stack_inode_generation(inode_item,
 					 BTRFS_I(inode)->generation);
-	btrfs_set_stack_inode_sequence(inode_item, inode->i_version);
+	btrfs_set_stack_inode_sequence(inode_item, inode_get_iversion(inode));
 	btrfs_set_stack_inode_transid(inode_item, trans->transid);
 	btrfs_set_stack_inode_rdev(inode_item, inode->i_rdev);
 	btrfs_set_stack_inode_flags(inode_item, BTRFS_I(inode)->flags);
@@ -1798,7 +1798,7 @@ int btrfs_fill_inode(struct inode *inode, u32 *rdev)
 	BTRFS_I(inode)->generation = btrfs_stack_inode_generation(inode_item);
         BTRFS_I(inode)->last_trans = btrfs_stack_inode_transid(inode_item);
 
-	inode->i_version = btrfs_stack_inode_sequence(inode_item);
+	inode_set_iversion_read(inode, btrfs_stack_inode_sequence(inode_item));
 	inode->i_rdev = 0;
 	*rdev = btrfs_stack_inode_rdev(inode_item);
 	BTRFS_I(inode)->flags = btrfs_stack_inode_flags(inode_item);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f2b281ad7af6..a56a45a3e992 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3732,7 +3732,7 @@ static int btrfs_read_locked_inode(struct inode *inode)
 	BTRFS_I(inode)->generation = btrfs_inode_generation(leaf, inode_item);
 	BTRFS_I(inode)->last_trans = btrfs_inode_transid(leaf, inode_item);
 
-	inode->i_version = btrfs_inode_sequence(leaf, inode_item);
+	inode_set_iversion_read(inode, btrfs_inode_sequence(leaf, inode_item));
 	inode->i_generation = BTRFS_I(inode)->generation;
 	inode->i_rdev = 0;
 	rdev = btrfs_inode_rdev(leaf, inode_item);
@@ -3903,7 +3903,7 @@ static void fill_inode_item(struct btrfs_trans_handle *trans,
 				     &token);
 	btrfs_set_token_inode_generation(leaf, item, BTRFS_I(inode)->generation,
 					 &token);
-	btrfs_set_token_inode_sequence(leaf, item, inode->i_version, &token);
+	btrfs_set_token_inode_sequence(leaf, item, inode_get_iversion(inode), &token);
 	btrfs_set_token_inode_transid(leaf, item, trans->transid, &token);
 	btrfs_set_token_inode_rdev(leaf, item, inode->i_rdev, &token);
 	btrfs_set_token_inode_flags(leaf, item, BTRFS_I(inode)->flags, &token);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index f10bf5213ed8..4acc4d27383a 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3585,7 +3585,7 @@ static void fill_inode_item(struct btrfs_trans_handle *trans,
 	btrfs_set_token_inode_nbytes(leaf, item, inode_get_bytes(inode),
 				     &token);
 
-	btrfs_set_token_inode_sequence(leaf, item, inode->i_version, &token);
+	btrfs_set_token_inode_sequence(leaf, item, inode_get_iversion(inode), &token);
 	btrfs_set_token_inode_transid(leaf, item, trans->transid, &token);
 	btrfs_set_token_inode_rdev(leaf, item, inode->i_rdev, &token);
 	btrfs_set_token_inode_flags(leaf, item, BTRFS_I(inode)->flags, &token);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1546223

On Wed, Dec 21, 2016 at 12:03:17PM -0500, Jeff Layton wrote:
> Only btrfs, ext4, and xfs implement it for data changes. Because of
> this, these filesystems must log the inode to disk whenever the
> i_version counter changes. That has a non-zero performance impact,
> especially on write-heavy workloads, because we end up dirtying the
> inode metadata on every write, not just when the times change. [1]

Do you have numbers to justify these changes?

[toc] | [prev] | [next] | [standalone]

#1546403 — Re: [RFC PATCH v1 00/30] fs: inode->i_version rework and optimization

On Thu, 2016-12-22 at 00:45 -0800, Christoph Hellwig wrote:
> On Wed, Dec 21, 2016 at 12:03:17PM -0500, Jeff Layton wrote:
> > 
> > Only btrfs, ext4, and xfs implement it for data changes. Because of
> > this, these filesystems must log the inode to disk whenever the
> > i_version counter changes. That has a non-zero performance impact,
> > especially on write-heavy workloads, because we end up dirtying the
> > inode metadata on every write, not just when the times change. [1]
> 
> Do you have numbers to justify these changes?

I have numbers. As to whether they justify the changes, I'm not sure.
This helps a lot on a (admittedly nonsensical) 1-byte write workload. On
XFS, with this fio jobfile:

--------------------8<------------------
[global]
direct=0
size=2g
filesize=512m
bsrange=1-1
timeout=60
numjobs=1
directory=/mnt/scratch

[f1]
filename=randwrite
rw=randwrite
--------------------8<------------------

Unpatched kernel:
  WRITE: io=7707KB, aggrb=128KB/s, minb=128KB/s, maxb=128KB/s, mint=60000msec, maxt=60000msec

Patched kernel:
  WRITE: io=12701KB, aggrb=211KB/s, minb=211KB/s, maxb=211KB/s, mint=60000msec, maxt=60000msec

So quite a difference there and it's pretty consistent across runs. If I
change the jobfile to have "direct=1" and "bsrange=4k-4k", then any
variation between the two doesn't seem to be significant (numbers vary
as much between runs on the same kernels and are roughly the same).

Playing with buffered I/O sizes between 1 byte and 4k shows that as the
I/O sizes get larger, this makes less difference (which is what I'd
expect).

Previous testing with ext4 shows roughly the same results. btrfs shows
some benefit here but significantly less than with ext4 or xfs. Not sure
why that is yet -- maybe CoW effects?

That said, I don't have a great test rig for this. I'm using VMs with a
dedicated LVM volume that's on a random SSD I had laying around. It
could use testing on a wider set of configurations and workloads.

I was also hoping that others may have workloads that they think might
be (postively or negatively) affected by these changes. If you can think
of any in particular, then I'm interested to hear about them.

-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web