Re: [PATCH v11 25/25] list_lru: dynamically adjust node arrays

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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:
> > 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.

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 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




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux