On Sun, Oct 16, 2022 at 10:19:36AM -0400, Jeff Layton wrote: > On Sun, 2022-10-16 at 15:24 +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote: > > This is a note to let you know that I've just added the patch titled > > > > ext4: unconditionally enable the i_version counter > > > > to the 5.19-stable tree which can be found at: > > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > > > The filename of the patch is: > > ext4-unconditionally-enable-the-i_version-counter.patch > > and it can be found in the queue-5.19 subdirectory. > > > > If you, or anyone else, feels it should not be added to the stable tree, > > please let <stable@xxxxxxxxxxxxxxx> know about it. > > > > > > From 1ff20307393e17dc57fde62226df625a3a3c36e9 Mon Sep 17 00:00:00 2001 > > From: Jeff Layton <jlayton@xxxxxxxxxx> > > Date: Wed, 24 Aug 2022 18:03:49 +0200 > > Subject: ext4: unconditionally enable the i_version counter > > > > From: Jeff Layton <jlayton@xxxxxxxxxx> > > > > commit 1ff20307393e17dc57fde62226df625a3a3c36e9 upstream. > > > > The original i_version implementation was pretty expensive, requiring a > > log flush on every change. Because of this, it was gated behind a mount > > option (implemented via the MS_I_VERSION mountoption flag). > > > > Commit ae5e165d855d (fs: new API for handling inode->i_version) made the > > i_version flag much less expensive, so there is no longer a performance > > penalty from enabling it. xfs and btrfs already enable it > > unconditionally when the on-disk format can support it. > > > > Have ext4 ignore the SB_I_VERSION flag, and just enable it > > unconditionally. While we're in here, mark the i_version mount > > option Opt_removed. > > > > [ Removed leftover bits of i_version from ext4_apply_options() since it > > now can't ever be set in ctx->mask_s_flags -- lczerner ] > > > > Cc: stable@xxxxxxxxxx > > Cc: Dave Chinner <david@xxxxxxxxxxxxx> > > Cc: Benjamin Coddington <bcodding@xxxxxxxxxx> > > Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx> > > Cc: Darrick J. Wong <djwong@xxxxxxxxxx> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx> > > Reviewed-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx> > > Reviewed-by: Jan Kara <jack@xxxxxxx> > > Link: https://lore.kernel.org/r/20220824160349.39664-3-lczerner@xxxxxxxxxx > > Signed-off-by: Theodore Ts'o <tytso@xxxxxxx> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > --- > > fs/ext4/inode.c | 5 ++--- > > fs/ext4/super.c | 22 +++++----------------- > > 2 files changed, 7 insertions(+), 20 deletions(-) > > > > --- a/fs/ext4/inode.c > > +++ b/fs/ext4/inode.c > > @@ -5425,7 +5425,7 @@ int ext4_setattr(struct user_namespace * > > return -EINVAL; > > } > > > > - if (IS_I_VERSION(inode) && attr->ia_size != inode->i_size) > > + if (attr->ia_size != inode->i_size) > > inode_inc_iversion(inode); > > > > if (shrink) { > > @@ -5735,8 +5735,7 @@ int ext4_mark_iloc_dirty(handle_t *handl > > * ea_inodes are using i_version for storing reference count, don't > > * mess with it > > */ > > - if (IS_I_VERSION(inode) && > > - !(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)) > > + if (!(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL)) > > inode_inc_iversion(inode); > > > > /* the do_update_inode consumes one bh->b_count */ > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > @@ -1585,7 +1585,7 @@ enum { > > Opt_inlinecrypt, > > Opt_usrjquota, Opt_grpjquota, Opt_quota, > > Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err, > > - Opt_usrquota, Opt_grpquota, Opt_prjquota, Opt_i_version, > > + Opt_usrquota, Opt_grpquota, Opt_prjquota, > > Opt_dax, Opt_dax_always, Opt_dax_inode, Opt_dax_never, > > Opt_stripe, Opt_delalloc, Opt_nodelalloc, Opt_warn_on_error, > > Opt_nowarn_on_error, Opt_mblk_io_submit, Opt_debug_want_extra_isize, > > @@ -1694,7 +1694,7 @@ static const struct fs_parameter_spec ex > > fsparam_flag ("barrier", Opt_barrier), > > fsparam_u32 ("barrier", Opt_barrier), > > fsparam_flag ("nobarrier", Opt_nobarrier), > > - fsparam_flag ("i_version", Opt_i_version), > > + fsparam_flag ("i_version", Opt_removed), > > fsparam_flag ("dax", Opt_dax), > > fsparam_enum ("dax", Opt_dax_type, ext4_param_dax), > > fsparam_u32 ("stripe", Opt_stripe), > > @@ -2140,11 +2140,6 @@ static int ext4_parse_param(struct fs_co > > case Opt_abort: > > ctx_set_mount_flag(ctx, EXT4_MF_FS_ABORTED); > > return 0; > > - case Opt_i_version: > > - ext4_msg(NULL, KERN_WARNING, deprecated_msg, param->key, "5.20"); > > - ext4_msg(NULL, KERN_WARNING, "Use iversion instead\n"); > > - ctx_set_flags(ctx, SB_I_VERSION); > > - return 0; > > case Opt_inlinecrypt: > > #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT > > ctx_set_flags(ctx, SB_INLINECRYPT); > > @@ -2814,14 +2809,6 @@ static void ext4_apply_options(struct fs > > sb->s_flags &= ~ctx->mask_s_flags; > > sb->s_flags |= ctx->vals_s_flags; > > > > - /* > > - * i_version differs from common mount option iversion so we have > > - * to let vfs know that it was set, otherwise it would get cleared > > - * on remount > > - */ > > - if (ctx->mask_s_flags & SB_I_VERSION) > > - fc->sb_flags |= SB_I_VERSION; > > - > > #define APPLY(X) ({ if (ctx->spec & EXT4_SPEC_##X) sbi->X = ctx->X; }) > > APPLY(s_commit_interval); > > APPLY(s_stripe); > > @@ -2970,8 +2957,6 @@ static int _ext4_show_options(struct seq > > SEQ_OPTS_PRINT("min_batch_time=%u", sbi->s_min_batch_time); > > if (nodefs || sbi->s_max_batch_time != EXT4_DEF_MAX_BATCH_TIME) > > SEQ_OPTS_PRINT("max_batch_time=%u", sbi->s_max_batch_time); > > - if (sb->s_flags & SB_I_VERSION) > > - SEQ_OPTS_PUTS("i_version"); > > if (nodefs || sbi->s_stripe) > > SEQ_OPTS_PRINT("stripe=%lu", sbi->s_stripe); > > if (nodefs || EXT4_MOUNT_DATA_FLAGS & > > @@ -4631,6 +4616,9 @@ static int __ext4_fill_super(struct fs_c > > sb->s_flags = (sb->s_flags & ~SB_POSIXACL) | > > (test_opt(sb, POSIX_ACL) ? SB_POSIXACL : 0); > > > > + /* i_version is always enabled now */ > > + sb->s_flags |= SB_I_VERSION; > > + > > if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV && > > (ext4_has_compat_features(sb) || > > ext4_has_ro_compat_features(sb) || > > > > > > Patches currently in stable-queue which might be from jlayton@xxxxxxxxxx are > > > > queue-5.19/nfsd-protect-against-send-buffer-overflow-in-nfsv2-read.patch > > queue-5.19/nfsd-protect-against-send-buffer-overflow-in-nfsv3-read.patch > > queue-5.19/ext4-unconditionally-enable-the-i_version-counter.patch > > queue-5.19/nfsd-protect-against-send-buffer-overflow-in-nfsv3-readdir.patch > > queue-5.19/ext4-fix-i_version-handling-in-ext4.patch > > queue-5.19/ext4-don-t-increase-iversion-counter-for-ea_inodes.patch > > > I think the chance of regression here is very low, but changing the > default setting for this in a stable release seems a bit cavalier. Why did you have the cc: stable marking then? And if it could regress, shouldn't that be fixed no matter where it is? thanks, greg k-h
