On Tue, Sep 24, 2019 at 09:22:49PM +0800, Ian Kent wrote:
> Add the fs_context_operations method .get_tree that validates
> mount options and fills the super block as previously done
> by the file_system_type .mount method.
>
> Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
> ---
> fs/xfs/xfs_super.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ea3640ffd8f5..6f9fe92b4e21 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1933,6 +1933,51 @@ xfs_fs_fill_super(
> return error;
> }
>
> +STATIC int
> +xfs_fill_super(
> + struct super_block *sb,
> + struct fs_context *fc)
> +{
> + struct xfs_fs_context *ctx = fc->fs_private;
> + struct xfs_mount *mp = sb->s_fs_info;
> + int silent = fc->sb_flags & SB_SILENT;
> + int error = -ENOMEM;
> +
> + mp->m_super = sb;
> +
> + /*
> + * set up the mount name first so all the errors will refer to the
> + * correct device.
> + */
> + mp->m_fsname = kstrndup(sb->s_id, MAXNAMELEN, GFP_KERNEL);
> + if (!mp->m_fsname)
> + return -ENOMEM;
> + mp->m_fsname_len = strlen(mp->m_fsname) + 1;
> +
> + error = xfs_validate_params(mp, ctx, false);
> + if (error)
> + goto out_free_fsname;
> +
> + error = __xfs_fs_fill_super(mp, silent);
> + if (error)
> + goto out_free_fsname;
> +
> + return 0;
> +
> + out_free_fsname:
> + sb->s_fs_info = NULL;
> + xfs_free_fsname(mp);
> +
I'm still not following the (intended) lifecycle of mp here. Looking
ahead in the series, we allocate mp in xfs_init_fs_context() and set
some state. It looks like at some point we grow an xfs_fc_free()
callback that frees mp, but that doesn't exist as of yet. So is that a
memory leak as of this patch?
We also call xfs_free_fsname() here (which doesn't reset pointers to
NULL) and open-code kfree()'s of a couple of the same fields in
xfs_fc_free(). Those look like double frees to me.
Hmm.. I guess I'm kind of wondering why we lift the mp alloc out of the
fill super call in the first place. At a glance, it doesn't look like we
do anything in that xfs_init_fs_context() call that we couldn't do a bit
later..
Brian
> + return error;
> +}
> +
> +STATIC int
> +xfs_get_tree(
> + struct fs_context *fc)
> +{
> + return vfs_get_block_super(fc, xfs_fill_super);
> +}
> +
> STATIC void
> xfs_fs_put_super(
> struct super_block *sb)
> @@ -2003,6 +2048,11 @@ static const struct super_operations xfs_super_operations = {
> .free_cached_objects = xfs_fs_free_cached_objects,
> };
>
> +static const struct fs_context_operations xfs_context_ops = {
> + .parse_param = xfs_parse_param,
> + .get_tree = xfs_get_tree,
> +};
> +
> static struct file_system_type xfs_fs_type = {
> .owner = THIS_MODULE,
> .name = "xfs",
>
