Re: [PATCH] ceph: use kill_anon_super helper

[Jeff Layton <jlayton@kernel.org>]

On Sun, 2020-09-13 at 14:00 +0200, Ilya Dryomov wrote:
> On Sat, Sep 12, 2020 at 12:14 PM Jeff Layton <jlayton@kernel.org> wrote:
> > ceph open-codes this around some other activity and the rationale
> > for it isn't clear. There is no need to delay free_anon_bdev until
> > the end of kill_sb.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/super.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> > index 7ec0e6d03d10..b3fc9bb61afc 100644
> > --- a/fs/ceph/super.c
> > +++ b/fs/ceph/super.c
> > @@ -1205,14 +1205,13 @@ static int ceph_init_fs_context(struct fs_context *fc)
> >  static void ceph_kill_sb(struct super_block *s)
> >  {
> >         struct ceph_fs_client *fsc = ceph_sb_to_client(s);
> > -       dev_t dev = s->s_dev;
> > 
> >         dout("kill_sb %p\n", s);
> > 
> >         ceph_mdsc_pre_umount(fsc->mdsc);
> >         flush_fs_workqueues(fsc);
> > 
> > -       generic_shutdown_super(s);
> > +       kill_anon_super(s);
> > 
> >         fsc->client->extra_mon_dispatch = NULL;
> >         ceph_fs_debugfs_cleanup(fsc);
> > @@ -1220,7 +1219,6 @@ static void ceph_kill_sb(struct super_block *s)
> >         ceph_fscache_unregister_fs(fsc);
> > 
> >         destroy_fs_client(fsc);
> > -       free_anon_bdev(dev);
> >  }
> > 
> >  static struct file_system_type ceph_fs_type = {
> > --
> > 2.26.2
> > 
> 
> Hi Jeff,
> 
> Just curious, did you attempt to figure out why it used to be
> necessary?  Looks like it goes back to a very old commit 5dfc589a8467
> ("ceph: unregister bdi before kill_anon_super releases device name"),
> but it's not obvious to me at first sight...
> 
> A lot has changed in the bdi area since then, so if not, it probably
> doesn't matter much -- AFAICT bdi would still get unregistered before
> the minor number is released with this patch.
> 

I did see that commit, but the rationale for it wasn't terribly clear,
and like you said the bdi handling has had a lot of changes since then.

I mainly just looked to see whether the activities between
generic_shutdown_super and free_anon_bdev had any bearing on the minor
number and bdi, but it didn't seem to be.
-- 
Jeff Layton <jlayton@kernel.org>