| Message ID | 20230531095742.2480623-4-qi.zheng@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | make unregistration of super_block shrinker more faster | expand |
On Wed, May 31, 2023 at 09:57:37AM +0000, Qi Zheng wrote: > From: Kirill Tkhai <tkhai@ya.ru> > > The patch makes s_dentry_lru and s_inode_lru be destroyed > later from the workqueue. This is preparation to split > unregister_shrinker(super_block::s_shrink) in two stages, > and to call finalize stage from destroy_super_work(). > > Note, that generic filesystem shrinker unregistration > is safe to be split in two stages right after this > patch, since super_cache_count() and super_cache_scan() > have a deal with s_dentry_lru and s_inode_lru only. > > But there are two exceptions: XFS and SHMEM, which > define .nr_cached_objects() and .free_cached_objects() > callbacks. These two do not allow us to do the splitting > right after this patch. They touch fs-specific data, > which is destroyed earlier, than destroy_super_work(). > So, we can't call unregister_shrinker_delayed_finalize() > from destroy_super_work() because of them, and next > patches make preparations to make this possible. > > Signed-off-by: Kirill Tkhai <tkhai@ya.ru> > Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> > --- > fs/super.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/fs/super.c b/fs/super.c > index 8d8d68799b34..2ce4c72720f3 100644 > --- a/fs/super.c > +++ b/fs/super.c > @@ -159,6 +159,11 @@ static void destroy_super_work(struct work_struct *work) > destroy_work); > int i; > > + WARN_ON(list_lru_count(&s->s_dentry_lru)); > + WARN_ON(list_lru_count(&s->s_inode_lru)); > + list_lru_destroy(&s->s_dentry_lru); > + list_lru_destroy(&s->s_inode_lru); > + > for (i = 0; i < SB_FREEZE_LEVELS; i++) > percpu_free_rwsem(&s->s_writers.rw_sem[i]); > kfree(s); > @@ -177,8 +182,6 @@ static void destroy_unused_super(struct super_block *s) > if (!s) > return; > up_write(&s->s_umount); > - list_lru_destroy(&s->s_dentry_lru); > - list_lru_destroy(&s->s_inode_lru); > security_sb_free(s); > put_user_ns(s->s_user_ns); > kfree(s->s_subtype); > @@ -287,8 +290,6 @@ static void __put_super(struct super_block *s) > { > if (!--s->s_count) { > list_del_init(&s->s_list); > - WARN_ON(s->s_dentry_lru.node); > - WARN_ON(s->s_inode_lru.node); Why are you removing the wanrings from here? Regardless of where we tear down the lru lists, they *must* be empty here otherwise we have a memory leak. Hence I don't think these warnings should be moved at all. Cheers, Dave.
diff --git a/fs/super.c b/fs/super.c index 8d8d68799b34..2ce4c72720f3 100644 --- a/fs/super.c +++ b/fs/super.c @@ -159,6 +159,11 @@ static void destroy_super_work(struct work_struct *work) destroy_work); int i; + WARN_ON(list_lru_count(&s->s_dentry_lru)); + WARN_ON(list_lru_count(&s->s_inode_lru)); + list_lru_destroy(&s->s_dentry_lru); + list_lru_destroy(&s->s_inode_lru); + for (i = 0; i < SB_FREEZE_LEVELS; i++) percpu_free_rwsem(&s->s_writers.rw_sem[i]); kfree(s); @@ -177,8 +182,6 @@ static void destroy_unused_super(struct super_block *s) if (!s) return; up_write(&s->s_umount); - list_lru_destroy(&s->s_dentry_lru); - list_lru_destroy(&s->s_inode_lru); security_sb_free(s); put_user_ns(s->s_user_ns); kfree(s->s_subtype); @@ -287,8 +290,6 @@ static void __put_super(struct super_block *s) { if (!--s->s_count) { list_del_init(&s->s_list); - WARN_ON(s->s_dentry_lru.node); - WARN_ON(s->s_inode_lru.node); WARN_ON(!list_empty(&s->s_mounts)); security_sb_free(s); put_user_ns(s->s_user_ns); @@ -330,14 +331,6 @@ void deactivate_locked_super(struct super_block *s) unregister_shrinker(&s->s_shrink); fs->kill_sb(s); - /* - * Since list_lru_destroy() may sleep, we cannot call it from - * put_super(), where we hold the sb_lock. Therefore we destroy - * the lru lists right now. - */ - list_lru_destroy(&s->s_dentry_lru); - list_lru_destroy(&s->s_inode_lru); - put_filesystem(fs); put_super(s); } else {