Re: [PATCH v10 2/9] ext4: add fast_commit feature and handling for extended mount options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> ---
>  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
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux