| Message ID | 20201015203802.3597742-3-harshadshirwadkar@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add fast commits in Ext4 file system | expand |
On Thu 15-10-20 13:37:54, Harshad Shirwadkar wrote: > We are running out of mount option bits. Add handling for using > s_mount_opt2. Add ext4 and jbd2 fast commit feature flag and also add > ability to turn off the fast commit feature in Ext4. > > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> > --- > fs/ext4/ext4.h | 4 ++++ > fs/ext4/super.c | 27 ++++++++++++++++++++++----- > include/linux/jbd2.h | 5 ++++- > 3 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 1879531a119f..02d7dc378505 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -1213,6 +1213,8 @@ struct ext4_inode_info { > #define EXT4_MOUNT2_EXPLICIT_JOURNAL_CHECKSUM 0x00000008 /* User explicitly > specified journal checksum */ > > +#define EXT4_MOUNT2_JOURNAL_FAST_COMMIT 0x00000010 /* Journal fast commit */ > + > #define clear_opt(sb, opt) EXT4_SB(sb)->s_mount_opt &= \ > ~EXT4_MOUNT_##opt > #define set_opt(sb, opt) EXT4_SB(sb)->s_mount_opt |= \ > @@ -1813,6 +1815,7 @@ static inline bool ext4_verity_in_progress(struct inode *inode) > #define EXT4_FEATURE_COMPAT_RESIZE_INODE 0x0010 > #define EXT4_FEATURE_COMPAT_DIR_INDEX 0x0020 > #define EXT4_FEATURE_COMPAT_SPARSE_SUPER2 0x0200 > +#define EXT4_FEATURE_COMPAT_FAST_COMMIT 0x0400 > #define EXT4_FEATURE_COMPAT_STABLE_INODES 0x0800 Is fast commit really a compat feature? IMO if there are fast commits stored in the journal, the filesystem is actually incompatible with the old kernels because data we guranteed to be permanenly stored may be invisible for the old kernel (since it won't replay fastcommit transactions). ... Oh, now I see that the journal FAST_COMMIT is actually incompat. So what's the point of compat ext4 feature with incompat JBD2 feature? > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 901c1c938276..70256a240442 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -1709,7 +1709,7 @@ enum { > Opt_dioread_nolock, Opt_dioread_lock, > Opt_discard, Opt_nodiscard, Opt_init_itable, Opt_noinit_itable, > Opt_max_dir_size_kb, Opt_nojournal_checksum, Opt_nombcache, > - Opt_prefetch_block_bitmaps, > + Opt_prefetch_block_bitmaps, Opt_no_fc, It would be more consistent to use a name 'Opt_nofc' and IMHO 'fc' is really too short an ambiguous. I agree "nofastcommit" is somewhat long but still OK and much more descriptive... > }; > > static const match_table_t tokens = { > @@ -1796,6 +1796,7 @@ static const match_table_t tokens = { > {Opt_init_itable, "init_itable=%u"}, > {Opt_init_itable, "init_itable"}, > {Opt_noinit_itable, "noinit_itable"}, > + {Opt_no_fc, "no_fc"}, And here "nofastcommit", or perhaps "nofast_commit". Honza
On Wed, Oct 21, 2020 at 9:18 AM Jan Kara <jack@suse.cz> wrote: > > On Thu 15-10-20 13:37:54, Harshad Shirwadkar wrote: > > We are running out of mount option bits. Add handling for using > > s_mount_opt2. Add ext4 and jbd2 fast commit feature flag and also add > > ability to turn off the fast commit feature in Ext4. > > > > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> > > --- > > fs/ext4/ext4.h | 4 ++++ > > fs/ext4/super.c | 27 ++++++++++++++++++++++----- > > include/linux/jbd2.h | 5 ++++- > > 3 files changed, 30 insertions(+), 6 deletions(-) > > > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > > index 1879531a119f..02d7dc378505 100644 > > --- a/fs/ext4/ext4.h > > +++ b/fs/ext4/ext4.h > > @@ -1213,6 +1213,8 @@ struct ext4_inode_info { > > #define EXT4_MOUNT2_EXPLICIT_JOURNAL_CHECKSUM 0x00000008 /* User explicitly > > specified journal checksum */ > > > > +#define EXT4_MOUNT2_JOURNAL_FAST_COMMIT 0x00000010 /* Journal fast commit */ > > + > > #define clear_opt(sb, opt) EXT4_SB(sb)->s_mount_opt &= \ > > ~EXT4_MOUNT_##opt > > #define set_opt(sb, opt) EXT4_SB(sb)->s_mount_opt |= \ > > @@ -1813,6 +1815,7 @@ static inline bool ext4_verity_in_progress(struct inode *inode) > > #define EXT4_FEATURE_COMPAT_RESIZE_INODE 0x0010 > > #define EXT4_FEATURE_COMPAT_DIR_INDEX 0x0020 > > #define EXT4_FEATURE_COMPAT_SPARSE_SUPER2 0x0200 > > +#define EXT4_FEATURE_COMPAT_FAST_COMMIT 0x0400 > > #define EXT4_FEATURE_COMPAT_STABLE_INODES 0x0800 > > Is fast commit really a compat feature? IMO if there are fast commits > stored in the journal, the filesystem is actually incompatible with the > old kernels because data we guranteed to be permanenly stored may be > invisible for the old kernel (since it won't replay fastcommit > transactions). > > ... > > Oh, now I see that the journal FAST_COMMIT is actually incompat. So what's > the point of compat ext4 feature with incompat JBD2 feature? So having fast commits enabled on an ext4 file system doesn't immediately make it incompatible with the older kernels. FS becomes incompatible only if there are fast commits blocks that are stored in the journal. So, one of the tricks that this patchset does is on a clean unmount, since it's guaranteed that there are no fast commit blocks in journal, we clear out the JBD2 incompat flag and preserve the compat flag in ext4. So, we can think of ext4 compat flag as "FS will try fast commits when possible" while jbd2 incompat flag as "There are fast commits blocks present in the journal". Does that make sense? > > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > index 901c1c938276..70256a240442 100644 > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > @@ -1709,7 +1709,7 @@ enum { > > Opt_dioread_nolock, Opt_dioread_lock, > > Opt_discard, Opt_nodiscard, Opt_init_itable, Opt_noinit_itable, > > Opt_max_dir_size_kb, Opt_nojournal_checksum, Opt_nombcache, > > - Opt_prefetch_block_bitmaps, > > + Opt_prefetch_block_bitmaps, Opt_no_fc, > > It would be more consistent to use a name 'Opt_nofc' and IMHO 'fc' is > really too short an ambiguous. I agree "nofastcommit" is somewhat long but > still OK and much more descriptive... Ack > > > }; > > > > static const match_table_t tokens = { > > @@ -1796,6 +1796,7 @@ static const match_table_t tokens = { > > {Opt_init_itable, "init_itable=%u"}, > > {Opt_init_itable, "init_itable"}, > > {Opt_noinit_itable, "noinit_itable"}, > > + {Opt_no_fc, "no_fc"}, > > And here "nofastcommit", or perhaps "nofast_commit". Ack Thanks, Harshad > > Honza > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
On Wed 21-10-20 10:31:48, harshad shirwadkar wrote: > On Wed, Oct 21, 2020 at 9:18 AM Jan Kara <jack@suse.cz> wrote: > > > > On Thu 15-10-20 13:37:54, Harshad Shirwadkar wrote: > > > We are running out of mount option bits. Add handling for using > > > s_mount_opt2. Add ext4 and jbd2 fast commit feature flag and also add > > > ability to turn off the fast commit feature in Ext4. > > > > > > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> > > > --- > > > fs/ext4/ext4.h | 4 ++++ > > > fs/ext4/super.c | 27 ++++++++++++++++++++++----- > > > include/linux/jbd2.h | 5 ++++- > > > 3 files changed, 30 insertions(+), 6 deletions(-) > > > > > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > > > index 1879531a119f..02d7dc378505 100644 > > > --- a/fs/ext4/ext4.h > > > +++ b/fs/ext4/ext4.h > > > @@ -1213,6 +1213,8 @@ struct ext4_inode_info { > > > #define EXT4_MOUNT2_EXPLICIT_JOURNAL_CHECKSUM 0x00000008 /* User explicitly > > > specified journal checksum */ > > > > > > +#define EXT4_MOUNT2_JOURNAL_FAST_COMMIT 0x00000010 /* Journal fast commit */ > > > + > > > #define clear_opt(sb, opt) EXT4_SB(sb)->s_mount_opt &= \ > > > ~EXT4_MOUNT_##opt > > > #define set_opt(sb, opt) EXT4_SB(sb)->s_mount_opt |= \ > > > @@ -1813,6 +1815,7 @@ static inline bool ext4_verity_in_progress(struct inode *inode) > > > #define EXT4_FEATURE_COMPAT_RESIZE_INODE 0x0010 > > > #define EXT4_FEATURE_COMPAT_DIR_INDEX 0x0020 > > > #define EXT4_FEATURE_COMPAT_SPARSE_SUPER2 0x0200 > > > +#define EXT4_FEATURE_COMPAT_FAST_COMMIT 0x0400 > > > #define EXT4_FEATURE_COMPAT_STABLE_INODES 0x0800 > > > > Is fast commit really a compat feature? IMO if there are fast commits > > stored in the journal, the filesystem is actually incompatible with the > > old kernels because data we guranteed to be permanenly stored may be > > invisible for the old kernel (since it won't replay fastcommit > > transactions). > > > > ... > > > > Oh, now I see that the journal FAST_COMMIT is actually incompat. So what's > > the point of compat ext4 feature with incompat JBD2 feature? > So having fast commits enabled on an ext4 file system doesn't > immediately make it incompatible with the older kernels. FS becomes > incompatible only if there are fast commits blocks that are stored in > the journal. So, one of the tricks that this patchset does is on a > clean unmount, since it's guaranteed that there are no fast commit > blocks in journal, we clear out the JBD2 incompat flag and preserve > the compat flag in ext4. So, we can think of ext4 compat flag as "FS > will try fast commits when possible" while jbd2 incompat flag as > "There are fast commits blocks present in the journal". Does that make > sense? Yes, understood. That's clever. Thanks for explanation! But please add the above justification to the description of EXT4_FEATURE_COMPAT_FAST_COMMIT feature or somewhere around that. Honza
Will do, thanks! On Thu, Oct 22, 2020 at 6:09 AM Jan Kara <jack@suse.cz> wrote: > > On Wed 21-10-20 10:31:48, harshad shirwadkar wrote: > > On Wed, Oct 21, 2020 at 9:18 AM Jan Kara <jack@suse.cz> wrote: > > > > > > On Thu 15-10-20 13:37:54, Harshad Shirwadkar wrote: > > > > We are running out of mount option bits. Add handling for using > > > > s_mount_opt2. Add ext4 and jbd2 fast commit feature flag and also add > > > > ability to turn off the fast commit feature in Ext4. > > > > > > > > Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> > > > > --- > > > > fs/ext4/ext4.h | 4 ++++ > > > > fs/ext4/super.c | 27 ++++++++++++++++++++++----- > > > > include/linux/jbd2.h | 5 ++++- > > > > 3 files changed, 30 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > > > > index 1879531a119f..02d7dc378505 100644 > > > > --- a/fs/ext4/ext4.h > > > > +++ b/fs/ext4/ext4.h > > > > @@ -1213,6 +1213,8 @@ struct ext4_inode_info { > > > > #define EXT4_MOUNT2_EXPLICIT_JOURNAL_CHECKSUM 0x00000008 /* User explicitly > > > > specified journal checksum */ > > > > > > > > +#define EXT4_MOUNT2_JOURNAL_FAST_COMMIT 0x00000010 /* Journal fast commit */ > > > > + > > > > #define clear_opt(sb, opt) EXT4_SB(sb)->s_mount_opt &= \ > > > > ~EXT4_MOUNT_##opt > > > > #define set_opt(sb, opt) EXT4_SB(sb)->s_mount_opt |= \ > > > > @@ -1813,6 +1815,7 @@ static inline bool ext4_verity_in_progress(struct inode *inode) > > > > #define EXT4_FEATURE_COMPAT_RESIZE_INODE 0x0010 > > > > #define EXT4_FEATURE_COMPAT_DIR_INDEX 0x0020 > > > > #define EXT4_FEATURE_COMPAT_SPARSE_SUPER2 0x0200 > > > > +#define EXT4_FEATURE_COMPAT_FAST_COMMIT 0x0400 > > > > #define EXT4_FEATURE_COMPAT_STABLE_INODES 0x0800 > > > > > > Is fast commit really a compat feature? IMO if there are fast commits > > > stored in the journal, the filesystem is actually incompatible with the > > > old kernels because data we guranteed to be permanenly stored may be > > > invisible for the old kernel (since it won't replay fastcommit > > > transactions). > > > > > > ... > > > > > > Oh, now I see that the journal FAST_COMMIT is actually incompat. So what's > > > the point of compat ext4 feature with incompat JBD2 feature? > > So having fast commits enabled on an ext4 file system doesn't > > immediately make it incompatible with the older kernels. FS becomes > > incompatible only if there are fast commits blocks that are stored in > > the journal. So, one of the tricks that this patchset does is on a > > clean unmount, since it's guaranteed that there are no fast commit > > blocks in journal, we clear out the JBD2 incompat flag and preserve > > the compat flag in ext4. So, we can think of ext4 compat flag as "FS > > will try fast commits when possible" while jbd2 incompat flag as > > "There are fast commits blocks present in the journal". Does that make > > sense? > > Yes, understood. That's clever. Thanks for explanation! But please add the > above justification to the description of EXT4_FEATURE_COMPAT_FAST_COMMIT > feature or somewhere around that. > > Honza > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 1879531a119f..02d7dc378505 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1213,6 +1213,8 @@ struct ext4_inode_info { #define EXT4_MOUNT2_EXPLICIT_JOURNAL_CHECKSUM 0x00000008 /* User explicitly specified journal checksum */ +#define EXT4_MOUNT2_JOURNAL_FAST_COMMIT 0x00000010 /* Journal fast commit */ + #define clear_opt(sb, opt) EXT4_SB(sb)->s_mount_opt &= \ ~EXT4_MOUNT_##opt #define set_opt(sb, opt) EXT4_SB(sb)->s_mount_opt |= \ @@ -1813,6 +1815,7 @@ static inline bool ext4_verity_in_progress(struct inode *inode) #define EXT4_FEATURE_COMPAT_RESIZE_INODE 0x0010 #define EXT4_FEATURE_COMPAT_DIR_INDEX 0x0020 #define EXT4_FEATURE_COMPAT_SPARSE_SUPER2 0x0200 +#define EXT4_FEATURE_COMPAT_FAST_COMMIT 0x0400 #define EXT4_FEATURE_COMPAT_STABLE_INODES 0x0800 #define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER 0x0001 @@ -1915,6 +1918,7 @@ EXT4_FEATURE_COMPAT_FUNCS(xattr, EXT_ATTR) EXT4_FEATURE_COMPAT_FUNCS(resize_inode, RESIZE_INODE) EXT4_FEATURE_COMPAT_FUNCS(dir_index, DIR_INDEX) EXT4_FEATURE_COMPAT_FUNCS(sparse_super2, SPARSE_SUPER2) +EXT4_FEATURE_COMPAT_FUNCS(fast_commit, FAST_COMMIT) EXT4_FEATURE_COMPAT_FUNCS(stable_inodes, STABLE_INODES) EXT4_FEATURE_RO_COMPAT_FUNCS(sparse_super, SPARSE_SUPER) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 901c1c938276..70256a240442 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1709,7 +1709,7 @@ enum { Opt_dioread_nolock, Opt_dioread_lock, Opt_discard, Opt_nodiscard, Opt_init_itable, Opt_noinit_itable, Opt_max_dir_size_kb, Opt_nojournal_checksum, Opt_nombcache, - Opt_prefetch_block_bitmaps, + Opt_prefetch_block_bitmaps, Opt_no_fc, }; static const match_table_t tokens = { @@ -1796,6 +1796,7 @@ static const match_table_t tokens = { {Opt_init_itable, "init_itable=%u"}, {Opt_init_itable, "init_itable"}, {Opt_noinit_itable, "noinit_itable"}, + {Opt_no_fc, "no_fc"}, {Opt_max_dir_size_kb, "max_dir_size_kb=%u"}, {Opt_test_dummy_encryption, "test_dummy_encryption=%s"}, {Opt_test_dummy_encryption, "test_dummy_encryption"}, @@ -1922,6 +1923,7 @@ static int clear_qf_name(struct super_block *sb, int qtype) #define MOPT_EXT4_ONLY (MOPT_NO_EXT2 | MOPT_NO_EXT3) #define MOPT_STRING 0x0400 #define MOPT_SKIP 0x0800 +#define MOPT_2 0x1000 static const struct mount_opts { int token; @@ -2022,6 +2024,8 @@ static const struct mount_opts { {Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET}, {Opt_prefetch_block_bitmaps, EXT4_MOUNT_PREFETCH_BLOCK_BITMAPS, MOPT_SET}, + {Opt_no_fc, EXT4_MOUNT2_JOURNAL_FAST_COMMIT, + MOPT_CLEAR | MOPT_2 | MOPT_EXT4_ONLY}, {Opt_err, 0, 0} }; @@ -2398,10 +2402,17 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token, WARN_ON(1); return -1; } - if (arg != 0) - sbi->s_mount_opt |= m->mount_opt; - else - sbi->s_mount_opt &= ~m->mount_opt; + if (m->flags & MOPT_2) { + if (arg != 0) + sbi->s_mount_opt2 |= m->mount_opt; + else + sbi->s_mount_opt2 &= ~m->mount_opt; + } else { + if (arg != 0) + sbi->s_mount_opt |= m->mount_opt; + else + sbi->s_mount_opt &= ~m->mount_opt; + } } return 1; } @@ -2618,6 +2629,9 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb, SEQ_OPTS_PUTS("dax=inode"); } + if (test_opt2(sb, JOURNAL_FAST_COMMIT)) + SEQ_OPTS_PUTS("fast_commit"); + ext4_show_quota_options(seq, sb); return 0; } @@ -4121,6 +4135,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) #ifdef CONFIG_EXT4_FS_POSIX_ACL set_opt(sb, POSIX_ACL); #endif + if (ext4_has_feature_fast_commit(sb)) + set_opt2(sb, JOURNAL_FAST_COMMIT); /* don't forget to enable journal_csum when metadata_csum is enabled. */ if (ext4_has_metadata_csum(sb)) set_opt(sb, JOURNAL_CHECKSUM); @@ -4777,6 +4793,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) sbi->s_def_mount_opt &= ~EXT4_MOUNT_JOURNAL_CHECKSUM; clear_opt(sb, JOURNAL_CHECKSUM); clear_opt(sb, DATA_FLAGS); + clear_opt2(sb, JOURNAL_FAST_COMMIT); sbi->s_journal = NULL; needs_recovery = 0; goto no_journal; diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h index 04afa6dcd60d..0685cc95e501 100644 --- a/include/linux/jbd2.h +++ b/include/linux/jbd2.h @@ -289,6 +289,7 @@ typedef struct journal_superblock_s #define JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT 0x00000004 #define JBD2_FEATURE_INCOMPAT_CSUM_V2 0x00000008 #define JBD2_FEATURE_INCOMPAT_CSUM_V3 0x00000010 +#define JBD2_FEATURE_INCOMPAT_FAST_COMMIT 0x00000020 /* See "journal feature predicate functions" below */ @@ -299,7 +300,8 @@ typedef struct journal_superblock_s JBD2_FEATURE_INCOMPAT_64BIT | \ JBD2_FEATURE_INCOMPAT_ASYNC_COMMIT | \ JBD2_FEATURE_INCOMPAT_CSUM_V2 | \ - JBD2_FEATURE_INCOMPAT_CSUM_V3) + JBD2_FEATURE_INCOMPAT_CSUM_V3 | \ + JBD2_FEATURE_INCOMPAT_FAST_COMMIT) #ifdef __KERNEL__ @@ -1263,6 +1265,7 @@ JBD2_FEATURE_INCOMPAT_FUNCS(64bit, 64BIT) JBD2_FEATURE_INCOMPAT_FUNCS(async_commit, ASYNC_COMMIT) JBD2_FEATURE_INCOMPAT_FUNCS(csum2, CSUM_V2) JBD2_FEATURE_INCOMPAT_FUNCS(csum3, CSUM_V3) +JBD2_FEATURE_INCOMPAT_FUNCS(fast_commit, FAST_COMMIT) /* * Journal flag definitions
We are running out of mount option bits. Add handling for using s_mount_opt2. Add ext4 and jbd2 fast commit feature flag and also add ability to turn off the fast commit feature in Ext4. Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com> --- fs/ext4/ext4.h | 4 ++++ fs/ext4/super.c | 27 ++++++++++++++++++++++----- include/linux/jbd2.h | 5 ++++- 3 files changed, 30 insertions(+), 6 deletions(-)