CEPH filesystem development
 help / color / mirror / Atom feed
* [PATCH] ceph: use kill_anon_super helper
@ 2020-09-12 10:14 Jeff Layton
  2020-09-13 12:00 ` Ilya Dryomov
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2020-09-12 10:14 UTC (permalink / raw)
  To: ceph-devel; +Cc: idryomov

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ceph: use kill_anon_super helper
  2020-09-12 10:14 [PATCH] ceph: use kill_anon_super helper Jeff Layton
@ 2020-09-13 12:00 ` Ilya Dryomov
  2020-09-13 12:41   ` Jeff Layton
  0 siblings, 1 reply; 3+ messages in thread
From: Ilya Dryomov @ 2020-09-13 12:00 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Ceph Development

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.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ceph: use kill_anon_super helper
  2020-09-13 12:00 ` Ilya Dryomov
@ 2020-09-13 12:41   ` Jeff Layton
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Layton @ 2020-09-13 12:41 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: Ceph Development

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>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-09-13 12:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-12 10:14 [PATCH] ceph: use kill_anon_super helper Jeff Layton
2020-09-13 12:00 ` Ilya Dryomov
2020-09-13 12:41   ` Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox