On Wed, 2013-06-19 at 17:29 +0400, Glauber Costa wrote:

On Wed, Jun 19, 2013 at 05:12:28PM +0800, Li Zhong wrote:

quoted

On Wed, 2013-06-19 at 11:31 +0400, Glauber Costa wrote:

quoted

On Tue, Jun 18, 2013 at 05:42:01PM +0800, Li Zhong wrote:

quoted

On Fri, 2013-06-07 at 00:34 +0400, Glauber Costa wrote:

 > 

quoted

quoted

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 ?

quoted

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.

OK, I see. However, in some rare cases, destroy_super() might be called
in a row with alloc_super(), e.g. in sget(), if !test and err when set()
the fs private info. 

By the way, there is memory allocated in register_shrinker() based on
nr_node_ids, so maybe we need to free it in unregister_shrinker()? Maybe
that's already covered in the deferred part.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>