| Message ID | 20220819113620.12048-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ext4: fix i_version handling in ext4 | expand |
Oops, I should have labeled this "PATCH v3". In any case, I think we've covered all of the places where it should get bumped now. On Fri, 2022-08-19 at 07:36 -0400, Jeff Layton wrote: > ext4 currently updates the i_version counter when the atime is updated > during a read. This is less than ideal as it can cause unnecessary cache > invalidations with NFSv4 and unnecessary remeasurements for IMA. The > increment in ext4_mark_iloc_dirty is also problematic since it can also > corrupt the i_version counter for ea_inodes. We aren't bumping the file > times in ext4_mark_iloc_dirty, so changing the i_version there seems > wrong, and is the cause of both problems. > > Remove that callsite and add increments to the setattr, setxattr and > ioctl codepaths (at the same time that we update the ctime). The > i_version bump that already happens during timestamp updates should take > care of the rest. > > In ext4_move_extents, increment the i_version on both inodes, and also > add in missing ctime updates. > > Cc: Lukas Czerner <lczerner@redhat.com> > Reviewed-by: Jan Kara <jack@suse.cz> > Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org> > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ext4/inode.c | 10 +++++----- > fs/ext4/ioctl.c | 4 ++++ > fs/ext4/move_extent.c | 6 ++++++ > fs/ext4/xattr.c | 2 ++ > 4 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 601214453c3a..aa37bce4c541 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -5342,6 +5342,7 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > int error, rc = 0; > int orphan = 0; > const unsigned int ia_valid = attr->ia_valid; > + bool inc_ivers = IS_I_VERSION(inode); > > if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb)))) > return -EIO; > @@ -5425,8 +5426,8 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > return -EINVAL; > } > > - if (IS_I_VERSION(inode) && attr->ia_size != inode->i_size) > - inode_inc_iversion(inode); > + if (attr->ia_size == inode->i_size) > + inc_ivers = false; > > if (shrink) { > if (ext4_should_order_data(inode)) { > @@ -5528,6 +5529,8 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > } > > if (!error) { > + if (inc_ivers) > + inode_inc_iversion(inode); > setattr_copy(mnt_userns, inode, attr); > mark_inode_dirty(inode); > } > @@ -5731,9 +5734,6 @@ int ext4_mark_iloc_dirty(handle_t *handle, > } > ext4_fc_track_inode(handle, inode); > > - if (IS_I_VERSION(inode)) > - inode_inc_iversion(inode); > - > /* the do_update_inode consumes one bh->b_count */ > get_bh(iloc->bh); > > diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c > index 3cf3ec4b1c21..ad3a294a88eb 100644 > --- a/fs/ext4/ioctl.c > +++ b/fs/ext4/ioctl.c > @@ -452,6 +452,7 @@ static long swap_inode_boot_loader(struct super_block *sb, > swap_inode_data(inode, inode_bl); > > inode->i_ctime = inode_bl->i_ctime = current_time(inode); > + inode_inc_iversion(inode); > > inode->i_generation = prandom_u32(); > inode_bl->i_generation = prandom_u32(); > @@ -665,6 +666,7 @@ static int ext4_ioctl_setflags(struct inode *inode, > ext4_set_inode_flags(inode, false); > > inode->i_ctime = current_time(inode); > + inode_inc_iversion(inode); > > err = ext4_mark_iloc_dirty(handle, inode, &iloc); > flags_err: > @@ -775,6 +777,7 @@ static int ext4_ioctl_setproject(struct inode *inode, __u32 projid) > > EXT4_I(inode)->i_projid = kprojid; > inode->i_ctime = current_time(inode); > + inode_inc_iversion(inode); > out_dirty: > rc = ext4_mark_iloc_dirty(handle, inode, &iloc); > if (!err) > @@ -1257,6 +1260,7 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > err = ext4_reserve_inode_write(handle, inode, &iloc); > if (err == 0) { > inode->i_ctime = current_time(inode); > + inode_inc_iversion(inode); > inode->i_generation = generation; > err = ext4_mark_iloc_dirty(handle, inode, &iloc); > } > diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c > index 701f1d6a217f..285700b00d38 100644 > --- a/fs/ext4/move_extent.c > +++ b/fs/ext4/move_extent.c > @@ -6,6 +6,7 @@ > */ > > #include <linux/fs.h> > +#include <linux/iversion.h> > #include <linux/quotaops.h> > #include <linux/slab.h> > #include <linux/sched/mm.h> > @@ -683,6 +684,11 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, > break; > o_start += cur_len; > d_start += cur_len; > + > + orig_inode->i_ctime = current_time(orig_inode); > + donor_inode->i_ctime = current_time(donor_inode); > + inode_inc_iversion(orig_inode); > + inode_inc_iversion(donor_inode); > } > *moved_len = o_start - orig_blk; > if (*moved_len > len) > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index 533216e80fa2..e975442e4ab2 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -2412,6 +2412,8 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index, > if (!error) { > ext4_xattr_update_super_block(handle, inode->i_sb); > inode->i_ctime = current_time(inode); > + if (IS_I_VERSION(inode)) > + inode_inc_iversion(inode); > if (!value) > no_expand = 0; > error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 601214453c3a..aa37bce4c541 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -5342,6 +5342,7 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, int error, rc = 0; int orphan = 0; const unsigned int ia_valid = attr->ia_valid; + bool inc_ivers = IS_I_VERSION(inode); if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb)))) return -EIO; @@ -5425,8 +5426,8 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, return -EINVAL; } - if (IS_I_VERSION(inode) && attr->ia_size != inode->i_size) - inode_inc_iversion(inode); + if (attr->ia_size == inode->i_size) + inc_ivers = false; if (shrink) { if (ext4_should_order_data(inode)) { @@ -5528,6 +5529,8 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, } if (!error) { + if (inc_ivers) + inode_inc_iversion(inode); setattr_copy(mnt_userns, inode, attr); mark_inode_dirty(inode); } @@ -5731,9 +5734,6 @@ int ext4_mark_iloc_dirty(handle_t *handle, } ext4_fc_track_inode(handle, inode); - if (IS_I_VERSION(inode)) - inode_inc_iversion(inode); - /* the do_update_inode consumes one bh->b_count */ get_bh(iloc->bh); diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index 3cf3ec4b1c21..ad3a294a88eb 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -452,6 +452,7 @@ static long swap_inode_boot_loader(struct super_block *sb, swap_inode_data(inode, inode_bl); inode->i_ctime = inode_bl->i_ctime = current_time(inode); + inode_inc_iversion(inode); inode->i_generation = prandom_u32(); inode_bl->i_generation = prandom_u32(); @@ -665,6 +666,7 @@ static int ext4_ioctl_setflags(struct inode *inode, ext4_set_inode_flags(inode, false); inode->i_ctime = current_time(inode); + inode_inc_iversion(inode); err = ext4_mark_iloc_dirty(handle, inode, &iloc); flags_err: @@ -775,6 +777,7 @@ static int ext4_ioctl_setproject(struct inode *inode, __u32 projid) EXT4_I(inode)->i_projid = kprojid; inode->i_ctime = current_time(inode); + inode_inc_iversion(inode); out_dirty: rc = ext4_mark_iloc_dirty(handle, inode, &iloc); if (!err) @@ -1257,6 +1260,7 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) err = ext4_reserve_inode_write(handle, inode, &iloc); if (err == 0) { inode->i_ctime = current_time(inode); + inode_inc_iversion(inode); inode->i_generation = generation; err = ext4_mark_iloc_dirty(handle, inode, &iloc); } diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c index 701f1d6a217f..285700b00d38 100644 --- a/fs/ext4/move_extent.c +++ b/fs/ext4/move_extent.c @@ -6,6 +6,7 @@ */ #include <linux/fs.h> +#include <linux/iversion.h> #include <linux/quotaops.h> #include <linux/slab.h> #include <linux/sched/mm.h> @@ -683,6 +684,11 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, break; o_start += cur_len; d_start += cur_len; + + orig_inode->i_ctime = current_time(orig_inode); + donor_inode->i_ctime = current_time(donor_inode); + inode_inc_iversion(orig_inode); + inode_inc_iversion(donor_inode); } *moved_len = o_start - orig_blk; if (*moved_len > len) diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index 533216e80fa2..e975442e4ab2 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -2412,6 +2412,8 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index, if (!error) { ext4_xattr_update_super_block(handle, inode->i_sb); inode->i_ctime = current_time(inode); + if (IS_I_VERSION(inode)) + inode_inc_iversion(inode); if (!value) no_expand = 0; error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);