Re: [RFC PATCH] fat: add noflush mount option

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

David Timber <dxdt@dev.snart.me> writes:

> On most distros, udisks manages users' removable volume mount requests.
> Udisks is configured to always mount FAT volumes with the 'flush' mount
> option. This is largely okay for populating directory in small sizes,
> but when a large number of files are involved, the flush behaviour acts
> as a bottleneck point in fat_file_release(). The user may want to
> disable the flush option temporarily before commencing such an intensive
> operation.
>
> To cover this use case, introduce the new 'noflush' mount option. When
> used in the mount options to mount a volume, it overrides the 'flush'
> option previously specified. When used in remount, update it updates the
> flag.
>
> This is a breaking change as traditionally, the mount options other than
> rw and ro are ignored. The patch breaks this tradition by allowing
> reconfiguration of 'flush' and 'noflush' mount options.
>
> Signed-off-by: David Timber <dxdt@dev.snart.me>
> ---
>  fs/fat/inode.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> index 28f78df086ef..850df43ba354 100644
> --- a/fs/fat/inode.c
> +++ b/fs/fat/inode.c
> @@ -813,10 +813,14 @@ int fat_reconfigure(struct fs_context *fc)
>  	bool new_rdonly;
>  	struct super_block *sb = fc->root->d_sb;
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
> +	struct fat_mount_options *new_opts = fc->fs_private;
>  	fc->sb_flags |= SB_NODIRATIME | (sbi->options.isvfat ? 0 : SB_NOATIME);
>  
>  	sync_filesystem(sb);
>  
> +	/* allow reconfiguring "flush" or "noflush" */
> +	sbi->options.flush = new_opts->flush;
> +
>  	/* make sure we update state on remount. */
>  	new_rdonly = fc->sb_flags & SB_RDONLY;
>  	if (new_rdonly != sb_rdonly(sb)) {

Maybe, better to set after changed the read-only?

> @@ -1047,7 +1051,7 @@ enum {
>  	Opt_charset, Opt_shortname, Opt_utf8, Opt_utf8_bool,
>  	Opt_uni_xl, Opt_uni_xl_bool, Opt_nonumtail, Opt_nonumtail_bool,
>  	Opt_obsolete, Opt_flush, Opt_tz, Opt_rodir, Opt_errors, Opt_discard,
> -	Opt_nfs, Opt_nfs_enum, Opt_time_offset, Opt_dos1xfloppy,
> +	Opt_nfs, Opt_nfs_enum, Opt_time_offset, Opt_dos1xfloppy, Opt_noflush
>  };
>  
>  static const struct constant_table fat_param_check[] = {
> @@ -1110,6 +1114,7 @@ const struct fs_parameter_spec fat_param_spec[] = {
>  	fsparam_flag	("debug",	Opt_debug),
>  	fsparam_flag	("sys_immutable", Opt_immutable),
>  	fsparam_flag	("flush",	Opt_flush),
> +	fsparam_flag	("noflush",	Opt_noflush),

fsparam_flag() is not including the "noflush" too?

>  	fsparam_enum	("tz",		Opt_tz, fat_param_tz),
>  	fsparam_s32	("time_offset",	Opt_time_offset),
>  	fsparam_enum	("errors",	Opt_errors, fat_param_errors),
> @@ -1167,10 +1172,6 @@ int fat_parse_param(struct fs_context *fc, struct fs_parameter *param,
>  	struct fs_parse_result result;
>  	int opt;
>  
> -	/* remount options have traditionally been ignored */
> -	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
> -		return 0;
> -
>  	opt = fs_parse(fc, fat_param_spec, param, &result);
>  	/* If option not found in fat_param_spec, try vfat/msdos options */
>  	if (opt == -ENOPARAM) {
> @@ -1183,6 +1184,18 @@ int fat_parse_param(struct fs_context *fc, struct fs_parameter *param,
>  	if (opt < 0)
>  		return opt;
>  
> +	/* remount options have traditionally been ignored */
> +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> +		switch (opt) {
> +		/* but there are exceptions */
> +		case Opt_flush:
> +		case Opt_noflush:
> +			break;
> +		default:
> +			return 0;
> +		}
> +	}
> +
>  	switch (opt) {
>  	case Opt_check:
>  		opts->name_check = result.uint_32;
> @@ -1235,6 +1248,9 @@ int fat_parse_param(struct fs_context *fc, struct fs_parameter *param,
>  	case Opt_flush:
>  		opts->flush = 1;
>  		break;
> +	case Opt_noflush:
> +		opts->flush = 0;
> +		break;
>  	case Opt_time_offset:
>  		/*
>  		 * GMT+-12 zones may have DST corrections so at least

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>