Re: [PATCH v2 14/15] xfs: mount-api - switch to new mount-api

[Ian Kent <raven@themaw.net>]

On Wed, 2019-08-28 at 09:29 -0400, Brian Foster wrote:
> On Fri, Aug 23, 2019 at 09:00:32AM +0800, Ian Kent wrote:
> > The infrastructure needed to use the new mount api is now
> > in place, switch over to use it.
> > 
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > ---
> >  fs/xfs/xfs_super.c |   51
> > +++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 49 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index d2a1a62a3edc..fe7acd8ddd48 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -2123,7 +2123,6 @@ static const struct super_operations
> > xfs_super_operations = {
> >  	.freeze_fs		= xfs_fs_freeze,
> >  	.unfreeze_fs		= xfs_fs_unfreeze,
> >  	.statfs			= xfs_fs_statfs,
> > -	.remount_fs		= xfs_fs_remount,
> 
> Not clear why this needs to go away here, or at least why we don't
> remove the function at the same time.
> 
> Indeed.. this patch actually throws a couple warnings:
> 
> ...
>   CC [M]  fs/xfs/xfs_super.o
> fs/xfs/xfs_super.c:2088:1: warning: ‘xfs_fs_mount’ defined but not
> used [-Wunused-function]
>  xfs_fs_mount(
>  ^~~~~~~~~~~~
> fs/xfs/xfs_super.c:1448:1: warning: ‘xfs_fs_remount’ defined but not
> used [-Wunused-function]
>  xfs_fs_remount(
>  ^~~~~~~~~~~~~~
> ...

Yes I saw those too and I was very tempted to remove those
functions at the time.

But I decided the warning was preferable to obsuring what
the change was really doing.

I think now you find this a problem I'll just remove the
functions.

IIRC .remount_fs is removed so it won't be called.

But to be honest I'm not certain that needs to be done.

In any case it really should go away when changing to
use the mount api so I think it needs to be done.

> 
> >  	.show_options		= xfs_fs_show_options,
> >  	.nr_cached_objects	= xfs_fs_nr_cached_objects,
> >  	.free_cached_objects	= xfs_fs_free_cached_objects,
> > @@ -2157,10 +2156,58 @@ static const struct fs_context_operations
> > xfs_context_ops = {
> ...
> >  static struct file_system_type xfs_fs_type = {
> >  	.owner			= THIS_MODULE,
> >  	.name			= "xfs",
> > -	.mount			= xfs_fs_mount,
> > +	.init_fs_context	= xfs_init_fs_context,
> > +	.parameters		= &xfs_fs_parameters,
> 
> Just a random observation.. we have a .name == "xfs" field here and
> the
> parameters struct has a .name == "XFS" field. Perhaps we should be
> consistent?

Yeah, others are seeing similar inconsistencies.

Not sure what I'll do about these yet but it is a problem I think
so I'll need to come up with something sensible.

> 
> Brian
> 
> >  	.kill_sb		= kill_block_super,
> >  	.fs_flags		= FS_REQUIRES_DEV,
> >  };
> >