Re: [PATCH v6 12/12] xfs: switch to use the new mount-api

[Christoph Hellwig <hch@infradead.org>]

On Wed, Oct 16, 2019 at 08:41:48AM +0800, Ian Kent wrote:
> +static const struct fs_parameter_description xfs_fs_parameters = {
> +	.name		= "XFS",
> +	.specs		= xfs_param_specs,
> +};

Well spell xfs in lower case in the file system type, so I think we should
spell it the same here.

Btw, can we keep all the mount code together where most of it already
is at the top of the file?  I know the existing version has some remount
stuff at the bottom, but as that get entirely rewritten we might as well
move it all up.

> +	int			silent = fc->sb_flags & SB_SILENT;

The silent variable is only used once, so we might as well remove it.

> +	struct xfs_mount	*mp = fc->s_fs_info;
> +
> +	/*
> +	 * mp and ctx are stored in the fs_context when it is
> +	 * initialized. mp is transferred to the superblock on
> +	 * a successful mount, but if an error occurs before the
> +	 * transfer we have to free it here.
> +	 */
> +	if (mp) {
> +		xfs_free_names(mp);
> +		kfree(mp);
> +	}

We always pair xfs_free_names with freeing the mount structure.
I think it would be nice to add an xfs_free_mount that does both
as a refactoring at the beginning of the series. 

> +static const struct fs_context_operations xfs_context_ops = {
> +	.parse_param = xfs_parse_param,
> +	.get_tree    = xfs_get_tree,
> +	.reconfigure = xfs_reconfigure,
> +	.free        = xfs_fc_free,
> +};

Should these all get a prefix like xfs_fc_free?  Maybe xfs_fsctx
to be a little bit more descriptive?