fs: rework and optimize i_version handling in filesystems
Subsystems (15)
Jeff Layton
maintainerv5 cover fs: rework and optimize i_version handling in filesystems
From: Jeff Layton <[email protected]>
v5:
- don't corrupt refcounts stashed in i_version of ext4 xattr inodes
- add raw variants of inc and cmp functions, and have nfs use them
v4:
- fix SB_LAZYTIME handling in generic_update_time
- add memory barriers to patch to convert i_version field to atomic64_t
v3:
- move i_version handling functions to new header file
- document that the kernel-managed i_version implementation will appear to increase over time
- fix inode_cmp_iversion to handle wraparound correctly
v2:
- xfs should use inode_peek_iversion instead of inode_peek_iversion_raw
- rework file_update_time patch
- don't dirty inode when only S_ATIME is set and SB_LAZYTIME is enabled
- better comments and documentation
I think this is now approaching merge readiness.
Special thanks to Jan Kara and Dave Chinner who helped me tighten up the memory barriers in the final patch, and Krzysztof Kozlowski for help in tracking down a set of bugs in the NFS client patch.
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().
Note that this set relies on a few patches that are in other trees. The full stack that I've been testing with is here:
https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/log/?h=iversion
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 to optimize away some remeasurement if it's available. NFS and AFS just use it to store an opaque change attribute from the server.
Only btrfs, ext4, and xfs increment 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.
It turns out though that none of these users of i_version require that it 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.
If we keep track of when something queries the value, we can avoid bumping the counter and an on-disk update when nothing else has changed if no one has queried it since it was last incremented.
This patchset changes the code to only bump the i_version counter when it's strictly necessary, or when we're updating the inode metadata anyway (e.g. when times change).
It takes the approach of converting the existing accessors of i_version to use a new API, while leaving the underlying implementation mostly the same. The last patch then converts the existing implementation to keep track of whether the value has been queried since it was last incremented. It then uses that to avoid incrementing the counter when it can.
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).
I can see measurable performance gains on xfs and ext4 with iversion enabled, when streaming small (4k) I/Os.
btrfs shows some slight gain in testing, but not quite the magnitude that xfs and ext4 show. I'm not sure why yet and would appreciate some input from btrfs folks.
My goal is to get this into linux-next fairly soon. If it shows no problems then we can look at merging it for 4.16, or 4.17 if all of the prequisite patches are not yet merged.
Jeff Layton (19):
fs: new API for handling inode->i_version
fs: don't take the i_lock in inode_inc_iversion
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: only set S_VERSION when updating times if necessary
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: handle inode->i_version more efficiently
fs/affs/amigaffs.c | 5 +- fs/affs/dir.c | 5 +- fs/affs/super.c | 3 +- fs/afs/fsclient.c | 3 +- fs/afs/inode.c | 5 +- fs/btrfs/delayed-inode.c | 7 +- fs/btrfs/file.c | 1 + fs/btrfs/inode.c | 12 +- fs/btrfs/ioctl.c | 1 + fs/btrfs/tree-log.c | 4 +- fs/btrfs/xattr.c | 1 + fs/exofs/dir.c | 9 +- fs/exofs/super.c | 3 +- fs/ext2/dir.c | 9 +- fs/ext2/super.c | 5 +- fs/ext4/dir.c | 9 +- fs/ext4/inline.c | 7 +- fs/ext4/inode.c | 13 +- fs/ext4/ioctl.c | 3 +- fs/ext4/namei.c | 5 +- fs/ext4/super.c | 3 +- fs/ext4/xattr.c | 5 +- fs/fat/dir.c | 3 +- fs/fat/inode.c | 9 +- fs/fat/namei_msdos.c | 7 +- fs/fat/namei_vfat.c | 22 +-- fs/inode.c | 11 +- fs/nfs/delegation.c | 3 +- fs/nfs/fscache-index.c | 5 +- fs/nfs/inode.c | 18 +- fs/nfs/nfs4proc.c | 10 +- fs/nfs/nfstrace.h | 5 +- fs/nfs/write.c | 8 +- fs/nfsd/nfsfh.h | 3 +- fs/ocfs2/dir.c | 15 +- fs/ocfs2/inode.c | 3 +- fs/ocfs2/namei.c | 3 +- fs/ocfs2/quota_global.c | 3 +- fs/ufs/dir.c | 9 +- fs/ufs/inode.c | 3 +- fs/ufs/super.c | 3 +- fs/xfs/libxfs/xfs_inode_buf.c | 7 +- fs/xfs/xfs_icache.c | 5 +- fs/xfs/xfs_inode.c | 3 +- fs/xfs/xfs_inode_item.c | 3 +- fs/xfs/xfs_trans_inode.c | 16 +- include/linux/fs.h | 17 +- include/linux/iversion.h | 335 ++++++++++++++++++++++++++++++++++++++ security/integrity/ima/ima_api.c | 3 +- security/integrity/ima/ima_main.c | 3 +- 50 files changed, 518 insertions(+), 135 deletions(-) create mode 100644 include/linux/iversion.h
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 1/19 fs: new API for handling inode->i_version
From: Jeff Layton <[email protected]>
Add a documentation blob that explains what the i_version field is, how it is expected to work, and how it is currently implemented by various filesystems.
We already have inode_inc_iversion. Add several other functions for manipulating and accessing the i_version counter. For now, the implementation is trivial and basically works the way that all of the open-coded i_version accesses work today.
Future patches will convert existing users of i_version to use the new API, and then convert the backend implementation to do things more efficiently.
--- fs/btrfs/file.c | 1 + fs/btrfs/inode.c | 1 + fs/btrfs/ioctl.c | 1 + fs/btrfs/xattr.c | 1 + fs/ext4/inode.c | 1 + fs/ext4/namei.c | 1 + fs/inode.c | 1 + include/linux/fs.h | 15 --- include/linux/iversion.h | 236 +++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 243 insertions(+), 15 deletions(-) create mode 100644 include/linux/iversion.h
fs/btrfs/file.c +1 -0
@@ -31,6 +31,7 @@#include <linux/slab.h> #include <linux/btrfs.h> #include <linux/uio.h> +#include <linux/iversion.h> #include "ctree.h" #include "disk-io.h" #include "transaction.h"
fs/btrfs/inode.c +1 -0
@@ -43,6 +43,7 @@#include <linux/posix_acl_xattr.h> #include <linux/uio.h> #include <linux/magic.h> +#include <linux/iversion.h> #include "ctree.h" #include "disk-io.h" #include "transaction.h"
fs/btrfs/ioctl.c +1 -0
@@ -43,6 +43,7 @@#include <linux/uuid.h> #include <linux/btrfs.h> #include <linux/uaccess.h> +#include <linux/iversion.h> #include "ctree.h" #include "disk-io.h" #include "transaction.h"
fs/btrfs/xattr.c +1 -0
@@ -23,6 +23,7 @@#include <linux/xattr.h> #include <linux/security.h> #include <linux/posix_acl_xattr.h> +#include <linux/iversion.h> #include "ctree.h" #include "btrfs_inode.h" #include "transaction.h"
fs/ext4/inode.c +1 -0
@@ -39,6 +39,7 @@#include <linux/slab.h> #include <linux/bitops.h> #include <linux/iomap.h> +#include <linux/iversion.h> #include "ext4_jbd2.h" #include "xattr.h"
fs/ext4/namei.c +1 -0
@@ -34,6 +34,7 @@#include <linux/quotaops.h> #include <linux/buffer_head.h> #include <linux/bio.h> +#include <linux/iversion.h> #include "ext4.h" #include "ext4_jbd2.h"
fs/inode.c +1 -0
@@ -18,6 +18,7 @@#include <linux/buffer_head.h> /* for inode_has_buffers */ #include <linux/ratelimit.h> #include <linux/list_lru.h> +#include <linux/iversion.h> #include <trace/events/writeback.h> #include "internal.h"
include/linux/fs.h +0 -15
@@ -2036,21 +2036,6 @@ static inline void inode_dec_link_count(struct inode *inode)mark_inode_dirty (inode); } -/** -* inode_inc_iversion - increments i_version -* @inode: inode that need to be updated -* -* Every time the inode is modified, the i_version field will be incremented. -* The filesystem has to be mounted with i_version flag -*/ - -static inline void inode_inc_iversion (struct inode *inode) -{ -spin_lock (&inode->i_lock); - inode->i_version++; -spin_unlock (&inode->i_lock); -} -enum file_time_flags {S_ATIME =1 ,S_MTIME =2 ,
include/linux/iversion.h new file +236 -0
236 lines not on this page · raw message
Signature · 1 line
2.14.3
Re: [PATCH v5 01/19] fs: new API for handling inode->i_version
Quoted 6 lines · Jeff Layton
include/linux/iversion.h new file +3
-0@@ -0,0 +1,236 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_IVERSION_H +#define _LINUX_IVERSION_H
include/linux/iversion.h line 13
Quoted 10 lines
include/linux/iversion.h new file +10
-0+ +#include <linux/fs.h> + +/* +* The change attribute (i_version) is mandated by NFSv4 and is mostly for +* knfsd, but is also used for other purposes (e.g. IMA). The i_version must +* appear different to observers if there was a change to the inode's data or +* metadata since it was last queried. +* +* Observers see the i_version as a 64-bit number that never changes.
I don't understand that sentence.
Quoted 5 lines
If it
+ * remains the same since it was last checked, then nothing has changed in the
+ * inode. If it's different then something has changed. Observers cannot infer
+ * anything about the nature or magnitude of the changes from the value, only
+ * that the inode has changed in some fashion.
As we've discussed before, there may be brief windows where the first two statements aren't quite correct. I think that would be worth a mention if we can keep it concise. Maybe add something like this?:
It may be impractical for filesystems to keep i_version updates atomic with respect to the changes that cause them. They should, however, guarantee that i_version updates are never visible before the changes that caused them. Also, i_version updates should never be delayed longer than it takes the original change to reach disk.
Or maybe those details are best left to documentation on the relevant parts of the api below (maybe inode_maybe_inc_iversion?).
I dunno if it's also worth mentioning that nfsd doesn't actually use the raw i_version--it mixes it with ctime to prevent i_version reuse after reboot. Presumably that doesn't matter to IMA since it doesn't compare i_version across reboots.
The documentation here is all very helpful, thanks.
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Jeff Layton
maintainerRe: [PATCH v5 01/19] fs: new API for handling inode->i_version
Quoted 11 lines · J. Bruce Fields
Quoted 10 lines · Jeff Layton
include/linux/iversion.h new file +7
-0@@ -0,0 +1,236 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_IVERSION_H +#define _LINUX_IVERSION_H + +#include <linux/fs.h> + +/*
Quoted 9 lines
include/linux/iversion.h
Quoted 6 lines
include/linux/iversion.h new file +6
-0+* The change attribute (i_version) is mandated by NFSv4 and is mostly for +* knfsd, but is also used for other purposes (e.g. IMA). The i_version must +* appear different to observers if there was a change to the inode's data or +* metadata since it was last queried. +* +* Observers see the i_version as a 64-bit number that never changes. I don't understand that sentence.
That's because it's utter nonsense. I noticed that the other day and fixed it in my tree. It now reads:
* Observers see the i_version as a 64-bit number that never decreases.
Quoted 10 lines
Quoted 5 lines
If it
+ * remains the same since it was last checked, then nothing has changed in the
+ * inode. If it's different then something has changed. Observers cannot infer
+ * anything about the nature or magnitude of the changes from the value, only
+ * that the inode has changed in some fashion.As we've discussed before, there may be brief windows where the first two statements aren't quite correct. I think that would be worth a mention if we can keep it concise. Maybe add something like this?:
Quoted 6 lines
It may be impractical for filesystems to keep
i_versionupdates atomic with respect to the changes that cause them. They should, however, guarantee thati_versionupdates are never visible before the changes that caused them. Also,i_versionupdates should never be delayed longer than it takes the original change to reach disk.
That makes sense. I added it in pretty much verbatim. I think we mostly follow the latter should already.
Quoted 8 lines
Or maybe those details are best left to documentation on the relevant parts of the api below (maybe
inode_maybe_inc_iversion?).I dunno if it's also worth mentioning that nfsd doesn't actually use the raw i_version--it mixes it with ctime to prevent
i_versionreuse after reboot. Presumably that doesn't matter to IMA since it doesn't comparei_versionacross reboots.
I think I won't document that here. nfsd is a consumer of i_version. What it does with it is sort of its own business. Might be good to have a comment blurb in the nfsd code about it though.
Quoted 1 line
The documentation here is all very helpful, thanks.
Thanks for all of the suggestions so far!
Signature · 1 line
Jeff Layton <[email protected]>
Jeff Layton
maintainerPATCH v5 2/19 fs: don't take the i_lock in inode_inc_iversion
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
The rationale for taking the i_lock when incrementing this value is lost in antiquity. The readers of the field don't take it (at least not universally), so my assumption is that it was only done here to serialize incrementors.
If that is indeed the case, then we can drop the i_lock from this codepath and treat it as a atomic64_t for the purposes of incrementing it. This allows us to use inode_inc_iversion without any danger of lock inversion.
Note that the read side is not fetched atomically with this change. The assumption here is that that is not a critical issue since the i_version is not fully synchronized with anything else anyway.
--- include/linux/iversion.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
include/linux/iversion.h +4 -3
@@ -104,12 +104,13 @@ inode_set_iversion_queried(struct inode *inode, u64 new)static inline bool inode_maybe_inc_iversion (struct inode *inode,bool force) { -spin_lock (&inode->i_lock); - inode->i_version++; -spin_unlock (&inode->i_lock); +atomic64_t *ivp = (atomic64_t *)&inode->i_version; + +atomic64_inc (ivp);return true; } +/** * inode_inc_iversion - forcibly increment i_version * @inode: inode that needs to be updated
Signature · 1 line
2.14.3
Re: [PATCH v5 02/19] fs: don't take the i_lock in inode_inc_iversion
Quoted 12 lines · Jeff Layton
From: Jeff Layton <[email protected]>
The rationale for taking the
i_lockwhen incrementing this value is lost in antiquity. The readers of the field don't take it (at least not universally), so my assumption is that it was only done here to serialize incrementors.If that is indeed the case, then we can drop the
i_lockfrom this codepath and treat it as aatomic64_tfor the purposes of incrementing it. This allows us to useinode_inc_iversionwithout any danger of lock inversion.
Quoted 5 lines
Note that the read side is not fetched atomically with this change. The assumption here is that that is not a critical issue since the
i_versionis not fully synchronized with anything else anyway.Signed-off-by Jeff Layton <[email protected]>
This changes the memory barrier behavior but IMO it is good enough for an intermediate version. You can add:
Honza
Quoted 28 lines
--- include/linux/iversion.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)include/linux/iversion.h +4
-3@@ -104,12 +104,13 @@ inode_set_iversion_queried(struct inode *inode, u64 new)static inline bool inode_maybe_inc_iversion (struct inode *inode,bool force) { -spin_lock (&inode->i_lock); - inode->i_version++; -spin_unlock (&inode->i_lock); +atomic64_t *ivp = (atomic64_t *)&inode->i_version; + +atomic64_inc (ivp);return true; } +/** * inode_inc_iversion - forcibly increment i_version * @inode: inode that needs to be updated Signature · 1 line
2.14.3
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Re: [PATCH v5 02/19] fs: don't take the i_lock in inode_inc_iversion
In reply to Jeff LaytonQuoted 7 lines · Jeff Layton
From: Jeff Layton <[email protected]>
The rationale for taking the
i_lockwhen incrementing this value is lost in antiquity. The readers of the field don't take it (at least not universally), so my assumption is that it was only done here to serialize incrementors.
Quoted 8 lines
If that is indeed the case, then we can drop the
i_lockfrom this codepath and treat it as aatomic64_tfor the purposes of incrementing it. This allows us to useinode_inc_iversionwithout any danger of lock inversion.Note that the read side is not fetched atomically with this change. The assumption here is that that is not a critical issue since the
i_versionis not fully synchronized with anything else anyway.
So I guess it's theoretically possible that e.g. if you read while it's incrementing from 2^32-1 to 2^32 you could read 0, 1, or 2^32+1?
If so then you could see an i_version value reused and incorrectly decide that a file hadn't changed.
But it's such a tiny case, and I think you convert this to atomic64_t later anyway, so, whatever.
--b.
Quoted 29 lines
Signed-off-by Jeff Layton <[email protected]>--- include/linux/iversion.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)include/linux/iversion.h +4
-3@@ -104,12 +104,13 @@ inode_set_iversion_queried(struct inode *inode, u64 new)static inline bool inode_maybe_inc_iversion (struct inode *inode,bool force) { -spin_lock (&inode->i_lock); - inode->i_version++; -spin_unlock (&inode->i_lock); +atomic64_t *ivp = (atomic64_t *)&inode->i_version; + +atomic64_inc (ivp);return true; } +/** * inode_inc_iversion - forcibly increment i_version * @inode: inode that needs to be updated Signature · 1 line
2.14.3
Jeff Layton
maintainerRe: [PATCH v5 02/19] fs: don't take the i_lock in inode_inc_iversion
Quoted 20 lines · J. Bruce Fields
Quoted 7 lines · Jeff Layton
From: Jeff Layton <[email protected]>
The rationale for taking the
i_lockwhen incrementing this value is lost in antiquity. The readers of the field don't take it (at least not universally), so my assumption is that it was only done here to serialize incrementors.Quoted 8 lines
If that is indeed the case, then we can drop the
i_lockfrom this codepath and treat it as aatomic64_tfor the purposes of incrementing it. This allows us to useinode_inc_iversionwithout any danger of lock inversion.Note that the read side is not fetched atomically with this change. The assumption here is that that is not a critical issue since the
i_versionis not fully synchronized with anything else anyway.So I guess it's theoretically possible that e.g. if you read while it's incrementing from 2^32-1 to 2^32 you could read 0, 1, or 2^32+1?
Quoted 8 lines
If so then you could see an
i_versionvalue reused and incorrectly decide that a file hadn't changed.But it's such a tiny case, and I think you convert this to
atomic64_tlater anyway, so, whatever.--b.
Shrug...we have that problem with the spinlock in place too. The bottom line is that reads of this value are not serialized with the increment at all.
I'm not 100% thrilled with this patch, but I think it's probably better not to add the i_lock all over the place, even as an interim step in cleaning this stuff up.
The good news here (as you mention) is that this nastiness gets cleaned up in the last patch when we convert the thing to an atomic64_t.
Quoted 29 lines
Quoted 29 lines
Signed-off-by Jeff Layton <[email protected]>--- include/linux/iversion.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)include/linux/iversion.h +4
-3@@ -104,12 +104,13 @@ inode_set_iversion_queried(struct inode *inode, u64 new)static inline bool inode_maybe_inc_iversion (struct inode *inode,bool force) { -spin_lock (&inode->i_lock); - inode->i_version++; -spin_unlock (&inode->i_lock); +atomic64_t *ivp = (atomic64_t *)&inode->i_version; + +atomic64_inc (ivp);return true; } +/** * inode_inc_iversion - forcibly increment i_version * @inode: inode that needs to be updated Signature · 1 line
2.14.3
Signature · 1 line
Jeff Layton <[email protected]>
Re: [PATCH v5 02/19] fs: don't take the i_lock in inode_inc_iversion
Quoted 3 lines · Jeff Layton
Shrug...we have that problem with the spinlock in place too. The bottom line is that reads of this value are not serialized with the increment at all.
OK, so this wouldn't even be a new bug.
Quoted 3 lines
I'm not 100% thrilled with this patch, but I think it's probably better not to add the
i_lockall over the place, even as an interim step in cleaning this stuff up.
Makes sense to me.
I've got no comments on the rest of the series, except that I'm all for it.
Thanks for persisting--it turned out to be more involved than I'd imagined!
--b.
Jeff Layton
maintainerPATCH v5 3/19 fat: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/fat/dir.c | 3 ++- fs/fat/inode.c | 9 +++++---- fs/fat/namei_msdos.c | 7 ++++--- fs/fat/namei_vfat.c | 22 +++++++++++----------- 4 files changed, 22 insertions(+), 19 deletions(-)
fs/fat/dir.c +2 -1
@@ -16,6 +16,7 @@#include <linux/slab.h> #include <linux/compat.h> #include <linux/uaccess.h> +#include <linux/iversion.h> #include "fat.h" /*
@@ -1055,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 (dir);if (nr_slots) {/*
fs/fat/inode.c +5 -4
@@ -20,6 +20,7 @@#include <linux/blkdev.h> #include <linux/backing-dev.h> #include <asm/unaligned.h> +#include <linux/iversion.h> #include "fat.h" #ifndef CONFIG_FAT_DEFAULT_IOCHARSET
@@ -507,7 +508,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 (inode); inode->i_generation =get_seconds ();if ((de->attr &ATTR_DIR ) && !IS_FREE (de->name)) {
@@ -590,7 +591,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);
@@ -1377,7 +1378,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 (inode); inode->i_generation =0 ; inode->i_mode =fat_make_mode (sbi,ATTR_DIR ,S_IRWXUGO ); inode->i_op = sbi->dir_ops;
@@ -1828,7 +1829,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);
fs/fat/namei_msdos.c +4 -3
@@ -7,6 +7,7 @@*/ #include <linux/module.h> +#include <linux/iversion.h> #include "fat.h" /* Characters that are undesirable in an MS-DOS file name */
@@ -480,7 +481,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 (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 +509,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 (new_dir);fat_detach (old_inode);fat_attach (old_inode, new_i_pos);
@@ -540,7 +541,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 (old_dir); old_dir->i_ctime = old_dir->i_mtime = ts;if (IS_DIRSYNC (old_dir)) (void )fat_sync_inode (old_dir);
fs/fat/namei_vfat.c +11 -11
@@ -20,7 +20,7 @@#include <linux/slab.h> #include <linux/namei.h> #include <linux/kernel.h> - +#include <linux/iversion.h> #include "fat.h" static inline unsigned long vfat_d_version (struct dentry *dentry)
@@ -46,7 +46,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;
@@ -759,7 +759,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_query_iversion (dir));return d_splice_alias (inode, dentry); error:mutex_unlock (&MSDOS_SB (sb)->s_lock);
@@ -781,7 +781,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 (dir); inode =fat_build_inode (sb, sinfo.de, sinfo.i_pos);brelse (sinfo.bh);
@@ -789,7 +789,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 (inode); inode->i_mtime = inode->i_atime = inode->i_ctime = ts;/* timestamp is already written, so mark_inode_dirty() is unneeded. */
@@ -823,7 +823,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_query_iversion (dir)); out:mutex_unlock (&MSDOS_SB (sb)->s_lock);
@@ -849,7 +849,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_query_iversion (dir)); out:mutex_unlock (&MSDOS_SB (sb)->s_lock);
@@ -875,7 +875,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 (dir);inc_nlink (dir); inode =fat_build_inode (sb, sinfo.de, sinfo.i_pos);
@@ -885,7 +885,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 (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. */
@@ -951,7 +951,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 (new_dir);fat_detach (old_inode);fat_attach (old_inode, new_i_pos);
@@ -979,7 +979,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 (old_dir); old_dir->i_ctime = old_dir->i_mtime = ts;if (IS_DIRSYNC (old_dir)) (void )fat_sync_inode (old_dir);
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 4/19 affs: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/affs/amigaffs.c | 5 +++-- fs/affs/dir.c | 5 +++-- fs/affs/super.c | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-)
fs/affs/amigaffs.c +3 -2
@@ -10,6 +10,7 @@*/ #include <linux/math64.h> +#include <linux/iversion.h> #include "affs.h" /*
@@ -60,7 +61,7 @@ affs_insert_hash(struct inode *dir, struct buffer_head *bh)affs_brelse (dir_bh); dir->i_mtime = dir->i_ctime =current_time (dir); - dir->i_version++; +inode_inc_iversion (dir);mark_inode_dirty (dir);return 0 ;
@@ -114,7 +115,7 @@ affs_remove_hash(struct inode *dir, struct buffer_head *rem_bh)affs_brelse (bh); dir->i_mtime = dir->i_ctime =current_time (dir); - dir->i_version++; +inode_inc_iversion (dir);mark_inode_dirty (dir);return retval;
fs/affs/dir.c +3 -2
@@ -14,6 +14,7 @@* */ +#include <linux/iversion.h> #include "affs.h" static int affs_readdir (struct file *,struct dir_context *);
@@ -80,7 +81,7 @@ affs_readdir(struct file *file, struct dir_context *ctx)* we can jump directly to where we left off. */ ino = (u32 )(long )file->private_data; -if (ino && file->f_version == inode->i_version) { +if (ino &&inode_cmp_iversion (inode, file->f_version) ==0 ) {pr_debug ("readdir() left off=%d\n" , ino); goto inside; }
@@ -130,7 +131,7 @@ affs_readdir(struct file *file, struct dir_context *ctx) }while (ino); } done: - file->f_version = inode->i_version; + file->f_version =inode_query_iversion (inode); file->private_data = (void *)(long )ino;affs_brelse (fh_bh);
fs/affs/super.c +2 -1
@@ -21,6 +21,7 @@#include <linux/writeback.h> #include <linux/blkdev.h> #include <linux/seq_file.h> +#include <linux/iversion.h> #include "affs.h" static int affs_statfs (struct dentry *dentry,struct kstatfs *buf);
@@ -102,7 +103,7 @@ static struct inode *affs_alloc_inode(struct super_block *sb)if (!i)return NULL ; - i->vfs_inode.i_version =1 ; +inode_set_iversion (&i->vfs_inode,1 ); i->i_lc =NULL ; i->i_ext_bh =NULL ; i->i_pa_cnt =0 ;
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 5/19 afs: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
For AFS, it's generally treated as an opaque value, so we use the *_raw variants of the API here.
Note that AFS has quite a different definition for this counter. AFS only increments it on changes to the data to the data in regular files and contents of the directories. Inode metadata changes do not result in a version increment.
We'll need to reconcile that somehow if we ever want to present this to userspace via statx.
--- fs/afs/fsclient.c | 3 ++- fs/afs/inode.c | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-)
fs/afs/fsclient.c +2 -1
@@ -13,6 +13,7 @@#include <linux/slab.h> #include <linux/sched.h> #include <linux/circ_buf.h> +#include <linux/iversion.h> #include "internal.h" #include "afs_fs.h"
@@ -124,7 +125,7 @@ static void xdr_decode_AFSFetchStatus(const __be32 **_bp, vnode->vfs_inode.i_ctime.tv_sec = status->mtime_client; vnode->vfs_inode.i_mtime = vnode->vfs_inode.i_ctime; vnode->vfs_inode.i_atime = vnode->vfs_inode.i_ctime; - vnode->vfs_inode.i_version = data_version; +inode_set_iversion_raw (&vnode->vfs_inode, data_version); } expected_version = status->data_version;
fs/afs/inode.c +3 -2
@@ -21,6 +21,7 @@#include <linux/sched.h> #include <linux/mount.h> #include <linux/namei.h> +#include <linux/iversion.h> #include "internal.h" static const struct inode_operations afs_symlink_inode_operations = {
@@ -89,7 +90,7 @@ static int afs_inode_map_status(struct afs_vnode *vnode, struct key *key) inode->i_atime = inode->i_mtime = inode->i_ctime; inode->i_blocks =0 ; inode->i_generation = vnode->fid.unique; - inode->i_version = vnode->status.data_version; +inode_set_iversion_raw (inode, vnode->status.data_version); inode->i_mapping->a_ops = &afs_fs_aops;read_sequnlock_excl (&vnode->cb_lock);
@@ -218,7 +219,7 @@ struct inode *afs_iget_autocell(struct inode *dir, const char *dev_name, inode->i_ctime.tv_nsec =0 ; inode->i_atime = inode->i_mtime = inode->i_ctime; inode->i_blocks =0 ; - inode->i_version =0 ; +inode_set_iversion_raw (inode,0 ); inode->i_generation =0 ;set_bit (AFS_VNODE_PSEUDODIR , &vnode->flags);
Signature · 6 lines
2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Jeff Layton
maintainerPATCH v5 6/19 btrfs: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/btrfs/delayed-inode.c | 7 +++++-- fs/btrfs/inode.c | 6 ++++-- fs/btrfs/tree-log.c | 4 +++- 3 files changed, 12 insertions(+), 5 deletions(-)
fs/btrfs/delayed-inode.c +5 -2
@@ -18,6 +18,7 @@*/ #include <linux/slab.h> +#include <linux/iversion.h> #include "delayed-inode.h" #include "disk-io.h" #include "transaction.h"
@@ -1700,7 +1701,8 @@ 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_peek_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);
@@ -1754,7 +1756,8 @@ 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_queried (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);
fs/btrfs/inode.c +4 -2
@@ -3778,7 +3778,8 @@ 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_queried (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);
@@ -3946,7 +3947,8 @@ 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_peek_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);
fs/btrfs/tree-log.c +3 -1
@@ -20,6 +20,7 @@#include <linux/slab.h> #include <linux/blkdev.h> #include <linux/list_sort.h> +#include <linux/iversion.h> #include "tree-log.h" #include "disk-io.h" #include "locking.h"
@@ -3609,7 +3610,8 @@ 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_peek_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);
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 7/19 exofs: switch to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/exofs/dir.c | 9 +++++---- fs/exofs/super.c | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-)
fs/exofs/dir.c +5 -4
@@ -31,6 +31,7 @@* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ +#include <linux/iversion.h> #include "exofs.h" static inline unsigned exofs_chunk_size (struct inode *inode)
@@ -60,7 +61,7 @@ static int exofs_commit_chunk(struct page *page, loff_t pos, unsigned len)struct inode *dir = mapping->host;int err =0 ; - dir->i_version++; +inode_inc_iversion (dir);if (!PageUptodate (page))SetPageUptodate (page);
@@ -241,7 +242,7 @@ exofs_readdir(struct file *file, struct dir_context *ctx)unsigned long n = pos >>PAGE_SHIFT ;unsigned long npages =dir_pages (inode);unsigned chunk_mask = ~(exofs_chunk_size (inode)-1 ); -int need_revalidate = (file->f_version != inode->i_version); +bool need_revalidate =inode_cmp_iversion (inode, file->f_version);if (pos > inode->i_size -EXOFS_DIR_REC_LEN (1 ))return 0 ;
@@ -264,8 +265,8 @@ exofs_readdir(struct file *file, struct dir_context *ctx) chunk_mask); ctx->pos = (n<<PAGE_SHIFT ) + offset; } - file->f_version = inode->i_version; - need_revalidate =0 ; + file->f_version =inode_query_iversion (inode); + need_revalidate = false; } de = (struct exofs_dir_entry *)(kaddr + offset); limit = kaddr + exofs_last_byte(inode, n) -
fs/exofs/super.c +2 -1
@@ -38,6 +38,7 @@#include <linux/module.h> #include <linux/exportfs.h> #include <linux/slab.h> +#include <linux/iversion.h> #include "exofs.h"
@@ -159,7 +160,7 @@ static struct inode *exofs_alloc_inode(struct super_block *sb)if (!oi)return NULL ; - oi->vfs_inode.i_version =1 ; +inode_set_iversion (&oi->vfs_inode,1 );return &oi->vfs_inode; }
Signature · 6 lines
2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Jeff Layton
maintainerPATCH v5 8/19 ext2: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/ext2/dir.c | 9 +++++---- fs/ext2/super.c | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-)
fs/ext2/dir.c +5 -4
@@ -26,6 +26,7 @@#include <linux/buffer_head.h> #include <linux/pagemap.h> #include <linux/swap.h> +#include <linux/iversion.h> typedef struct ext2_dir_entry_2 ext2_dirent ;
@@ -92,7 +93,7 @@ static int ext2_commit_chunk(struct page *page, loff_t pos, unsigned len)struct inode *dir = mapping->host;int err =0 ; - dir->i_version++; +inode_inc_iversion (dir);block_write_end (NULL , mapping, pos, len, len, page,NULL );if (pos+len > dir->i_size) {
@@ -293,7 +294,7 @@ ext2_readdir(struct file *file, struct dir_context *ctx)unsigned long npages =dir_pages (inode);unsigned chunk_mask = ~(ext2_chunk_size (inode)-1 );unsigned char *types =NULL ; -int need_revalidate = file->f_version != inode->i_version; +bool need_revalidate =inode_cmp_iversion (inode, file->f_version);if (pos > inode->i_size -EXT2_DIR_REC_LEN (1 ))return 0 ;
@@ -319,8 +320,8 @@ ext2_readdir(struct file *file, struct dir_context *ctx) offset =ext2_validate_entry (kaddr, offset, chunk_mask); ctx->pos = (n<<PAGE_SHIFT ) + offset; } - file->f_version = inode->i_version; - need_revalidate =0 ; + file->f_version =inode_query_iversion (inode); + need_revalidate = false; } de = (ext2_dirent *)(kaddr+offset); limit = kaddr +ext2_last_byte (inode, n) -EXT2_DIR_REC_LEN (1 );
fs/ext2/super.c +3 -2
@@ -33,6 +33,7 @@#include <linux/quotaops.h> #include <linux/uaccess.h> #include <linux/dax.h> +#include <linux/iversion.h> #include "ext2.h" #include "xattr.h" #include "acl.h"
@@ -184,7 +185,7 @@ static struct inode *ext2_alloc_inode(struct super_block *sb)if (!ei)return NULL ; ei->i_block_alloc_info =NULL ; - ei->vfs_inode.i_version =1 ; +inode_set_iversion (&ei->vfs_inode,1 );#ifdef CONFIG_QUOTA memset (&ei->i_dquot,0 ,sizeof (ei->i_dquot));#endif
@@ -1569,7 +1570,7 @@ static ssize_t ext2_quota_write(struct super_block *sb, int type,return err;if (inode->i_size < off+len-towrite)i_size_write (inode, off+len-towrite); - inode->i_version++; +inode_inc_iversion (inode); inode->i_mtime = inode->i_ctime =current_time (inode);mark_inode_dirty (inode);return len - towrite;
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 9/19 ext4: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/ext4/dir.c | 9 +++++---- fs/ext4/inline.c | 7 ++++--- fs/ext4/inode.c | 12 ++++++++---- fs/ext4/ioctl.c | 3 ++- fs/ext4/namei.c | 4 ++-- fs/ext4/super.c | 3 ++- fs/ext4/xattr.c | 5 +++-- 7 files changed, 26 insertions(+), 17 deletions(-)
fs/ext4/dir.c +5 -4
@@ -25,6 +25,7 @@#include <linux/fs.h> #include <linux/buffer_head.h> #include <linux/slab.h> +#include <linux/iversion.h> #include "ext4.h" #include "xattr.h"
@@ -208,7 +209,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)* readdir(2), then we might be pointing to an invalid * dirent right now. Scan from the start of the block * to make sure. */ -if (file->f_version != inode->i_version) { +if (inode_cmp_iversion (inode, file->f_version)) {for (i =0 ; i < sb->s_blocksize && i < offset; ) { de = (struct ext4_dir_entry_2 *) (bh->b_data + i);
@@ -227,7 +228,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx) offset = i; ctx->pos = (ctx->pos & ~(sb->s_blocksize -1 )) | offset; - file->f_version = inode->i_version; + file->f_version =inode_query_iversion (inode); } while (ctx->pos < inode->i_size
@@ -568,10 +569,10 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)* cached entries. */ if ((!info->curr_node) || - (file->f_version != inode->i_version)) { +inode_cmp_iversion (inode, file->f_version)) { info->curr_node =NULL ;free_rb_tree_fname (&info->root); - file->f_version = inode->i_version; + file->f_version =inode_query_iversion (inode); ret =ext4_htree_fill_tree (file, info->curr_hash, info->curr_minor_hash, &info->next_hash);
fs/ext4/inline.c +4 -3
@@ -14,6 +14,7 @@#include <linux/iomap.h> #include <linux/fiemap.h> +#include <linux/iversion.h> #include "ext4_jbd2.h" #include "ext4.h"
@@ -1042,7 +1043,7 @@ static int ext4_add_dirent_to_inline(handle_t *handle,*/ dir->i_mtime = dir->i_ctime =current_time (dir);ext4_update_dx_flag (dir); - dir->i_version++; +inode_inc_iversion (dir);return 1 ; }
@@ -1494,7 +1495,7 @@ int ext4_read_inline_dir(struct file *file,* dirent right now. Scan from the start of the inline * dir to make sure. */ -if (file->f_version != inode->i_version) { +if (inode_cmp_iversion (inode, file->f_version)) {for (i =0 ; i < extra_size && i < offset;) {/* * "." is with offset 0 and
@@ -1526,7 +1527,7 @@ int ext4_read_inline_dir(struct file *file, } offset = i; ctx->pos = offset; - file->f_version = inode->i_version; + file->f_version =inode_query_iversion (inode); }while (ctx->pos < extra_size) {
fs/ext4/inode.c +8 -4
@@ -4874,12 +4874,14 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)EXT4_EINODE_GET_XTIME (i_crtime, ei, raw_inode);if (likely (!test_opt2 (inode->i_sb,HURD_COMPAT ))) { - inode->i_version =le32_to_cpu (raw_inode->i_disk_version); +u64 ivers =le32_to_cpu (raw_inode->i_disk_version); +if (EXT4_INODE_SIZE (inode->i_sb) >EXT4_GOOD_OLD_INODE_SIZE ) {if (EXT4_FITS_IN_INODE (raw_inode, ei, i_version_hi)) - inode->i_version |= + ivers |= (__u64)(le32_to_cpu (raw_inode->i_version_hi)) <<32 ; } +inode_set_iversion_queried (inode, ivers); } ret =0 ;
@@ -5165,11 +5167,13 @@ static int ext4_do_update_inode(handle_t *handle, }if (likely (!test_opt2 (inode->i_sb,HURD_COMPAT ))) { - raw_inode->i_disk_version =cpu_to_le32 (inode->i_version); +u64 ivers =inode_peek_iversion (inode); + + raw_inode->i_disk_version =cpu_to_le32 (ivers);if (ei->i_extra_isize) {if (EXT4_FITS_IN_INODE (raw_inode, ei, i_version_hi)) raw_inode->i_version_hi = -cpu_to_le32 (inode->i_version >>32 ); +cpu_to_le32 (ivers >>32 ); raw_inode->i_extra_isize =cpu_to_le16 (ei->i_extra_isize); }
fs/ext4/ioctl.c +2 -1
@@ -19,6 +19,7 @@#include <linux/uuid.h> #include <linux/uaccess.h> #include <linux/delay.h> +#include <linux/iversion.h> #include "ext4_jbd2.h" #include "ext4.h" #include <linux/fsmap.h>
@@ -144,7 +145,7 @@ static long swap_inode_boot_loader(struct super_block *sb,i_gid_write (inode_bl,0 ); inode_bl->i_flags =0 ; ei_bl->i_flags =0 ; - inode_bl->i_version =1 ; +inode_set_iversion (inode_bl,1 );i_size_write (inode_bl,0 ); inode_bl->i_mode =S_IFREG ;if (ext4_has_feature_extents (sb)) {
fs/ext4/namei.c +2 -2
@@ -2956,7 +2956,7 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)"empty directory '%.*s' has too many links (%u)" , dentry->d_name.len, dentry->d_name.name, inode->i_nlink); - inode->i_version++; +inode_inc_iversion (inode);clear_nlink (inode);/* There's no need to set i_disksize: the fact that i_nlink is * zero will ensure that the right thing happens during any
@@ -3362,7 +3362,7 @@ static int ext4_setent(handle_t *handle, struct ext4_renament *ent, ent->de->inode =cpu_to_le32 (ino);if (ext4_has_feature_filetype (ent->dir->i_sb)) ent->de->file_type = file_type; - ent->dir->i_version++; +inode_inc_iversion (ent->dir); ent->dir->i_ctime = ent->dir->i_mtime =current_time (ent->dir);ext4_mark_inode_dirty (handle, ent->dir);
fs/ext4/super.c +2 -1
@@ -40,6 +40,7 @@#include <linux/dax.h> #include <linux/cleancache.h> #include <linux/uaccess.h> +#include <linux/iversion.h> #include <linux/kthread.h> #include <linux/freezer.h>
@@ -967,7 +968,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)if (!ei)return NULL ; - ei->vfs_inode.i_version =1 ; +inode_set_iversion (&ei->vfs_inode,1 );spin_lock_init (&ei->i_raw_lock);INIT_LIST_HEAD (&ei->i_prealloc_list);spin_lock_init (&ei->i_prealloc_lock);
fs/ext4/xattr.c +3 -2
@@ -56,6 +56,7 @@#include <linux/slab.h> #include <linux/mbcache.h> #include <linux/quotaops.h> +#include <linux/iversion.h> #include "ext4_jbd2.h" #include "ext4.h" #include "xattr.h"
@@ -294,13 +295,13 @@ ext4_xattr_inode_hash(struct ext4_sb_info *sbi, const void *buffer, size_t size)static u64 ext4_xattr_inode_get_ref (struct inode *ea_inode) {return ((u64 )ea_inode->i_ctime.tv_sec <<32 ) | - ((u32 )ea_inode->i_version); + (u32 )inode_peek_iversion_raw (ea_inode); }static void ext4_xattr_inode_set_ref (struct inode *ea_inode,u64 ref_count) { ea_inode->i_ctime.tv_sec = (u32)(ref_count >>32 ); - ea_inode->i_version = (u32 )ref_count; +inode_set_iversion_raw (ea_inode, ref_count &0xffffffff ); }static u32 ext4_xattr_inode_get_hash (struct inode *ea_inode)
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 10/19 nfs: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
For NFS, we just use the "raw" API since the i_version is mostly managed by the server. The exception there is when the client holds a write delegation, but we only need to bump it once there anyway to handle CB_GETATTR.
--- fs/nfs/delegation.c | 3 ++- fs/nfs/fscache-index.c | 5 +++-- fs/nfs/inode.c | 18 +++++++++--------- fs/nfs/nfs4proc.c | 10 ++++++---- fs/nfs/nfstrace.h | 5 +++-- fs/nfs/write.c | 8 +++----- 6 files changed, 26 insertions(+), 23 deletions(-)
fs/nfs/delegation.c +2 -1
@@ -12,6 +12,7 @@#include <linux/sched.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/iversion.h> #include <linux/nfs4.h> #include <linux/nfs_fs.h>
@@ -347,7 +348,7 @@ int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, structnfs4_stateid_copy (&delegation->stateid, &res->delegation); delegation->type = res->delegation_type; delegation->pagemod_limit = res->pagemod_limit; - delegation->change_attr = inode->i_version; + delegation->change_attr =inode_peek_iversion_raw (inode); delegation->cred =get_rpccred (cred); delegation->inode = inode; delegation->flags =1 <<NFS_DELEGATION_REFERENCED ;
fs/nfs/fscache-index.c +3 -2
@@ -16,6 +16,7 @@#include <linux/nfs_fs.h> #include <linux/nfs_fs_sb.h> #include <linux/in6.h> +#include <linux/iversion.h> #include "internal.h" #include "fscache.h"
@@ -211,7 +212,7 @@ static uint16_t nfs_fscache_inode_get_aux(const void *cookie_netfs_data, auxdata.ctime = nfsi->vfs_inode.i_ctime;if (NFS_SERVER (&nfsi->vfs_inode)->nfs_client->rpc_ops->version ==4 ) - auxdata.change_attr = nfsi->vfs_inode.i_version; + auxdata.change_attr =inode_peek_iversion_raw (&nfsi->vfs_inode);if (bufmax >sizeof (auxdata)) bufmax =sizeof (auxdata);
@@ -243,7 +244,7 @@ enum fscache_checkaux nfs_fscache_inode_check_aux(void *cookie_netfs_data, auxdata.ctime = nfsi->vfs_inode.i_ctime;if (NFS_SERVER (&nfsi->vfs_inode)->nfs_client->rpc_ops->version ==4 ) - auxdata.change_attr = nfsi->vfs_inode.i_version; + auxdata.change_attr =inode_peek_iversion_raw (&nfsi->vfs_inode);if (memcmp (data, &auxdata, datalen) !=0 )return FSCACHE_CHECKAUX_OBSOLETE ;
fs/nfs/inode.c +9 -9
@@ -38,8 +38,8 @@#include <linux/slab.h> #include <linux/compat.h> #include <linux/freezer.h> -#include <linux/uaccess.h> +#include <linux/iversion.h> #include "nfs4_fs.h" #include "callback.h"
@@ -483,7 +483,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr, stmemset (&inode->i_atime,0 ,sizeof (inode->i_atime));memset (&inode->i_mtime,0 ,sizeof (inode->i_mtime));memset (&inode->i_ctime,0 ,sizeof (inode->i_ctime)); - inode->i_version =0 ; +inode_set_iversion_raw (inode,0 ); inode->i_size =0 ;clear_nlink (inode); inode->i_uid =make_kuid (&init_user_ns,-2 );
@@ -508,7 +508,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr, stelse if (nfs_server_capable (inode,NFS_CAP_CTIME ))nfs_set_cache_invalid (inode,NFS_INO_INVALID_ATTR );if (fattr->valid &NFS_ATTR_FATTR_CHANGE ) - inode->i_version = fattr->change_attr; +inode_set_iversion_raw (inode, fattr->change_attr);else nfs_set_cache_invalid (inode,NFS_INO_INVALID_ATTR |NFS_INO_REVAL_PAGECACHE );
@@ -1289,8 +1289,8 @@ static unsigned long nfs_wcc_update_inode(struct inode *inode, struct nfs_fattrif ((fattr->valid &NFS_ATTR_FATTR_PRECHANGE ) && (fattr->valid &NFS_ATTR_FATTR_CHANGE ) - && inode->i_version == fattr->pre_change_attr) { - inode->i_version = fattr->change_attr; + && !inode_cmp_iversion_raw (inode, fattr->pre_change_attr)) { +inode_set_iversion_raw (inode, fattr->change_attr);if (S_ISDIR (inode->i_mode))nfs_set_cache_invalid (inode,NFS_INO_INVALID_DATA ); ret |=NFS_INO_INVALID_ATTR ;
@@ -1348,7 +1348,7 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fatif (!nfs_file_has_buffered_writers (nfsi)) {/* Verify a few of the more important attributes */ -if ((fattr->valid &NFS_ATTR_FATTR_CHANGE ) !=0 && inode->i_version != fattr->change_attr) +if ((fattr->valid &NFS_ATTR_FATTR_CHANGE ) !=0 &&inode_cmp_iversion_raw (inode, fattr->change_attr)) invalid |=NFS_INO_INVALID_ATTR |NFS_INO_REVAL_PAGECACHE ;if ((fattr->valid &NFS_ATTR_FATTR_MTIME ) && !timespec_equal (&inode->i_mtime, &fattr->mtime))
@@ -1642,7 +1642,7 @@ int nfs_post_op_update_inode_force_wcc_locked(struct inode *inode, struct nfs_fa }if ((fattr->valid &NFS_ATTR_FATTR_CHANGE ) !=0 && (fattr->valid &NFS_ATTR_FATTR_PRECHANGE ) ==0 ) { - fattr->pre_change_attr = inode->i_version; + fattr->pre_change_attr =inode_peek_iversion_raw (inode); fattr->valid |=NFS_ATTR_FATTR_PRECHANGE ; } if ((fattr->valid & NFS_ATTR_FATTR_CTIME) != 0 &&
@@ -1778,7 +1778,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)/* More cache consistency checks */ if (fattr->valid &NFS_ATTR_FATTR_CHANGE ) { -if (inode->i_version != fattr->change_attr) { +if (inode_cmp_iversion_raw (inode, fattr->change_attr)) {dprintk ("NFS: change_attr change on server for file %s/%ld\n" , inode->i_sb->s_id, inode->i_ino);/* Could it be a race with writeback? */
@@ -1790,7 +1790,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)if (S_ISDIR (inode->i_mode))nfs_force_lookup_revalidate (inode); } - inode->i_version = fattr->change_attr; +inode_set_iversion_raw (inode, fattr->change_attr); } }else { nfsi->cache_validity |= save_cache_validity;
fs/nfs/nfs4proc.c +6 -4
@@ -54,6 +54,7 @@#include <linux/xattr.h> #include <linux/utsname.h> #include <linux/freezer.h> +#include <linux/iversion.h> #include "nfs4_fs.h" #include "delegation.h"
@@ -1045,16 +1046,16 @@ static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo,spin_lock (&dir->i_lock); nfsi->cache_validity |=NFS_INO_INVALID_ATTR |NFS_INO_INVALID_DATA ; -if (cinfo->atomic && cinfo->before == dir->i_version) { +if (cinfo->atomic && cinfo->before ==inode_peek_iversion_raw (dir)) { nfsi->cache_validity &= ~NFS_INO_REVAL_PAGECACHE ; nfsi->attrtimeo_timestamp = jiffies; }else {nfs_force_lookup_revalidate (dir); -if (cinfo->before != dir->i_version) +if (cinfo->before !=inode_peek_iversion_raw (dir)) nfsi->cache_validity |=NFS_INO_INVALID_ACCESS |NFS_INO_INVALID_ACL ; } - dir->i_version = cinfo->after; +inode_set_iversion_raw (dir, cinfo->after); nfsi->read_cache_jiffies = timestamp; nfsi->attr_gencount =nfs_inc_attr_generation_counter ();nfs_fscache_invalidate (dir);
@@ -2454,7 +2455,8 @@ static int _nfs4_proc_open(struct nfs4_opendata *data) data->file_created = true; else if (o_res->cinfo.before != o_res->cinfo.after) data->file_created = true; -if (data->file_created || dir->i_version != o_res->cinfo.after) +if (data->file_created || +inode_peek_iversion_raw (dir) != o_res->cinfo.after)update_changeattr (dir, &o_res->cinfo, o_res->f_attr->time_start); }
fs/nfs/nfstrace.h +3 -2
@@ -9,6 +9,7 @@#define _TRACE_NFS_H#include <linux/tracepoint.h> +#include <linux/iversion.h> #define nfs_show_file_type (ftype) \ __print_symbolic(ftype, \
@@ -61,7 +62,7 @@ DECLARE_EVENT_CLASS(nfs_inode_event, __entry->dev = inode->i_sb->s_dev; __entry->fileid = nfsi->fileid; __entry->fhandle =nfs_fhandle_hash (&nfsi->fh); - __entry->version = inode->i_version; + __entry->version =inode_peek_iversion_raw (inode); ), TP_printk(
@@ -100,7 +101,7 @@ DECLARE_EVENT_CLASS(nfs_inode_event_done, __entry->fileid = nfsi->fileid; __entry->fhandle =nfs_fhandle_hash (&nfsi->fh); __entry->type =nfs_umode_to_dtype (inode->i_mode); - __entry->version = inode->i_version; + __entry->version =inode_peek_iversion_raw (inode); __entry->size =i_size_read (inode); __entry->nfsi_flags = nfsi->flags; __entry->cache_validity = nfsi->cache_validity;
fs/nfs/write.c +3 -5
@@ -23,6 +23,7 @@#include <linux/export.h> #include <linux/freezer.h> #include <linux/wait.h> +#include <linux/iversion.h> #include <linux/uaccess.h>
@@ -753,11 +754,8 @@ static void nfs_inode_add_request(struct inode *inode, struct nfs_page *req)*/ spin_lock (&mapping->private_lock);if (!nfs_have_writebacks (inode) && -NFS_PROTO (inode)->have_delegation (inode,FMODE_WRITE )) { -spin_lock (&inode->i_lock); - inode->i_version++; -spin_unlock (&inode->i_lock); - } +NFS_PROTO (inode)->have_delegation (inode,FMODE_WRITE )) +inode_inc_iversion_raw (inode);if (likely (!PageSwapCache (req->wb_page))) {set_bit (PG_MAPPED , &req->wb_flags);SetPagePrivate (req->wb_page);
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 11/19 nfsd: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
Mostly just making sure we use the "get" wrappers so we know when it is being fetched for later use.
--- fs/nfsd/nfsfh.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/nfsd/nfsfh.h +2 -1
@@ -11,6 +11,7 @@#include <linux/crc32.h> #include <linux/sunrpc/svc.h> #include <uapi/linux/nfsd/nfsfh.h> +#include <linux/iversion.h> static inline __u32 ino_t_to_u32 (ino_t ino) {
@@ -259,7 +260,7 @@ static inline u64 nfsd4_change_attribute(struct inode *inode) chattr = inode->i_ctime.tv_sec; chattr <<=30 ; chattr += inode->i_ctime.tv_nsec; - chattr += inode->i_version; + chattr +=inode_query_iversion (inode);return chattr; }
Signature · 6 lines
2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Jeff Layton
maintainerPATCH v5 12/19 ocfs2: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/ocfs2/dir.c | 15 ++++++++------- fs/ocfs2/inode.c | 3 ++- fs/ocfs2/namei.c | 3 ++- fs/ocfs2/quota_global.c | 3 ++- 4 files changed, 14 insertions(+), 10 deletions(-)
fs/ocfs2/dir.c +8 -7
@@ -42,6 +42,7 @@#include <linux/highmem.h> #include <linux/quotaops.h> #include <linux/sort.h> +#include <linux/iversion.h> #include <cluster/masklog.h>
@@ -1174,7 +1175,7 @@ static int __ocfs2_delete_entry(handle_t *handle, struct inode *dir,le16_add_cpu (&pde->rec_len,le16_to_cpu (de->rec_len)); de->inode =0 ; - dir->i_version++; +inode_inc_iversion (dir);ocfs2_journal_dirty (handle, bh); goto bail; }
@@ -1729,7 +1730,7 @@ int __ocfs2_add_entry(handle_t *handle,if (ocfs2_dir_indexed (dir))ocfs2_recalc_free_list (dir, handle, lookup); - dir->i_version++; +inode_inc_iversion (dir);ocfs2_journal_dirty (handle, insert_bh); retval =0 ; goto bail;
@@ -1775,7 +1776,7 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode,* readdir(2), then we might be pointing to an invalid * dirent right now. Scan from the start of the block * to make sure. */ -if (*f_version != inode->i_version) { +if (inode_cmp_iversion (inode, *f_version)) {for (i =0 ; i <i_size_read (inode) && i < offset; ) { de = (struct ocfs2_dir_entry *) (data->id_data + i);
@@ -1791,7 +1792,7 @@ static int ocfs2_dir_foreach_blk_id(struct inode *inode, i +=le16_to_cpu (de->rec_len); } ctx->pos = offset = i; - *f_version = inode->i_version; + *f_version =inode_query_iversion (inode); } de = (struct ocfs2_dir_entry *) (data->id_data + ctx->pos);
@@ -1869,7 +1870,7 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode,* readdir(2), then we might be pointing to an invalid * dirent right now. Scan from the start of the block * to make sure. */ -if (*f_version != inode->i_version) { +if (inode_cmp_iversion (inode, *f_version)) {for (i =0 ; i < sb->s_blocksize && i < offset; ) { de = (struct ocfs2_dir_entry *) (bh->b_data + i);/* It's too expensive to do a full
@@ -1886,7 +1887,7 @@ static int ocfs2_dir_foreach_blk_el(struct inode *inode, offset = i; ctx->pos = (ctx->pos & ~(sb->s_blocksize -1 )) | offset; - *f_version = inode->i_version; + *f_version =inode_query_iversion (inode); } while (ctx->pos < i_size_read(inode)
@@ -1940,7 +1941,7 @@ static int ocfs2_dir_foreach_blk(struct inode *inode, u64 *f_version,*/ int ocfs2_dir_foreach (struct inode *inode,struct dir_context *ctx) { -u64 version = inode->i_version; +u64 version =inode_query_iversion (inode);ocfs2_dir_foreach_blk (inode, &version, ctx, true);return 0 ; }
fs/ocfs2/inode.c +2 -1
@@ -28,6 +28,7 @@#include <linux/highmem.h> #include <linux/pagemap.h> #include <linux/quotaops.h> +#include <linux/iversion.h> #include <asm/byteorder.h>
@@ -302,7 +303,7 @@ void ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe,OCFS2_I (inode)->ip_attr =le32_to_cpu (fe->i_attr);OCFS2_I (inode)->ip_dyn_features =le16_to_cpu (fe->i_dyn_features); - inode->i_version =1 ; +inode_set_iversion (inode,1 ); inode->i_generation =le32_to_cpu (fe->i_generation); inode->i_rdev =huge_decode_dev (le64_to_cpu (fe->id1.dev1.i_rdev)); inode->i_mode =le16_to_cpu (fe->i_mode);
fs/ocfs2/namei.c +2 -1
@@ -41,6 +41,7 @@#include <linux/slab.h> #include <linux/highmem.h> #include <linux/quotaops.h> +#include <linux/iversion.h> #include <cluster/masklog.h>
@@ -1520,7 +1521,7 @@ static int ocfs2_rename(struct inode *old_dir,mlog_errno (status); goto bail; } - new_dir->i_version++; +inode_inc_iversion (new_dir);if (S_ISDIR (new_inode->i_mode))ocfs2_set_links_count (newfe,0 );
fs/ocfs2/quota_global.c +2 -1
@@ -12,6 +12,7 @@#include <linux/writeback.h> #include <linux/workqueue.h> #include <linux/llist.h> +#include <linux/iversion.h> #include <cluster/masklog.h>
@@ -289,7 +290,7 @@ ssize_t ocfs2_quota_write(struct super_block *sb, int type,mlog_errno (err);return err; } - gqinode->i_version++; +inode_inc_iversion (gqinode);ocfs2_mark_inode_dirty (handle, gqinode, oinfo->dqi_gqi_bh);return len; }
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 13/19 ufs: use new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/ufs/dir.c | 9 +++++---- fs/ufs/inode.c | 3 ++- fs/ufs/super.c | 3 ++- 3 files changed, 9 insertions(+), 6 deletions(-)
fs/ufs/dir.c +5 -4
@@ -20,6 +20,7 @@#include <linux/time.h> #include <linux/fs.h> #include <linux/swap.h> +#include <linux/iversion.h> #include "ufs_fs.h" #include "ufs.h"
@@ -47,7 +48,7 @@ static int ufs_commit_chunk(struct page *page, loff_t pos, unsigned len)struct inode *dir = mapping->host;int err =0 ; - dir->i_version++; +inode_inc_iversion (dir);block_write_end (NULL , mapping, pos, len, len, page,NULL );if (pos+len > dir->i_size) {i_size_write (dir, pos+len);
@@ -428,7 +429,7 @@ ufs_readdir(struct file *file, struct dir_context *ctx)unsigned long n = pos >>PAGE_SHIFT ;unsigned long npages =dir_pages (inode);unsigned chunk_mask = ~(UFS_SB (sb)->s_uspi->s_dirblksize -1 ); -int need_revalidate = file->f_version != inode->i_version; +bool need_revalidate =inode_cmp_iversion (inode, file->f_version);unsigned flags =UFS_SB (sb)->s_flags;UFSD ("BEGIN\n" );
@@ -455,8 +456,8 @@ ufs_readdir(struct file *file, struct dir_context *ctx) offset =ufs_validate_entry (sb, kaddr, offset, chunk_mask); ctx->pos = (n<<PAGE_SHIFT ) + offset; } - file->f_version = inode->i_version; - need_revalidate =0 ; + file->f_version =inode_query_iversion (inode); + need_revalidate = false; } de = (struct ufs_dir_entry *)(kaddr+offset); limit = kaddr +ufs_last_byte (inode, n) -UFS_DIR_REC_LEN (1 );
fs/ufs/inode.c +2 -1
@@ -36,6 +36,7 @@#include <linux/mm.h> #include <linux/buffer_head.h> #include <linux/writeback.h> +#include <linux/iversion.h> #include "ufs_fs.h" #include "ufs.h"
@@ -693,7 +694,7 @@ struct inode *ufs_iget(struct super_block *sb, unsigned long ino)if (err) goto bad_inode; - inode->i_version++; +inode_inc_iversion (inode); ufsi->i_lastfrag = (inode->i_size + uspi->s_fsize -1 ) >> uspi->s_fshift; ufsi->i_dir_start_lookup =0 ;
fs/ufs/super.c +2 -1
@@ -88,6 +88,7 @@#include <linux/log2.h> #include <linux/mount.h> #include <linux/seq_file.h> +#include <linux/iversion.h> #include "ufs_fs.h" #include "ufs.h"
@@ -1440,7 +1441,7 @@ static struct inode *ufs_alloc_inode(struct super_block *sb)if (!ei)return NULL ; - ei->vfs_inode.i_version =1 ; +inode_set_iversion (&ei->vfs_inode,1 );seqlock_init (&ei->meta_lock);mutex_init (&ei->truncate_mutex);return &ei->vfs_inode;
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 14/19 xfs: convert to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- fs/xfs/libxfs/xfs_inode_buf.c | 7 +++++-- fs/xfs/xfs_icache.c | 5 +++-- fs/xfs/xfs_inode.c | 3 ++- fs/xfs/xfs_inode_item.c | 3 ++- fs/xfs/xfs_trans_inode.c | 4 +++- 5 files changed, 15 insertions(+), 7 deletions(-)
fs/xfs/libxfs/xfs_inode_buf.c +5 -2
@@ -32,6 +32,8 @@#include "xfs_ialloc.h" #include "xfs_dir2.h" +#include <linux/iversion.h> +/* * Check that none of the inode's in the buffer have a next * unlinked field of 0.
@@ -264,7 +266,8 @@ xfs_inode_from_disk( to->di_flags =be16_to_cpu (from->di_flags);if (to->di_version ==3 ) { - inode->i_version =be64_to_cpu (from->di_changecount); +inode_set_iversion_queried (inode, +be64_to_cpu (from->di_changecount)); to->di_crtime.t_sec =be32_to_cpu (from->di_crtime.t_sec); to->di_crtime.t_nsec =be32_to_cpu (from->di_crtime.t_nsec); to->di_flags2 =be64_to_cpu (from->di_flags2);
@@ -314,7 +317,7 @@ xfs_inode_to_disk( to->di_flags =cpu_to_be16 (from->di_flags);if (from->di_version ==3 ) { - to->di_changecount =cpu_to_be64 (inode->i_version); + to->di_changecount =cpu_to_be64 (inode_peek_iversion (inode)); to->di_crtime.t_sec =cpu_to_be32 (from->di_crtime.t_sec); to->di_crtime.t_nsec =cpu_to_be32 (from->di_crtime.t_nsec); to->di_flags2 =cpu_to_be64 (from->di_flags2);
fs/xfs/xfs_icache.c +3 -2
@@ -37,6 +37,7 @@#include <linux/kthread.h> #include <linux/freezer.h> +#include <linux/iversion.h> /* * Allocate and initialise an xfs_inode.
@@ -293,14 +294,14 @@ xfs_reinit_inode(int error;uint32_t nlink = inode->i_nlink;uint32_t generation = inode->i_generation; -uint64_t version = inode->i_version; +uint64_t version =inode_peek_iversion (inode);umode_t mode = inode->i_mode; error =inode_init_always (mp->m_super, inode);set_nlink (inode, nlink); inode->i_generation = generation; - inode->i_version = version; +inode_set_iversion_queried (inode, version); inode->i_mode = mode;return error; }
fs/xfs/xfs_inode.c +2 -1
@@ -16,6 +16,7 @@* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ #include <linux/log2.h> +#include <linux/iversion.h> #include "xfs.h" #include "xfs_fs.h"
@@ -833,7 +834,7 @@ xfs_ialloc( ip->i_d.di_flags =0 ;if (ip->i_d.di_version ==3 ) { - inode->i_version =1 ; +inode_set_iversion (inode,1 ); ip->i_d.di_flags2 =0 ; ip->i_d.di_cowextsize =0 ; ip->i_d.di_crtime.t_sec = (int32_t )tv.tv_sec;
fs/xfs/xfs_inode_item.c +2 -1
@@ -30,6 +30,7 @@#include "xfs_buf_item.h" #include "xfs_log.h" +#include <linux/iversion.h> kmem_zone_t *xfs_ili_zone;/* inode log item zone */
@@ -354,7 +355,7 @@ xfs_inode_to_log_dinode( to->di_next_unlinked =NULLAGINO ;if (from->di_version ==3 ) { - to->di_changecount = inode->i_version; + to->di_changecount =inode_peek_iversion (inode); to->di_crtime.t_sec = from->di_crtime.t_sec; to->di_crtime.t_nsec = from->di_crtime.t_nsec; to->di_flags2 = from->di_flags2;
fs/xfs/xfs_trans_inode.c +3 -1
@@ -28,6 +28,8 @@#include "xfs_inode_item.h" #include "xfs_trace.h" +#include <linux/iversion.h> +/* * Add a locked inode to the transaction. *
@@ -117,7 +119,7 @@ xfs_trans_log_inode(*/ if (!(ip->i_itemp->ili_item.li_desc->lid_flags &XFS_LID_DIRTY ) &&IS_I_VERSION (VFS_I (ip))) { -VFS_I (ip)->i_version++; +inode_inc_iversion (VFS_I (ip)); flags |=XFS_ILOG_CORE ; }
Signature · 1 line
2.14.3
Re: [PATCH v5 14/19] xfs: convert to new i_version API
Quoted 4 lines · Jeff Layton
From: Jeff Layton <[email protected]>
Signed-off-by Jeff Layton <[email protected]>Acked-by Darrick J. Wong <[email protected]>
Looks ok, but I haven't tested it at all.
Signature · 2 lines
Dave Chinner [email protected]
Jeff Layton
maintainerPATCH v5 15/19 IMA: switch IMA over to new i_version API
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
--- security/integrity/ima/ima_api.c | 3 ++- security/integrity/ima/ima_main.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
security/integrity/ima/ima_api.c +2 -1
@@ -18,6 +18,7 @@#include <linux/fs.h> #include <linux/xattr.h> #include <linux/evm.h> +#include <linux/iversion.h> #include "ima.h"
@@ -215,7 +216,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,* which do not support i_version, support is limited to an initial * measurement/appraisal/audit. */ - i_version =file_inode (file)->i_version; + i_version =inode_query_iversion (inode); hash.hdr.algo = algo;/* Initialize hash digest to 0's in case of failure */
security/integrity/ima/ima_main.c +2 -1
@@ -24,6 +24,7 @@#include <linux/slab.h> #include <linux/xattr.h> #include <linux/ima.h> +#include <linux/iversion.h> #include "ima.h"
@@ -128,7 +129,7 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,inode_lock (inode);if (atomic_read (&inode->i_writecount) ==1 ) {if (!IS_I_VERSION (inode) || - (iint->version != inode->i_version) || +inode_cmp_iversion (inode, iint->version) || (iint->flags &IMA_NEW_FILE )) { iint->flags &= ~(IMA_DONE_MASK |IMA_NEW_FILE ); iint->measured_pcrs =0 ;
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 16/19 fs: only set S_VERSION when updating times if necessary
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
We only really need to update i_version if someone has queried for it since we last incremented it. By doing that, we can avoid having to update the inode if the times haven't changed.
If the times have changed, then we go ahead and forcibly increment the counter, under the assumption that we'll be going to the storage anyway, and the increment itself is relatively cheap.
--- fs/inode.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
fs/inode.c +7 -3
@@ -1635,17 +1635,21 @@ static int relatime_need_update(const struct path *path, struct inode *inode,int generic_update_time (struct inode *inode,struct timespec *time,int flags) {int iflags =I_DIRTY_TIME ; +bool dirty = false;if (flags &S_ATIME ) inode->i_atime = *time;if (flags &S_VERSION ) -inode_inc_iversion (inode); + dirty =inode_maybe_inc_iversion (inode, false);if (flags &S_CTIME ) inode->i_ctime = *time;if (flags &S_MTIME ) inode->i_mtime = *time; +if ((flags & (S_ATIME |S_CTIME |S_MTIME )) && + !(inode->i_sb->s_flags &SB_LAZYTIME )) + dirty = true; -if (!(inode->i_sb->s_flags &SB_LAZYTIME ) || (flags &S_VERSION )) +if (dirty) iflags |=I_DIRTY_SYNC ;__mark_inode_dirty (inode, iflags);return 0 ;
@@ -1864,7 +1868,7 @@ int file_update_time(struct file *file)if (!timespec_equal (&inode->i_ctime, &now)) sync_it |=S_CTIME ; -if (IS_I_VERSION (inode)) +if (IS_I_VERSION (inode) &&inode_iversion_need_inc (inode)) sync_it |=S_VERSION ;if (!sync_it)
Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 17/19 xfs: avoid setting XFS_ILOG_CORE if i_version doesn't need incrementing
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
If XFS_ILOG_CORE is already set then go ahead and increment it.
--- fs/xfs/xfs_trans_inode.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
fs/xfs/xfs_trans_inode.c +8 -6
@@ -112,15 +112,17 @@ xfs_trans_log_inode(/* * First time we log the inode in a transaction, bump the inode change -* counter if it is configured for this to occur. We don't use -* inode_inc_version() because there is no need for extra locking around -* i_version as we already hold the inode locked exclusively for -* metadata modification. +* counter if it is configured for this to occur. While we have the +* inode locked exclusively for metadata modification, we can usually +* avoid setting XFS_ILOG_CORE if no one has queried the value since +* the last time it was incremented. If we have XFS_ILOG_CORE already +* set however, then go ahead and bump the i_version counter +* unconditionally. */ if (!(ip->i_itemp->ili_item.li_desc->lid_flags &XFS_LID_DIRTY ) &&IS_I_VERSION (VFS_I (ip))) { -inode_inc_iversion (VFS_I (ip)); - flags |=XFS_ILOG_CORE ; +if (inode_maybe_inc_iversion (VFS_I (ip), flags &XFS_ILOG_CORE )) + flags |=XFS_ILOG_CORE ; } tp->t_flags |=XFS_TRANS_DIRTY ;
Signature · 1 line
2.14.3
Re: [PATCH v5 17/19] xfs: avoid setting XFS_ILOG_CORE if i_version doesn't need incrementing
Quoted 6 lines · Jeff Layton
From: Jeff Layton <[email protected]>
If
XFS_ILOG_COREis already set then go ahead and increment it.Signed-off-by Jeff Layton <[email protected]>Acked-by Darrick J. Wong <[email protected]>
Signature · 2 lines
Dave Chinner [email protected]
Jeff Layton
maintainerPATCH v5 18/19 btrfs: only dirty the inode in btrfs_update_time if something was changed
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
At this point, we know that "now" and the file times may differ, and we suspect that the i_version has been flagged to be bumped. Attempt to bump the i_version, and only mark the inode dirty if that actually occurred or if one of the times was updated.
--- fs/btrfs/inode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
fs/btrfs/inode.c +3 -2
@@ -6107,19 +6107,20 @@ static int btrfs_update_time(struct inode *inode, struct timespec *now, int flags) {struct btrfs_root *root =BTRFS_I (inode)->root; +bool dirty = flags & ~S_VERSION ;if (btrfs_root_readonly (root))return -EROFS ;if (flags &S_VERSION ) -inode_inc_iversion (inode); + dirty |=inode_maybe_inc_iversion (inode, dirty);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); +return dirty ?btrfs_dirty_inode (inode) :0 ; }/*
Signature · 1 line
2.14.3
Re: [PATCH v5 18/19] btrfs: only dirty the inode in btrfs_update_time if something was changed
Hi Jeff,
Quoted 29 lines · Jeff Layton
From: Jeff Layton <[email protected]>
At this point, we know that "now" and the file times may differ, and we suspect that the
i_versionhas been flagged to be bumped. Attempt to bump thei_version, and only mark the inode dirty if that actually occurred or if one of the times was updated.Signed-off-by Jeff Layton <[email protected]>Acked-by David Sterba <[email protected]>--- fs/btrfs/inode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)fs/btrfs/inode.c +2
-1@@ -6107,19 +6107,20 @@ static int btrfs_update_time(struct inode *inode, struct timespec *now, int flags) {struct btrfs_root *root =BTRFS_I (inode)->root; +bool dirty = flags & ~S_VERSION ;if (btrfs_root_readonly (root))return -EROFS ;if (flags &S_VERSION ) -inode_inc_iversion (inode); + dirty |=inode_maybe_inc_iversion (inode, dirty);
fs/btrfs/inode.c static int btrfs_update_time(struct inode *inode, struct timespec *now, · line 6125
Quoted 10 lines
fs/btrfs/inode.c +1
-1if (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); +return dirty ?btrfs_dirty_inode (inode) :0 ; }
In order to have a easier error handling for ENOSPC, btrfs has split file_update_time() into 2-step work,
- step 1 is to update time/iversion of in-memory inode in update_time_for_write(),
- step 2 is to update the time/iversion of on-disk inode in btrfs_update_inode()->fill_inode_item().
I think update_time_for_write() also needs to be updated to use the new iversion API.
With that,
Thanks,
-liubo
Quoted 4 lines
fs/btrfs/inode.c +0
-0/* Signature · 1 line
2.14.3
Jeff Layton
maintainerPATCH v5 19/19 fs: handle inode->i_version more efficiently
In reply to Jeff LaytonFrom: Jeff Layton <[email protected]>
Since i_version is mostly treated as an opaque value, we can exploit that fact to avoid incrementing it when no one is watching. With that change, we can avoid incrementing the counter on writes, unless someone has queried for it since it was last incremented. If the a/c/mtime don't change, and the i_version hasn't changed, then there's no need to dirty the inode metadata on a write.
Convert the i_version counter to an atomic64_t, and use the lowest order bit to hold a flag that will tell whether anyone has queried the value since it was last incremented.
When we go to maybe increment it, we fetch the value and check the flag bit. If it's clear then we don't need to do anything if the update isn't being forced.
If we do need to update, then we increment the counter by 2, and clear the flag bit, and then use a CAS op to swap it into place. If that works, we return true. If it doesn't then do it again with the value that we fetch from the CAS operation.
On the query side, if the flag is already set, then we just shift the value down by 1 bit and return it. Otherwise, we set the flag in our on-stack value and again use cmpxchg to swap it into place if it hasn't changed. If it has, then we use the value from the cmpxchg as the new "old" value and try again.
This method allows us to avoid incrementing the counter on writes (and dirtying the metadata) under typical workloads. We only need to increment if it has been queried since it was last changed.
--- include/linux/fs.h | 2 +- include/linux/iversion.h | 210 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 155 insertions(+), 57 deletions(-)
include/linux/fs.h +1 -1
@@ -639,7 +639,7 @@ struct inode {struct hlist_head i_dentry;struct rcu_head i_rcu; }; -u64 i_version; +atomic64_t i_version;atomic_t i_count;atomic_t i_dio_count;atomic_t i_writecount;
include/linux/iversion.h +154 -56
314 lines not on this page · raw message
Signature · 1 line
2.14.3
Re: [PATCH v5 19/19] fs: handle inode->i_version more efficiently
Quoted 28 lines · Jeff Layton
From: Jeff Layton <[email protected]>
Since
i_versionis mostly treated as an opaque value, we can exploit that fact to avoid incrementing it when no one is watching. With that change, we can avoid incrementing the counter on writes, unless someone has queried for it since it was last incremented. If the a/c/mtime don't change, and thei_versionhasn't changed, then there's no need to dirty the inode metadata on a write.Convert the
i_versioncounter to anatomic64_t, and use the lowest order bit to hold a flag that will tell whether anyone has queried the value since it was last incremented.When we go to maybe increment it, we fetch the value and check the flag bit. If it's clear then we don't need to do anything if the update isn't being forced.
If we do need to update, then we increment the counter by 2, and clear the flag bit, and then use a CAS op to swap it into place. If that works, we return true. If it doesn't then do it again with the value that we fetch from the CAS operation.
On the query side, if the flag is already set, then we just shift the value down by 1 bit and return it. Otherwise, we set the flag in our on-stack value and again use cmpxchg to swap it into place if it hasn't changed. If it has, then we use the value from the cmpxchg as the new "old" value and try again.
Quoted 6 lines
This method allows us to avoid incrementing the counter on writes (and dirtying the metadata) under typical workloads. We only need to increment if it has been queried since it was last changed.
Signed-off-by Jeff Layton <[email protected]>Reviewed-by Jan Kara <[email protected]>
Documentation helps a lot in understanding all this. Thanks for adding it into the patch!
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH v5 19/19] fs: handle inode->i_version more efficiently
In reply to Jeff LaytonQuoted 32 lines · Jeff Layton
From: Jeff Layton <[email protected]>
Since
i_versionis mostly treated as an opaque value, we can exploit that fact to avoid incrementing it when no one is watching. With that change, we can avoid incrementing the counter on writes, unless someone has queried for it since it was last incremented. If the a/c/mtime don't change, and thei_versionhasn't changed, then there's no need to dirty the inode metadata on a write.Convert the
i_versioncounter to anatomic64_t, and use the lowest order bit to hold a flag that will tell whether anyone has queried the value since it was last incremented.When we go to maybe increment it, we fetch the value and check the flag bit. If it's clear then we don't need to do anything if the update isn't being forced.
If we do need to update, then we increment the counter by 2, and clear the flag bit, and then use a CAS op to swap it into place. If that works, we return true. If it doesn't then do it again with the value that we fetch from the CAS operation.
On the query side, if the flag is already set, then we just shift the value down by 1 bit and return it. Otherwise, we set the flag in our on-stack value and again use cmpxchg to swap it into place if it hasn't changed. If it has, then we use the value from the cmpxchg as the new "old" value and try again.
This method allows us to avoid incrementing the counter on writes (and dirtying the metadata) under typical workloads. We only need to increment if it has been queried since it was last changed.
Quoted 6 lines
Signed-off-by Jeff Layton <[email protected]>Reviewed-by Jan Kara <[email protected]>--- include/linux/fs.h | 2 +- include/linux/iversion.h | 210 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 155 insertions(+), 57 deletions(-)
Works fine on my nfsroot configuration so looks like the issues from v4 are gone. FWIW:
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 76 lines · Jeff Layton
From: Jeff Layton <[email protected]>
v5:
- don't corrupt refcounts stashed ini_versionof ext4 xattr inodes
- add raw variants of inc and cmp functions, and have nfs use themv4:
- fixSB_LAZYTIMEhandling ingeneric_update_time
- add memory barriers to patch to converti_versionfield toatomic64_tv3:
- movei_versionhandling functions to new header file
- document that the kernel-managedi_versionimplementation will appear to increase over time
- fixinode_cmp_iversionto handle wraparound correctlyv2:
- xfs should useinode_peek_iversioninstead ofinode_peek_iversion_raw
- reworkfile_update_timepatch
- don't dirty inode when onlyS_ATIMEis set andSB_LAZYTIMEis enabled
- better comments and documentationI think this is now approaching merge readiness.
Special thanks to Jan Kara and Dave Chinner who helped me tighten up the memory barriers in the final patch, and Krzysztof Kozlowski for help in tracking down a set of bugs in the NFS client patch.
tl;dr: I think we can greatly reduce the cost of the
inode->i_versioncounter, 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 viastatx().Note that this set relies on a few patches that are in other trees. The full stack that I've been testing with is here:
https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/log/?h=iversion
The
inode->i_versionfield 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 hasMS_I_VERSIONset. IMA will also use it to optimize away some remeasurement if it's available. NFS and AFS just use it to store an opaque change attribute from the server.Only btrfs, ext4, and xfs increment it for data changes. Because of this, these filesystems must log the inode to disk whenever the
i_versioncounter 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.It turns out though that none of these users of
i_versionrequire that it 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.If we keep track of when something queries the value, we can avoid bumping the counter and an on-disk update when nothing else has changed if no one has queried it since it was last incremented.
This patchset changes the code to only bump the
i_versioncounter when it's strictly necessary, or when we're updating the inode metadata anyway (e.g. when times change).It takes the approach of converting the existing accessors of
i_versionto use a new API, while leaving the underlying implementation mostly the same. The last patch then converts the existing implementation to keep track of whether the value has been queried since it was last incremented. It then uses that to avoid incrementing the counter when it can.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).
Quoted 7 lines
I can see measurable performance gains on xfs and ext4 with iversion enabled, when streaming small (4k) I/Os.
btrfs shows some slight gain in testing, but not quite the magnitude that xfs and ext4 show. I'm not sure why yet and would appreciate some input from btrfs folks.
Thanks for the patchset.
Not sure about how you tested the performance, but in terms of write+fsync or synchronous write, btrfs's fsync doesn't check if no timestamp/iversion has been changed, instead only checks if inode has been logged by some btrfs internal flags and counters, probably because by default every write is cow and every write incurs a new block allocation and some update is required.
Long time ago I made an attempt to fix fsync of nocow case where we can skip the heavy flushing log part iff there is no timestamp/iversion update and no isize change[1], but it ended no where. It was not a complete fix, but is good to explain the problem we have.
[1]: https://www.spinics.net/lists/linux-btrfs/msg44762.html
Thanks,
-liubo
Quoted 71 lines
My goal is to get this into linux-next fairly soon. If it shows no problems then we can look at merging it for 4.16, or 4.17 if all of the prequisite patches are not yet merged.
Jeff Layton (19):
fs: new API for handlinginode->i_version
fs: don't take thei_lockininode_inc_iversion
fat: convert to newi_versionAPI
affs: convert to newi_versionAPI
afs: convert to newi_versionAPI
btrfs: convert to newi_versionAPI
exofs: switch to newi_versionAPI
ext2: convert to newi_versionAPI
ext4: convert to newi_versionAPI
nfs: convert to newi_versionAPI
nfsd: convert to newi_versionAPI
ocfs2: convert to newi_versionAPI
ufs: use newi_versionAPI
xfs: convert to newi_versionAPI
IMA: switch IMA over to newi_versionAPI
fs: only setS_VERSIONwhen updating times if necessary
xfs: avoid settingXFS_ILOG_COREifi_versiondoesn't need
incrementing
btrfs: only dirty the inode inbtrfs_update_timeif something was
changed
fs: handleinode->i_versionmore efficientlyfs/affs/amigaffs.c | 5 +- fs/affs/dir.c | 5 +- fs/affs/super.c | 3 +- fs/afs/fsclient.c | 3 +- fs/afs/inode.c | 5 +- fs/btrfs/delayed-inode.c | 7 +- fs/btrfs/file.c | 1 + fs/btrfs/inode.c | 12 +- fs/btrfs/ioctl.c | 1 + fs/btrfs/tree-log.c | 4 +- fs/btrfs/xattr.c | 1 + fs/exofs/dir.c | 9 +- fs/exofs/super.c | 3 +- fs/ext2/dir.c | 9 +- fs/ext2/super.c | 5 +- fs/ext4/dir.c | 9 +- fs/ext4/inline.c | 7 +- fs/ext4/inode.c | 13 +- fs/ext4/ioctl.c | 3 +- fs/ext4/namei.c | 5 +- fs/ext4/super.c | 3 +- fs/ext4/xattr.c | 5 +- fs/fat/dir.c | 3 +- fs/fat/inode.c | 9 +- fs/fat/namei_msdos.c | 7 +- fs/fat/namei_vfat.c | 22 +-- fs/inode.c | 11 +- fs/nfs/delegation.c | 3 +- fs/nfs/fscache-index.c | 5 +- fs/nfs/inode.c | 18 +- fs/nfs/nfs4proc.c | 10 +- fs/nfs/nfstrace.h | 5 +- fs/nfs/write.c | 8 +- fs/nfsd/nfsfh.h | 3 +- fs/ocfs2/dir.c | 15 +- fs/ocfs2/inode.c | 3 +- fs/ocfs2/namei.c | 3 +- fs/ocfs2/quota_global.c | 3 +- fs/ufs/dir.c | 9 +- fs/ufs/inode.c | 3 +- fs/ufs/super.c | 3 +- fs/xfs/libxfs/xfs_inode_buf.c | 7 +- fs/xfs/xfs_icache.c | 5 +- fs/xfs/xfs_inode.c | 3 +-
Quoted 12 lines
fs/xfs/xfs_inode_item.c | 3 +- fs/xfs/xfs_trans_inode.c | 16 +- include/linux/fs.h | 17 +- include/linux/iversion.h | 335 ++++++++++++++++++++++++++++++++++++++ security/integrity/ima/ima_api.c | 3 +- security/integrity/ima/ima_main.c | 3 +- 50 files changed, 518 insertions(+), 135 deletions(-) create mode 100644 include/linux/iversion.hSignature · 1 line
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Jeff Layton
maintainerQuoted 85 lines · Liu Bo
Quoted 76 lines · Jeff Layton
From: Jeff Layton <[email protected]>
v5:
- don't corrupt refcounts stashed ini_versionof ext4 xattr inodes
- add raw variants of inc and cmp functions, and have nfs use themv4:
- fixSB_LAZYTIMEhandling ingeneric_update_time
- add memory barriers to patch to converti_versionfield toatomic64_tv3:
- movei_versionhandling functions to new header file
- document that the kernel-managedi_versionimplementation will appear to increase over time
- fixinode_cmp_iversionto handle wraparound correctlyv2:
- xfs should useinode_peek_iversioninstead ofinode_peek_iversion_raw
- reworkfile_update_timepatch
- don't dirty inode when onlyS_ATIMEis set andSB_LAZYTIMEis enabled
- better comments and documentationI think this is now approaching merge readiness.
Special thanks to Jan Kara and Dave Chinner who helped me tighten up the memory barriers in the final patch, and Krzysztof Kozlowski for help in tracking down a set of bugs in the NFS client patch.
tl;dr: I think we can greatly reduce the cost of the
inode->i_versioncounter, 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 viastatx().Note that this set relies on a few patches that are in other trees. The full stack that I've been testing with is here:
https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git/log/?h=iversion
The
inode->i_versionfield 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 hasMS_I_VERSIONset. IMA will also use it to optimize away some remeasurement if it's available. NFS and AFS just use it to store an opaque change attribute from the server.Only btrfs, ext4, and xfs increment it for data changes. Because of this, these filesystems must log the inode to disk whenever the
i_versioncounter 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.It turns out though that none of these users of
i_versionrequire that it 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.If we keep track of when something queries the value, we can avoid bumping the counter and an on-disk update when nothing else has changed if no one has queried it since it was last incremented.
This patchset changes the code to only bump the
i_versioncounter when it's strictly necessary, or when we're updating the inode metadata anyway (e.g. when times change).It takes the approach of converting the existing accessors of
i_versionto use a new API, while leaving the underlying implementation mostly the same. The last patch then converts the existing implementation to keep track of whether the value has been queried since it was last incremented. It then uses that to avoid incrementing the counter when it can.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).
Quoted 7 lines
I can see measurable performance gains on xfs and ext4 with iversion enabled, when streaming small (4k) I/Os.
btrfs shows some slight gain in testing, but not quite the magnitude that xfs and ext4 show. I'm not sure why yet and would appreciate some input from btrfs folks.
Quoted 9 lines
Thanks for the patchset.
Not sure about how you tested the performance, but in terms of write+fsync or synchronous write, btrfs's fsync doesn't check if no timestamp/iversion has been changed, instead only checks if inode has been logged by some btrfs internal flags and counters, probably because by default every write is cow and every write incurs a new block allocation and some update is required.
I tested it in various ways, but it mostly seems to help small writes without fsyncing between them, particularly on fast storage. At least with xfs and ext4.
btrfs didn't show much improvement at all, which I took as an indication that it ends up logging to disk for other reasons in this situation.
Quoted 11 lines
Long time ago I made an attempt to fix fsync of nocow case where we can skip the heavy flushing log part iff there is no timestamp/iversion update and no isize change[1], but it ended no where. It was not a complete fix, but is good to explain the problem we have.
[1]: https://www.spinics.net/lists/linux-btrfs/msg44762.html
Thanks,
-liubo
Got it, thanks. I guess nocow is pretty rare under btrfs, so any benefit there is likely to be minimal?
In any case, let me know if you want to try to make better use of this new infrastructure, and I'll see if I can help.
Quoted 80 lines
Quoted 80 lines
My goal is to get this into linux-next fairly soon. If it shows no problems then we can look at merging it for 4.16, or 4.17 if all of the prequisite patches are not yet merged.
Jeff Layton (19):
fs: new API for handlinginode->i_version
fs: don't take thei_lockininode_inc_iversion
fat: convert to newi_versionAPI
affs: convert to newi_versionAPI
afs: convert to newi_versionAPI
btrfs: convert to newi_versionAPI
exofs: switch to newi_versionAPI
ext2: convert to newi_versionAPI
ext4: convert to newi_versionAPI
nfs: convert to newi_versionAPI
nfsd: convert to newi_versionAPI
ocfs2: convert to newi_versionAPI
ufs: use newi_versionAPI
xfs: convert to newi_versionAPI
IMA: switch IMA over to newi_versionAPI
fs: only setS_VERSIONwhen updating times if necessary
xfs: avoid settingXFS_ILOG_COREifi_versiondoesn't need
incrementing
btrfs: only dirty the inode inbtrfs_update_timeif something was
changed
fs: handleinode->i_versionmore efficientlyfs/affs/amigaffs.c | 5 +- fs/affs/dir.c | 5 +- fs/affs/super.c | 3 +- fs/afs/fsclient.c | 3 +- fs/afs/inode.c | 5 +- fs/btrfs/delayed-inode.c | 7 +- fs/btrfs/file.c | 1 + fs/btrfs/inode.c | 12 +- fs/btrfs/ioctl.c | 1 + fs/btrfs/tree-log.c | 4 +- fs/btrfs/xattr.c | 1 + fs/exofs/dir.c | 9 +- fs/exofs/super.c | 3 +- fs/ext2/dir.c | 9 +- fs/ext2/super.c | 5 +- fs/ext4/dir.c | 9 +- fs/ext4/inline.c | 7 +- fs/ext4/inode.c | 13 +- fs/ext4/ioctl.c | 3 +- fs/ext4/namei.c | 5 +- fs/ext4/super.c | 3 +- fs/ext4/xattr.c | 5 +- fs/fat/dir.c | 3 +- fs/fat/inode.c | 9 +- fs/fat/namei_msdos.c | 7 +- fs/fat/namei_vfat.c | 22 +-- fs/inode.c | 11 +- fs/nfs/delegation.c | 3 +- fs/nfs/fscache-index.c | 5 +- fs/nfs/inode.c | 18 +- fs/nfs/nfs4proc.c | 10 +- fs/nfs/nfstrace.h | 5 +- fs/nfs/write.c | 8 +- fs/nfsd/nfsfh.h | 3 +- fs/ocfs2/dir.c | 15 +- fs/ocfs2/inode.c | 3 +- fs/ocfs2/namei.c | 3 +- fs/ocfs2/quota_global.c | 3 +- fs/ufs/dir.c | 9 +- fs/ufs/inode.c | 3 +- fs/ufs/super.c | 3 +- fs/xfs/libxfs/xfs_inode_buf.c | 7 +- fs/xfs/xfs_icache.c | 5 +- fs/xfs/xfs_inode.c | 3 +- fs/xfs/xfs_inode_item.c | 3 +- fs/xfs/xfs_trans_inode.c | 16 +- include/linux/fs.h | 17 +- include/linux/iversion.h | 335 ++++++++++++++++++++++++++++++++++++++ security/integrity/ima/ima_api.c | 3 +- security/integrity/ima/ima_main.c | 3 +- 50 files changed, 518 insertions(+), 135 deletions(-) create mode 100644 include/linux/iversion.h
Quoted 3 lines
Quoted 3 lines
Signature · 1 line
2.14.3
Thanks,
Signature · 1 line
Jeff Layton <[email protected]>