On Wed, Jun 19, 2013 at 05:12:28PM +0800, Li Zhong wrote: > On Wed, 2013-06-19 at 11:31 +0400, Glauber Costa wrote: > > On Tue, Jun 18, 2013 at 05:42:01PM +0800, Li Zhong wrote: > > > On Fri, 2013-06-07 at 00:34 +0400, Glauber Costa wrote: > > > > > > > diff --git a/fs/super.c b/fs/super.c > > > > index 85a6104..1b6ef7b 100644 > > > > --- a/fs/super.c > > > > +++ b/fs/super.c > > > > @@ -199,8 +199,12 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags) > > > > INIT_HLIST_NODE(&s->s_instances); > > > > INIT_HLIST_BL_HEAD(&s->s_anon); > > > > INIT_LIST_HEAD(&s->s_inodes); > > > > - list_lru_init(&s->s_dentry_lru); > > > > - list_lru_init(&s->s_inode_lru); > > > > + > > > > + if (list_lru_init(&s->s_dentry_lru)) > > > > + goto err_out; > > > > + if (list_lru_init(&s->s_inode_lru)) > > > > + goto err_out_dentry_lru; > > > > + > > > > INIT_LIST_HEAD(&s->s_mounts); > > > > init_rwsem(&s->s_umount); > > > > lockdep_set_class(&s->s_umount, &type->s_umount_key); > > > > @@ -240,6 +244,9 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags) > > > > } > > > > out: > > > > return s; > > > > + > > > > +err_out_dentry_lru: > > > > + list_lru_destroy(&s->s_dentry_lru); > > > > err_out: > > > > security_sb_free(s); > > > > #ifdef CONFIG_SMP > > > > > > It seems we also need to call list_lru_destroy() in destroy_super()? > > > like below: > > > > > > ----------- > > > diff --git a/fs/super.c b/fs/super.c > > > index b79e732..06ee3af 100644 > > > --- a/fs/super.c > > > +++ b/fs/super.c > > > @@ -269,6 +269,8 @@ err_out: > > > */ > > > static inline void destroy_super(struct super_block *s) > > > { > > > + list_lru_destroy(&s->s_inode_lru); > > > + list_lru_destroy(&s->s_dentry_lru); > > > #ifdef CONFIG_SMP > > > free_percpu(s->s_files); > > > #endif > > > > Hi > > > > Thanks for taking a look at this. > > > > list_lru_destroy is called by deactivate_lock_super, so we should be fine already. > > Sorry, I'm a little confused... > > I didn't see list_lru_destroy() called in deactivate_locked_super(). > Maybe I missed something? Err... the code in my tree reads: unregister_shrinker(&s->s_shrink); list_lru_destroy(&s->s_dentry_lru); list_lru_destroy(&s->s_inode_lru); put_filesystem(fs); put_super(s); But then I have just checked Andrew's, and it is not there - thank you. Andrew, should I send a patch for you to fold it ? > > But it seems other memory allocated in alloc_super(), are freed in > destroy_super(), e.g. ->s_files, why don't we also free this one here? Because we want this close to unregister_shrinker, it is a more natural location for this. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html