Re: [patch 0/5] [RFC] vfs: per-superblock unused dentries list

Re: [patch 0/5] [RFC] vfs: per-superblock unused dentries list

[David Chinner]

On Fri, May 26, 2006 at 01:06:55PM +0200, Jan Blunck wrote:
> BTW: Dave, I just saw that you already posted some patches but I decided to
> send my patches anyway.

No problems. Your patches remove bits that I wasn't sure could be removed,
but effectively do the same thing. Removing shrink_dcache_anon(), for example.
The question is which patchset do we want to carry forward?

I'll add more comments in replies to the various patches...

Cheers,

Dave.
-- 
Dave Chinner
R&D Software Enginner
SGI Australian Software Group

Re: [patch 4/5] vfs: per superblock dentry stats

[David Chinner]

On Fri, May 26, 2006 at 01:06:59PM +0200, Jan Blunck wrote:
> Index: work-2.6/fs/super.c
> ===================================================================
> --- work-2.6.orig/fs/super.c
> +++ work-2.6/fs/super.c
> @@ -71,6 +71,9 @@ static struct super_block *alloc_super(v
>  		INIT_LIST_HEAD(&s->s_instances);
>  		INIT_HLIST_HEAD(&s->s_anon);
>  		INIT_LIST_HEAD(&s->s_inodes);
> +		s->s_dentry_stat.nr_dentry = 0;
> +		s->s_dentry_stat.nr_unused = 0;

No need for these as the superblock is memset to zero....

> -struct dentry_stat_t {
> +struct dentry_stat {
>  	int nr_dentry;
>  	int nr_unused;
> -	int age_limit;          /* age in seconds */
> -	int want_pages;         /* pages requested by system */
> -	int dummy[2];
> +	int age_limit;		/* age in seconds */
>  };

that changes the size of the structure from 6*sizeof(int) to
3*sizeof(int)....

> -extern struct dentry_stat_t dentry_stat;
> +extern struct dentry_stat global_dentry_stat;
> +
> +#define dentry_stat_inc(sb, x)		\
> +do {					\
> +	global_dentry_stat.x++;		\
> +	if (sb)				\
> +		(sb)->s_dentry_stat.x++;\
> +} while(0)
> +
> +#define dentry_stat_dec(sb, x)		\
> +do {					\
> +	global_dentry_stat.x--;		\
> +	if (sb)				\
> +		(sb)->s_dentry_stat.x--;\
> +} while(0)

	if (likely(sb))  ???

> Index: work-2.6/kernel/sysctl.c
> ===================================================================
> --- work-2.6.orig/kernel/sysctl.c
> +++ work-2.6/kernel/sysctl.c
> @@ -958,7 +958,7 @@ static ctl_table fs_table[] = {
>  	{
>  		.ctl_name	= FS_DENTRY,
>  		.procname	= "dentry-state",
> -		.data		= &dentry_stat,
> +		.data		= &global_dentry_stat,
>  		.maxlen		= 6*sizeof(int),

With the above change, maxlen = 3*sizeof(int).

Given that's a userspace visible change, should the above structure
change use a "int dummy[3];" to keep the global structure and userspace
export the same?

Cheers,

Dave.
-- 
Dave Chinner
R&D Software Enginner
SGI Australian Software Group

Re: [patch 5/5] vfs: per superblock dentry unused list

[David Chinner]

On Fri, May 26, 2006 at 01:07:00PM +0200, Jan Blunck wrote:
> This patch adds per superblock dentry unused lists. This should speedup the
> umounting/remounting of filesystems when there are alot of dentries in use.
> 
> Signed-off-by: Jan Blunck <jblunck@suse.de>
> ---
>  fs/dcache.c        |  143 +++++++++++++++++++++--------------------------------
>  fs/super.c         |    1 
>  include/linux/fs.h |    1 
>  3 files changed, 61 insertions(+), 84 deletions(-)
> 
> Index: work-2.6/fs/dcache.c
> ===================================================================
> --- work-2.6.orig/fs/dcache.c
> +++ work-2.6/fs/dcache.c
> @@ -61,7 +61,6 @@ static kmem_cache_t *dentry_cache __read
>  static unsigned int d_hash_mask __read_mostly;
>  static unsigned int d_hash_shift __read_mostly;
>  static struct hlist_head *dentry_hashtable __read_mostly;
> -static LIST_HEAD(dentry_unused);
>  
>  /* Statistics gathering. */
>  struct dentry_stat global_dentry_stat = {
> @@ -167,12 +166,15 @@ repeat:
>  		if (dentry->d_op->d_delete(dentry))
>  			goto unhash_it;
>  	}
> +	/* Kill dentry without superblock */
> +	if (unlikely(!dentry->d_sb))
> +		goto unhash_it;
>  	/* Unreachable? Get rid of it */
>  	if (d_unhashed(dentry))
>  		goto kill_it;
>  	if (list_empty(&dentry->d_lru)) {
>  		dentry->d_flags |= DCACHE_REFERENCED;
> -		list_add(&dentry->d_lru, &dentry_unused);
> +		list_add(&dentry->d_lru, &dentry->d_sb->s_unused);
>  		dentry_stat_inc(dentry->d_sb, nr_unused);
>  	}
>  	spin_unlock(&dentry->d_lock);
> @@ -382,19 +384,22 @@ static inline void prune_one_dentry(stru
>  }
>  
>  /**
> - * prune_dcache - shrink the dcache
> + * prune_dcache_sb - prune the dcache for a superblock
> + * @sb: superblock
>   * @count: number of entries to try and free
>   *
> - * Shrink the dcache. This is done when we need
> - * more memory. When we need to unmount something
> - * we call shrink_dcache_sb().
> + * Prune the dcache for the specified super block. This
> + * is called from prune_dcache().
>   *
> - * This function may fail to free any resources if
> - * all the dentries are in use.
> + * You need to have a reference to the super block and
> + * should have sb->s_umount locked. This function may fail
> + * to free any resources if all the dentries are in use.
>   */
> -
> -static void prune_dcache(int count)
> +static void prune_dcache_sb(struct super_block *sb, int count)
>  {
> +	if (count <= 0)
> +		return;
> +
>  	spin_lock(&dcache_lock);
>  	for (; count ; count--) {
>  		struct dentry *dentry;
> @@ -402,10 +407,10 @@ static void prune_dcache(int count)
>  
>  		cond_resched_lock(&dcache_lock);
>  
> -		tmp = dentry_unused.prev;
> -		if (tmp == &dentry_unused)
> +		tmp = sb->s_unused.prev;
> +		if (tmp == &sb->s_unused)
>  			break;
> -		prefetch(dentry_unused.prev);
> +		prefetch(sb->s_unused.prev);
>  		dentry = list_entry(tmp, struct dentry, d_lru);
>  		dentry_stat_dec(dentry->d_sb, nr_unused);
>  		list_del_init(&dentry->d_lru);
> @@ -423,7 +428,7 @@ static void prune_dcache(int count)
>  		/* If the dentry was recently referenced, don't free it. */
>  		if (dentry->d_flags & DCACHE_REFERENCED) {
>  			dentry->d_flags &= ~DCACHE_REFERENCED;
> -			list_add(&dentry->d_lru, &dentry_unused);
> +			list_add(&dentry->d_lru, &sb->s_unused);
>  			dentry_stat_inc(dentry->d_sb, nr_unused);
>  			spin_unlock(&dentry->d_lock);
>  			continue;
> @@ -433,64 +438,48 @@ static void prune_dcache(int count)
>  	spin_unlock(&dcache_lock);
>  }
>  
> -
> -/*
> - * parsing d_hash list does not hlist_for_each_entry_rcu() as it
> - * done under dcache_lock.
> +/**
> + * prune_dcache - shrink the dcache
> + * @count: number of entries to try and free
> + *
> + * Prune the dcache. This is done when we need
> + * more memory.
> + *
> + * This function may fail to free any resources if
> + * all the dentries are in use.
>   */
> -static void select_anon(struct super_block *sb)
> +static void prune_dcache(int count)
>  {
> -	struct dentry *dentry;
> -	struct hlist_node *lp;
> -
> -	spin_lock(&dcache_lock);
> -	hlist_for_each_entry(dentry, lp, &sb->s_anon, d_hash) {
> -		if (!list_empty(&dentry->d_lru)) {
> -			dentry_stat_dec(sb, nr_unused);
> -			list_del_init(&dentry->d_lru);
> -		}
> +	struct super_block *sb;
> +	int unused = global_dentry_stat.nr_unused;
>  
> -		/*
> -		 * move only zero ref count dentries to the beginning
> -		 * (the most recent end) of the unused list
> -		 */
> -		spin_lock(&dentry->d_lock);
> -		if (!atomic_read(&dentry->d_count)) {
> -			list_add(&dentry->d_lru, &dentry_unused);
> -			dentry_stat_inc(sb, nr_unused);
> -		}
> -		spin_unlock(&dentry->d_lock);
> -	}
> -	spin_unlock(&dcache_lock);
> -}
> +	if (count <= 0)
> +		return;
>  
> -static void select_sb(struct super_block *sb)
> -{
> -	struct dentry *dentry, *pos;
> +	spin_lock(&sb_lock);
> + restart:
> +	list_for_each_entry(sb, &super_blocks, s_list) {
> +		sb->s_count++;
> +		spin_unlock(&sb_lock);
> +		down_read(&sb->s_umount);

This should probably be a try-lock. See the comment in
writeback_inodes()....

> +		if (sb->s_root) {
> +			int tmp;
>  
> -	spin_lock(&dcache_lock);
> -	list_for_each_entry_safe(dentry, pos, &dentry_unused, d_lru) {
> -		if (dentry->d_sb != sb)
> -			continue;
> -		list_del(&dentry->d_lru);
> -		list_add(&dentry->d_lru, &dentry_unused);
> +			/*
> +			 * Try to be fair to the unused lists:
> +			 *  sb_count/sb_unused ~ global_count/global_unused
> +			 */
> +			tmp = sb->s_dentry_stat.nr_unused/((unused/count)+1);
> +			prune_dcache_sb(sb, tmp);

So if count = SHRINK_BATCH = 128, unused is 12800 (for easy maths) and we have
100 unused on the first superbloc, we end up with tmp = 100 / ((12800/128)+1)
= 100/101 = 0.

Essentially, if your superblock has less than (global_unused / count) dentries
on it, they'll never get shrunk. They need to take at least one dentry off
each superblock to ensure that the lru lists are slowly turned over. This is
needed to allow pages in the slab pinned by dentries on lesser used or
smaller filesystems to be freed before you've trimmed almost every dentry
from the superblocks that contain orders of magnitude more dentries...

IOWs, I think that tmp must be >= 1 for all calls here. 

Realistically, we are limited in resolution by the way the shrinker works
here. When we have a difference of greater than 2 orders of magnitude between
the small superblock and the large superblock lists we are either going to
trim the small superblock lists too much or not enough....

> @@ -499,30 +488,16 @@ static void select_sb(struct super_block
>   * is used to free the dcache before unmounting a file
>   * system
>   */
> -
>  void shrink_dcache_sb(struct super_block * sb)
>  {

The only difference between this function and prune_dcache_sb
is the handlingof the DCACHE_REFERENCED bit. i built a common
function for these, because....

> @@ -671,7 +646,7 @@ void shrink_dcache_parent(struct dentry 
>  	int found;
>  
>  	while ((found = select_parent(parent)) != 0)
> -		prune_dcache(found);
> +		prune_dcache_sb(parent->d_sb, found);
>  }

... prune_dcache_parent() uses the same code as well....

> Index: work-2.6/include/linux/fs.h
> ===================================================================
> --- work-2.6.orig/include/linux/fs.h
> +++ work-2.6/include/linux/fs.h
> @@ -846,6 +846,7 @@ struct super_block {
>  	struct list_head	s_io;		/* parked for writeback */
>  	struct hlist_head	s_anon;		/* anonymous dentries for (nfs) exporting */
>  	struct list_head	s_files;
> +	struct list_head	s_unused;	/* unused dentries */

Maybe change that name to something that has "dentry" in the name?

>  	struct dentry_stat	s_dentry_stat;

... especially as the matching stats structure uses that convention...

Cheers,

Dave.
-- 
Dave Chinner
R&D Software Enginner
SGI Australian Software Group

Re: [patch 4/5] vfs: per superblock dentry stats

[Jan Blunck]

On Mon, May 29, David Chinner wrote:

> > -struct dentry_stat_t {
> > +struct dentry_stat {
> >  	int nr_dentry;
> >  	int nr_unused;
> > -	int age_limit;          /* age in seconds */
> > -	int want_pages;         /* pages requested by system */
> > -	int dummy[2];
> > +	int age_limit;		/* age in seconds */
> >  };
> 
> that changes the size of the structure from 6*sizeof(int) to
> 3*sizeof(int)....
> 
> > Index: work-2.6/kernel/sysctl.c
> > ===================================================================
> > --- work-2.6.orig/kernel/sysctl.c
> > +++ work-2.6/kernel/sysctl.c
> > @@ -958,7 +958,7 @@ static ctl_table fs_table[] = {
> >  	{
> >  		.ctl_name	= FS_DENTRY,
> >  		.procname	= "dentry-state",
> > -		.data		= &dentry_stat,
> > +		.data		= &global_dentry_stat,
> >  		.maxlen		= 6*sizeof(int),
> 
> With the above change, maxlen = 3*sizeof(int).
> 
> Given that's a userspace visible change, should the above structure
> change use a "int dummy[3];" to keep the global structure and userspace
> export the same?

Hmm, probably you are right. I'll change this.

Jan

Re: [patch 5/5] vfs: per superblock dentry unused list

[Jan Blunck]

On Mon, May 29, David Chinner wrote:

> > -	spin_lock(&dcache_lock);
> > -	list_for_each_entry_safe(dentry, pos, &dentry_unused, d_lru) {
> > -		if (dentry->d_sb != sb)
> > -			continue;
> > -		list_del(&dentry->d_lru);
> > -		list_add(&dentry->d_lru, &dentry_unused);
> > +			/*
> > +			 * Try to be fair to the unused lists:
> > +			 *  sb_count/sb_unused ~ global_count/global_unused
> > +			 */
> > +			tmp = sb->s_dentry_stat.nr_unused/((unused/count)+1);
> > +			prune_dcache_sb(sb, tmp);
> 
> So if count = SHRINK_BATCH = 128, unused is 12800 (for easy maths) and we have
> 100 unused on the first superbloc, we end up with tmp = 100 / ((12800/128)+1)
> = 100/101 = 0.
> 
> Essentially, if your superblock has less than (global_unused / count) dentries
> on it, they'll never get shrunk. They need to take at least one dentry off
> each superblock to ensure that the lru lists are slowly turned over. This is
> needed to allow pages in the slab pinned by dentries on lesser used or
> smaller filesystems to be freed before you've trimmed almost every dentry
> from the superblocks that contain orders of magnitude more dentries...
> 
> IOWs, I think that tmp must be >= 1 for all calls here. 
> 
> Realistically, we are limited in resolution by the way the shrinker works
> here. When we have a difference of greater than 2 orders of magnitude between
> the small superblock and the large superblock lists we are either going to
> trim the small superblock lists too much or not enough....

Yeah, I have problems with that part as well. Some of your assumtions are
wrong. If the sb.nr_unused count is smaller than 128, the superblock is not
shrinked, thats true. But there is a superblock with more than 128 unused
dentries (since the global_unused count was 12800). So the prune_dcache() is
shrinking that one first. After a few runs, prune_dcache() is shrinking the
superblock with 128 unused dentries aswell.

Although, what happens when we have 100 superblocks with 128 unused dentries
each ... I have to think about this. The right solution would be to shrink the
dentries with the help of their age. But at the moment I don't have any bright
ideas in that direction.

> > @@ -499,30 +488,16 @@ static void select_sb(struct super_block
> >   * is used to free the dcache before unmounting a file
> >   * system
> >   */
> > -
> >  void shrink_dcache_sb(struct super_block * sb)
> >  {
> 
> The only difference between this function and prune_dcache_sb
> is the handlingof the DCACHE_REFERENCED bit. i built a common
> function for these, because....
> 
> > @@ -671,7 +646,7 @@ void shrink_dcache_parent(struct dentry 
> >  	int found;
> >  
> >  	while ((found = select_parent(parent)) != 0)
> > -		prune_dcache(found);
> > +		prune_dcache_sb(parent->d_sb, found);
> >  }
> 
> ... prune_dcache_parent() uses the same code as well....

No. prune_dcache() is working on the unused list in the opposite (reverse)
direction. shrink_dcache_sb() (basically my prune_dcache_sb()) is shrinking
all unused dentries. In that case it is better to visit the unused list in the
normal (forward) direction (~only one pass).

Jan

Re: [patch 5/5] vfs: per superblock dentry unused list

[David Chinner]

On Mon, May 29, 2006 at 01:54:43PM +0200, Jan Blunck wrote:
> On Mon, May 29, David Chinner wrote:
> 
> > > -	spin_lock(&dcache_lock);
> > > -	list_for_each_entry_safe(dentry, pos, &dentry_unused, d_lru) {
> > > -		if (dentry->d_sb != sb)
> > > -			continue;
> > > -		list_del(&dentry->d_lru);
> > > -		list_add(&dentry->d_lru, &dentry_unused);
> > > +			/*
> > > +			 * Try to be fair to the unused lists:
> > > +			 *  sb_count/sb_unused ~ global_count/global_unused
> > > +			 */
> > > +			tmp = sb->s_dentry_stat.nr_unused/((unused/count)+1);
> > > +			prune_dcache_sb(sb, tmp);
> > 
> > So if count = SHRINK_BATCH = 128, unused is 12800 (for easy maths) and we have
> > 100 unused on the first superbloc, we end up with tmp = 100 / ((12800/128)+1)
> > = 100/101 = 0.
> > 
> > Essentially, if your superblock has less than (global_unused / count) dentries
> > on it, they'll never get shrunk. They need to take at least one dentry off
> > each superblock to ensure that the lru lists are slowly turned over. This is
> > needed to allow pages in the slab pinned by dentries on lesser used or
> > smaller filesystems to be freed before you've trimmed almost every dentry
> > from the superblocks that contain orders of magnitude more dentries...
> > 
> > IOWs, I think that tmp must be >= 1 for all calls here. 
> > 
> > Realistically, we are limited in resolution by the way the shrinker works
> > here. When we have a difference of greater than 2 orders of magnitude between
> > the small superblock and the large superblock lists we are either going to
> > trim the small superblock lists too much or not enough....
> 
> Yeah, I have problems with that part as well. Some of your assumtions are
> wrong. If the sb.nr_unused count is smaller than 128, the superblock is not
> shrinked, thats true. But there is a superblock with more than 128 unused
> dentries (since the global_unused count was 12800). So the prune_dcache() is
> shrinking that one first. After a few runs, prune_dcache() is shrinking the
> superblock with 128 unused dentries aswell.

You've just described the embodiment of the two order's of magnitude
issue I mentioned. That's not a wrong assumption - think of the
above case with global_unused count now being 1.28*10^7 instead of
1.28x10^4. How many dentries do you have to free before freeing any
on the small superblock if we don't free one per call? (quick
answer: 99.9%).

If we shrink one per call, we've freed all 128 dentries while there
is still 1*10^5 dentries on the large list. That seems like a much
better balance to make within the constraints of the shrinker
resolution we have to work with.

FWIW, if we don't free a dentry per sb per prune_dcache call, any
prooblems caused by slab page pinning and fragmetnation get worse
than they are now as some dentries will take far, far longer
to be freed than others regardless of their age.

> Although, what happens when we have 100 superblocks with 128 unused dentries
> each ... I have to think about this. The right solution would be to shrink the
> dentries with the help of their age. But at the moment I don't have any bright
> ideas in that direction.

Hmm - need to do something with that age_limit field, right? That
would imply we need a timestamp in the dentry as well, and we don't
shrink any sb that doesn't have dentries older than the age limit.
If we scan all the sb's and still have more to free, then we halve
the age limit and scan again....

> > > @@ -499,30 +488,16 @@ static void select_sb(struct super_block
> > >   * is used to free the dcache before unmounting a file
> > >   * system
> > >   */
> > > -
> > >  void shrink_dcache_sb(struct super_block * sb)
> > >  {
> > 
> > The only difference between this function and prune_dcache_sb
> > is the handlingof the DCACHE_REFERENCED bit. i built a common
> > function for these, because....
> > 
> > > @@ -671,7 +646,7 @@ void shrink_dcache_parent(struct dentry 
> > >  	int found;
> > >  
> > >  	while ((found = select_parent(parent)) != 0)
> > > -		prune_dcache(found);
> > > +		prune_dcache_sb(parent->d_sb, found);
> > >  }
> > 
> > ... prune_dcache_parent() uses the same code as well....
> 
> No. prune_dcache() is working on the unused list in the opposite (reverse)
> direction. shrink_dcache_sb() (basically my prune_dcache_sb()) is shrinking
> all unused dentries. In that case it is better to visit the unused list in the
> normal (forward) direction (~only one pass).

Why? Forward or reverse it's only one traversal to free all dentries
- you go till the list is empty. Either way, with the prefetch of
the next entry in the list there's little perfomrance difference
once you've got outside some tiny subset of the list that might be
hot in cache....

Cheers,

Dave.
-- 
Dave Chinner
R&D Software Enginner
SGI Australian Software Group

Re: [patch 5/5] vfs: per superblock dentry unused list

[Jan Blunck]

On Tue, May 30, David Chinner wrote:

> You've just described the embodiment of the two order's of magnitude
> issue I mentioned. That's not a wrong assumption - think of the
> above case with global_unused count now being 1.28*10^7 instead of
> 1.28x10^4. How many dentries do you have to free before freeing any
> on the small superblock if we don't free one per call? (quick
> answer: 99.9%).
> 
> If we shrink one per call, we've freed all 128 dentries while there
> is still 1*10^5 dentries on the large list. That seems like a much
> better balance to make within the constraints of the shrinker
> resolution we have to work with.

With the effect that the dcache is completely useless for small filesystems
as long as there is one big one. Filesystems where regularily a small amount
of files is used don't have any cached dentries but the filesystem where
someone touched every file still has a lot of dentries in cache although they
are never used again.

> Hmm - need to do something with that age_limit field, right? That
> would imply we need a timestamp in the dentry as well, and we don't
> shrink any sb that doesn't have dentries older than the age limit.
> If we scan all the sb's and still have more to free, then we halve
> the age limit and scan again....

This probably is the way to go.

> > No. prune_dcache() is working on the unused list in the opposite (reverse)
> > direction. shrink_dcache_sb() (basically my prune_dcache_sb()) is shrinking
> > all unused dentries. In that case it is better to visit the unused list in the
> > normal (forward) direction (~only one pass).
> 
> Why? Forward or reverse it's only one traversal to free all dentries
> - you go till the list is empty. Either way, with the prefetch of
> the next entry in the list there's little perfomrance difference
> once you've got outside some tiny subset of the list that might be
> hot in cache....

Ooops, I was still thinking of the global-unused-list here.

Re: [patch 5/5] vfs: per superblock dentry unused list

[David Chinner]

On Tue, May 30, 2006 at 12:06:33PM +0200, Jan Blunck wrote:
> On Tue, May 30, David Chinner wrote:
> 
> > You've just described the embodiment of the two order's of magnitude
> > issue I mentioned. That's not a wrong assumption - think of the
> > above case with global_unused count now being 1.28*10^7 instead of
> > 1.28x10^4. How many dentries do you have to free before freeing any
> > on the small superblock if we don't free one per call? (quick
> > answer: 99.9%).
> > 
> > If we shrink one per call, we've freed all 128 dentries while there
> > is still 1*10^5 dentries on the large list. That seems like a much
> > better balance to make within the constraints of the shrinker
> > resolution we have to work with.
> 
> With the effect that the dcache is completely useless for small filesystems
> as long as there is one big one.

Not necessarily. I think that as long as the small filesystem is not
being used, then we _should_ be reclaiming slowly from it,
regardless of how big the other filesystems are.  That's the way the
global list ends up working now as the dentries for the small
filesystem get purged according to LRU.

> Filesystems where regularily a small amount
> of files is used don't have any cached dentries but the filesystem where
> someone touched every file still has a lot of dentries in cache although they
> are never used again.

Or alternatively small filesystems with no activity and dentries
that will never get used again never get trimmed while the large
fielsytem with lots of activity gets trimmed. This can lead to
thousands of pages being pinned in slabs that we don't try to
free up until we've already free >90% of the overall caches.
That's not very appealing, IMO.

So I suggest the question is this - how do you define "not being
used"? I guess dentry age or "last dentry was added to list at time
X" (recorded in dput()) would be one way of determining this.

Say something like:

dput():
	list_add(&dentry->d_lru, &dentry->d_sb->s_unused);
	dentry->s_sb->s_unused_age = jiffies +
				(&dentry->d_sb->s_dentry_stat.age_limit * HZ);

prune_dcache():

	tmp = sb->s_dentry_stat.nr_unused/((unused/count)+1);
	if ((tmp == 0) && time_after(jiffies, sb->s_unused_age))
		tmp = 1;

That would turn over small unused dentry lists that have not been
modified for age_limit seconds. That means that small caches that
are being used are not reclaimed prematurely, and those small
caches would also be reclaimed after some time if they are not
being used. That seems to address both our concerns.....

Thoughts?

Cheers,

Dave.
-- 
Dave Chinner
R&D Software Enginner
SGI Australian Software Group

[PATCH 5/5] vfs: per superblock dentry unused list

[jblunck]

[-- Attachment #1: patches.jbl/vfs-per-sb-dentry_unused.diff --]
[-- Type: text/plain, Size: 10339 bytes --]

This patch adds per superblock dentry unused lists. This should speedup the
umounting/remounting of filesystems when there are a lot of dentries on the
unused lists.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 fs/dcache.c        |  170 +++++++++++++++++++++++++----------------------------
 fs/super.c         |    2 
 include/linux/fs.h |    2 
 3 files changed, 86 insertions(+), 88 deletions(-)

Index: work-2.6/fs/dcache.c
===================================================================
--- work-2.6.orig/fs/dcache.c
+++ work-2.6/fs/dcache.c
@@ -61,7 +61,6 @@ static kmem_cache_t *dentry_cache __read
 static unsigned int d_hash_mask __read_mostly;
 static unsigned int d_hash_shift __read_mostly;
 static struct hlist_head *dentry_hashtable __read_mostly;
-static LIST_HEAD(dentry_unused);
 
 /* Statistics gathering. */
 struct dentry_stat global_dentry_stat = {
@@ -167,12 +166,15 @@ repeat:
 		if (dentry->d_op->d_delete(dentry))
 			goto unhash_it;
 	}
+	/* Kill dentry without superblock */
+	if (unlikely(!dentry->d_sb))
+		goto unhash_it;
 	/* Unreachable? Get rid of it */
 	if (d_unhashed(dentry))
 		goto kill_it;
 	if (list_empty(&dentry->d_lru)) {
 		dentry->d_flags |= DCACHE_REFERENCED;
-		list_add(&dentry->d_lru, &dentry_unused);
+		list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_unused);
 		dentry_stat_inc(dentry->d_sb, nr_unused);
 	}
 	spin_unlock(&dentry->d_lock);
@@ -382,18 +384,20 @@ static inline void prune_one_dentry(stru
 }
 
 /**
- * prune_dcache - shrink the dcache
+ * prune_dcache_sb - prune the dcache for a superblock
+ * @sb: superblock
  * @count: number of entries to try and free
  *
- * Shrink the dcache. This is done when we need
- * more memory. When we need to unmount something
- * we call shrink_dcache_sb().
- *
- * This function may fail to free any resources if
- * all the dentries are in use.
+ * Prune the dcache for the specified super block. This walks the dentry LRU
+ * list backwards to free the @count oldest entries on it. If it finds entries
+ * which were recently referenced (the DCACHE_REFERENCED d_flag is set) they
+ * moved to the beginning of the dentry LRU list instead.
+ *
+ * You need to have a reference to the super block and should have
+ * sb->s_umount locked. This function may fail to free any resources if all
+ * the dentries are in use.
  */
-
-static void prune_dcache(int count)
+static void prune_dcache_sb(struct super_block *sb, int count)
 {
 	spin_lock(&dcache_lock);
 	for (; count ; count--) {
@@ -402,10 +406,10 @@ static void prune_dcache(int count)
 
 		cond_resched_lock(&dcache_lock);
 
-		tmp = dentry_unused.prev;
-		if (tmp == &dentry_unused)
+		tmp = sb->s_dentry_unused.prev;
+		if (tmp == &sb->s_dentry_unused)
 			break;
-		prefetch(dentry_unused.prev);
+		prefetch(sb->s_dentry_unused.prev);
 		dentry = list_entry(tmp, struct dentry, d_lru);
 		dentry_stat_dec(dentry->d_sb, nr_unused);
 		list_del_init(&dentry->d_lru);
@@ -413,7 +417,7 @@ static void prune_dcache(int count)
 		spin_lock(&dentry->d_lock);
 		/*
 		 * We found an inuse dentry which was not removed from
-		 * dentry_unused because of laziness during lookup.  Do not free
+		 * dentry_unused because of laziness during lookup. Do not free
 		 * it - just keep it off the dentry_unused list.
 		 */
 		if (atomic_read(&dentry->d_count)) {
@@ -423,7 +427,7 @@ static void prune_dcache(int count)
 		/* If the dentry was recently referenced, don't free it. */
 		if (dentry->d_flags & DCACHE_REFERENCED) {
 			dentry->d_flags &= ~DCACHE_REFERENCED;
-			list_add(&dentry->d_lru, &dentry_unused);
+			list_add(&dentry->d_lru, &sb->s_dentry_unused);
 			dentry_stat_inc(dentry->d_sb, nr_unused);
 			spin_unlock(&dentry->d_lock);
 			continue;
@@ -433,64 +437,68 @@ static void prune_dcache(int count)
 	spin_unlock(&dcache_lock);
 }
 
-
-/*
- * parsing d_hash list does not hlist_for_each_entry_rcu() as it
- * done under dcache_lock.
+/**
+ * prune_dcache - shrink the dcache
+ * @count: number of entries to try and free
+ *
+ * Prune the dcache. This is done when we need more memory. We are walking the
+ * list of superblocks and try to shrink their dentry LRU lists.
+ *
+ * This function may fail to free any resources if all the dentries are in use.
  */
-static void select_anon(struct super_block *sb)
+static void prune_dcache(int count)
 {
-	struct dentry *dentry;
-	struct hlist_node *lp;
+	struct super_block *sb;
+	int unused = global_dentry_stat.nr_unused;
 
-	spin_lock(&dcache_lock);
-	hlist_for_each_entry(dentry, lp, &sb->s_anon, d_hash) {
-		if (!list_empty(&dentry->d_lru)) {
-			dentry_stat_dec(sb, nr_unused);
-			list_del_init(&dentry->d_lru);
-		}
+	if (count <= 0)
+		return;
 
-		/*
-		 * move only zero ref count dentries to the beginning
-		 * (the most recent end) of the unused list
-		 */
-		spin_lock(&dentry->d_lock);
-		if (!atomic_read(&dentry->d_count)) {
-			list_add(&dentry->d_lru, &dentry_unused);
-			dentry_stat_inc(sb, nr_unused);
+	spin_lock(&sb_lock);
+ restart:
+	list_for_each_entry(sb, &super_blocks, s_list) {
+		sb->s_count++;
+		spin_unlock(&sb_lock);
+		if (down_read_trylock(&sb->s_umount)) {
+			if (sb->s_root) {
+				int tmp;
+
+				/*
+				 * We try to be fair and distribute the amount
+				 * of dentries to be pruned by the easy rule:
+				 *
+				 *   sb_count/sb_unused ~ count/global_unused
+				 *
+				 * This is not enough if the superblock has
+				 * <= 128 unused dentries (this is always
+				 * called via shrink_slab() with a count of
+				 * 128) therefore we use the s_scan_count to
+				 * artifically increase the dentry unused count
+				 * if we haven't pruned any dentries lately (in
+				 * the last runs of prune_dcache().
+				 */
+				tmp = sb->s_dentry_stat.nr_unused /
+					((unused /
+					  ((atomic_read(&sb->s_scan_count)+1) *
+					   count))+1);
+				if (tmp) {
+					atomic_add_unless(
+						&sb->s_scan_count, -1, 0);
+					prune_dcache_sb(sb, tmp);
+				} else
+					atomic_inc(&sb->s_scan_count, 0);
+				if (!sb->s_dentry_stat.nr_unused)
+					atomic_set(&sb->s_scan_count, 0);
+			}
+			up_read(&sb->s_umount);
 		}
-		spin_unlock(&dentry->d_lock);
-	}
-	spin_unlock(&dcache_lock);
-}
-
-static void select_sb(struct super_block *sb)
-{
-	struct dentry *dentry, *pos;
-
-	spin_lock(&dcache_lock);
-	list_for_each_entry_safe(dentry, pos, &dentry_unused, d_lru) {
-		if (dentry->d_sb != sb)
-			continue;
-		list_del(&dentry->d_lru);
-		list_add(&dentry->d_lru, &dentry_unused);
+		spin_lock(&sb_lock);
+		if (__put_super_and_need_restart(sb))
+			goto restart;
 	}
-	spin_unlock(&dcache_lock);
+	spin_unlock(&sb_lock);
 }
 
-/*
- * Shrink the dcache for the specified super block.
- * This allows us to unmount a device without disturbing
- * the dcache for the other devices.
- *
- * This implementation makes just two traversals of the
- * unused list.  On the first pass we move the selected
- * dentries to the most recent end, and on the second
- * pass we free them.  The second pass must restart after
- * each dput(), but since the target dentries are all at
- * the end, it's really just a single traversal.
- */
-
 /**
  * shrink_dcache_sb - shrink dcache for a superblock
  * @sb: superblock
@@ -499,30 +507,16 @@ static void select_sb(struct super_block
  * is used to free the dcache before unmounting a file
  * system
  */
-
 void shrink_dcache_sb(struct super_block * sb)
 {
-	struct list_head *tmp, *next;
-	struct dentry *dentry;
-
-	/*
-	 * Pass one ... move the dentries for the specified
-	 * superblock to the most recent end of the unused list.
-	 */
-	select_anon(sb);
-	select_sb(sb);
+	struct dentry *dentry, *pos;
 
-	/*
-	 * Pass two ... free the dentries for this superblock.
-	 */
 	spin_lock(&dcache_lock);
 repeat:
-	list_for_each_safe(tmp, next, &dentry_unused) {
-		dentry = list_entry(tmp, struct dentry, d_lru);
-		if (dentry->d_sb != sb)
-			continue;
+	list_for_each_entry_safe(dentry, pos, &sb->s_dentry_unused, d_lru) {
+		BUG_ON(dentry->d_sb != sb);
 		dentry_stat_dec(sb, nr_unused);
-		list_del_init(tmp);
+		list_del_init(&dentry->d_lru);
 		spin_lock(&dentry->d_lock);
 		if (atomic_read(&dentry->d_count)) {
 			spin_unlock(&dentry->d_lock);
@@ -625,7 +619,7 @@ resume:
 		 * of the unused list for prune_dcache
 		 */
 		if (!atomic_read(&dentry->d_count)) {
-			list_add(&dentry->d_lru, dentry_unused.prev);
+			list_add_tail(&dentry->d_lru, &dentry->d_sb->s_dentry_unused);
 			dentry_stat_inc(dentry->d_sb, nr_unused);
 			found++;
 		}
@@ -671,7 +665,7 @@ void shrink_dcache_parent(struct dentry 
 	int found;
 
 	while ((found = select_parent(parent)) != 0)
-		prune_dcache(found);
+		prune_dcache_sb(parent->d_sb, found);
 }
 
 /*
@@ -1622,7 +1616,7 @@ resume:
 			list_del_init(&dentry->d_lru);
 		}
 		if (atomic_dec_and_test(&dentry->d_count)) {
-			list_add(&dentry->d_lru, dentry_unused.prev);
+			list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_unused);
 			dentry_stat_inc(dentry->d_sb, nr_unused);
 		}
 	}
@@ -1633,7 +1627,7 @@ resume:
 			list_del_init(&this_parent->d_lru);
 		}
 		if (atomic_dec_and_test(&this_parent->d_count)) {
-			list_add(&this_parent->d_lru, dentry_unused.prev);
+			list_add(&this_parent->d_lru, &this_parent->d_sb->s_dentry_unused);
 			dentry_stat_inc(this_parent->d_sb, nr_unused);
 		}
 		this_parent = this_parent->d_parent;
Index: work-2.6/fs/super.c
===================================================================
--- work-2.6.orig/fs/super.c
+++ work-2.6/fs/super.c
@@ -71,7 +71,9 @@ static struct super_block *alloc_super(v
 		INIT_LIST_HEAD(&s->s_instances);
 		INIT_HLIST_HEAD(&s->s_anon);
 		INIT_LIST_HEAD(&s->s_inodes);
+		INIT_LIST_HEAD(&s->s_dentry_unused);
 		s->s_dentry_stat.age_limit = 45;
+		atomic_set(&s->s_scan_count, 0);
 		init_rwsem(&s->s_umount);
 		mutex_init(&s->s_lock);
 		down_write(&s->s_umount);
Index: work-2.6/include/linux/fs.h
===================================================================
--- work-2.6.orig/include/linux/fs.h
+++ work-2.6/include/linux/fs.h
@@ -847,7 +847,9 @@ struct super_block {
 	struct list_head	s_io;		/* parked for writeback */
 	struct hlist_head	s_anon;		/* anonymous dentries for (nfs) exporting */
 	struct list_head	s_files;
+	struct list_head	s_dentry_unused;
 	struct dentry_stat	s_dentry_stat;
+	atomic_t		s_scan_count;
 
 	struct block_device	*s_bdev;
 	struct list_head	s_instances;

[patch 5/5] vfs: per superblock dentry unused list

[jblunck]

[-- Attachment #1: vfs-per-sb-dentry_unused.diff --]
[-- Type: text/plain, Size: 9354 bytes --]

This patch adds per superblock dentry unused lists. This should speedup the
umounting/remounting of filesystems when there are a lot of dentries on the
unused lists.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 fs/dcache.c        |  158 ++++++++++++++++++++++++-----------------------------
 fs/super.c         |    1 
 include/linux/fs.h |    2 
 3 files changed, 75 insertions(+), 86 deletions(-)

Index: work-2.6/fs/dcache.c
===================================================================
--- work-2.6.orig/fs/dcache.c
+++ work-2.6/fs/dcache.c
@@ -61,7 +61,6 @@ static kmem_cache_t *dentry_cache __read
 static unsigned int d_hash_mask __read_mostly;
 static unsigned int d_hash_shift __read_mostly;
 static struct hlist_head *dentry_hashtable __read_mostly;
-static LIST_HEAD(dentry_unused);
 
 /* Statistics gathering. */
 struct dentry_stat global_dentry_stat = {
@@ -167,13 +166,18 @@ repeat:
 		if (dentry->d_op->d_delete(dentry))
 			goto unhash_it;
 	}
+	/* Kill dentry without superblock */
+	if (unlikely(!dentry->d_sb))
+		goto unhash_it;
 	/* Unreachable? Get rid of it */
 	if (d_unhashed(dentry))
 		goto kill_it;
 	if (list_empty(&dentry->d_lru)) {
 		dentry->d_flags |= DCACHE_REFERENCED;
-		list_add(&dentry->d_lru, &dentry_unused);
+		list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_unused);
 		dentry_stat_inc(dentry->d_sb, nr_unused);
+		dentry->d_sb->s_dentry_unused_age = jiffies +
+			(dentry->d_sb->s_dentry_stat.age_limit * HZ);
 	}
 	spin_unlock(&dentry->d_lock);
 	spin_unlock(&dcache_lock);
@@ -382,19 +386,22 @@ static inline void prune_one_dentry(stru
 }
 
 /**
- * prune_dcache - shrink the dcache
+ * prune_dcache_sb - prune the dcache for a superblock
+ * @sb: superblock
  * @count: number of entries to try and free
  *
- * Shrink the dcache. This is done when we need
- * more memory. When we need to unmount something
- * we call shrink_dcache_sb().
+ * Prune the dcache for the specified super block. This
+ * is called from prune_dcache().
  *
- * This function may fail to free any resources if
- * all the dentries are in use.
+ * You need to have a reference to the super block and
+ * should have sb->s_umount locked. This function may fail
+ * to free any resources if all the dentries are in use.
  */
-
-static void prune_dcache(int count)
+static void prune_dcache_sb(struct super_block *sb, int count)
 {
+	if (count <= 0)
+		return;
+
 	spin_lock(&dcache_lock);
 	for (; count ; count--) {
 		struct dentry *dentry;
@@ -402,10 +409,10 @@ static void prune_dcache(int count)
 
 		cond_resched_lock(&dcache_lock);
 
-		tmp = dentry_unused.prev;
-		if (tmp == &dentry_unused)
+		tmp = sb->s_dentry_unused.prev;
+		if (tmp == &sb->s_dentry_unused)
 			break;
-		prefetch(dentry_unused.prev);
+		prefetch(sb->s_dentry_unused.prev);
 		dentry = list_entry(tmp, struct dentry, d_lru);
 		dentry_stat_dec(dentry->d_sb, nr_unused);
 		list_del_init(&dentry->d_lru);
@@ -423,7 +430,7 @@ static void prune_dcache(int count)
 		/* If the dentry was recently referenced, don't free it. */
 		if (dentry->d_flags & DCACHE_REFERENCED) {
 			dentry->d_flags &= ~DCACHE_REFERENCED;
-			list_add(&dentry->d_lru, &dentry_unused);
+			list_add(&dentry->d_lru, &sb->s_dentry_unused);
 			dentry_stat_inc(dentry->d_sb, nr_unused);
 			spin_unlock(&dentry->d_lock);
 			continue;
@@ -433,64 +440,57 @@ static void prune_dcache(int count)
 	spin_unlock(&dcache_lock);
 }
 
-
-/*
- * parsing d_hash list does not hlist_for_each_entry_rcu() as it
- * done under dcache_lock.
+/**
+ * prune_dcache - shrink the dcache
+ * @count: number of entries to try and free
+ *
+ * Prune the dcache. This is done when we need
+ * more memory.
+ *
+ * This function may fail to free any resources if
+ * all the dentries are in use.
  */
-static void select_anon(struct super_block *sb)
+static void prune_dcache(int count)
 {
-	struct dentry *dentry;
-	struct hlist_node *lp;
+	struct super_block *sb;
+	int unused = global_dentry_stat.nr_unused;
 
-	spin_lock(&dcache_lock);
-	hlist_for_each_entry(dentry, lp, &sb->s_anon, d_hash) {
-		if (!list_empty(&dentry->d_lru)) {
-			dentry_stat_dec(sb, nr_unused);
-			list_del_init(&dentry->d_lru);
-		}
+	if (count <= 0)
+		return;
 
-		/*
-		 * move only zero ref count dentries to the beginning
-		 * (the most recent end) of the unused list
-		 */
-		spin_lock(&dentry->d_lock);
-		if (!atomic_read(&dentry->d_count)) {
-			list_add(&dentry->d_lru, &dentry_unused);
-			dentry_stat_inc(sb, nr_unused);
+	spin_lock(&sb_lock);
+ restart:
+	list_for_each_entry(sb, &super_blocks, s_list) {
+		sb->s_count++;
+		spin_unlock(&sb_lock);
+		if (down_read_trylock(&sb->s_umount)) {
+			if (sb->s_root) {
+				int tmp;
+
+				/*
+				 * Try to be fair to the unused lists:
+				 *  sb_count/sb_unused ~ count/global_unused
+				 *
+				 * Additionally, if the age_limit of the
+				 * superblock is expired shrink at least one
+				 * dentry from the superblock
+				 */
+				tmp = sb->s_dentry_stat.nr_unused /
+					((unused / count) + 1);
+				if (!tmp && time_after(jiffies,
+						       sb->s_dentry_unused_age))
+					tmp = 1;
+				prune_dcache_sb(sb, tmp);
+			}
+			up_read(&sb->s_umount);
 		}
-		spin_unlock(&dentry->d_lock);
-	}
-	spin_unlock(&dcache_lock);
-}
-
-static void select_sb(struct super_block *sb)
-{
-	struct dentry *dentry, *pos;
-
-	spin_lock(&dcache_lock);
-	list_for_each_entry_safe(dentry, pos, &dentry_unused, d_lru) {
-		if (dentry->d_sb != sb)
-			continue;
-		list_del(&dentry->d_lru);
-		list_add(&dentry->d_lru, &dentry_unused);
+		spin_lock(&sb_lock);
+		if (__put_super_and_need_restart(sb))
+			goto restart;
 	}
-	spin_unlock(&dcache_lock);
+	spin_unlock(&sb_lock);
 }
 
-/*
- * Shrink the dcache for the specified super block.
- * This allows us to unmount a device without disturbing
- * the dcache for the other devices.
- *
- * This implementation makes just two traversals of the
- * unused list.  On the first pass we move the selected
- * dentries to the most recent end, and on the second
- * pass we free them.  The second pass must restart after
- * each dput(), but since the target dentries are all at
- * the end, it's really just a single traversal.
- */
-
 /**
  * shrink_dcache_sb - shrink dcache for a superblock
  * @sb: superblock
@@ -499,30 +499,16 @@ static void select_sb(struct super_block
  * is used to free the dcache before unmounting a file
  * system
  */
-
 void shrink_dcache_sb(struct super_block * sb)
 {
-	struct list_head *tmp, *next;
-	struct dentry *dentry;
-
-	/*
-	 * Pass one ... move the dentries for the specified
-	 * superblock to the most recent end of the unused list.
-	 */
-	select_anon(sb);
-	select_sb(sb);
+	struct dentry *dentry, *pos;
 
-	/*
-	 * Pass two ... free the dentries for this superblock.
-	 */
 	spin_lock(&dcache_lock);
 repeat:
-	list_for_each_safe(tmp, next, &dentry_unused) {
-		dentry = list_entry(tmp, struct dentry, d_lru);
-		if (dentry->d_sb != sb)
-			continue;
+	list_for_each_entry_safe(dentry, pos, &sb->s_dentry_unused, d_lru) {
+		BUG_ON(dentry->d_sb != sb);
 		dentry_stat_dec(sb, nr_unused);
-		list_del_init(tmp);
+		list_del_init(&dentry->d_lru);
 		spin_lock(&dentry->d_lock);
 		if (atomic_read(&dentry->d_count)) {
 			spin_unlock(&dentry->d_lock);
@@ -625,7 +611,7 @@ resume:
 		 * of the unused list for prune_dcache
 		 */
 		if (!atomic_read(&dentry->d_count)) {
-			list_add(&dentry->d_lru, dentry_unused.prev);
+			list_add_tail(&dentry->d_lru, &dentry->d_sb->s_dentry_unused);
 			dentry_stat_inc(dentry->d_sb, nr_unused);
 			found++;
 		}
@@ -671,7 +657,7 @@ void shrink_dcache_parent(struct dentry 
 	int found;
 
 	while ((found = select_parent(parent)) != 0)
-		prune_dcache(found);
+		prune_dcache_sb(parent->d_sb, found);
 }
 
 /*
@@ -1622,7 +1608,7 @@ resume:
 			list_del_init(&dentry->d_lru);
 		}
 		if (atomic_dec_and_test(&dentry->d_count)) {
-			list_add(&dentry->d_lru, dentry_unused.prev);
+			list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_unused);
 			dentry_stat_inc(dentry->d_sb, nr_unused);
 		}
 	}
@@ -1633,7 +1619,7 @@ resume:
 			list_del_init(&this_parent->d_lru);
 		}
 		if (atomic_dec_and_test(&this_parent->d_count)) {
-			list_add(&this_parent->d_lru, dentry_unused.prev);
+			list_add(&this_parent->d_lru, &this_parent->d_sb->s_dentry_unused);
 			dentry_stat_inc(this_parent->d_sb, nr_unused);
 		}
 		this_parent = this_parent->d_parent;
Index: work-2.6/fs/super.c
===================================================================
--- work-2.6.orig/fs/super.c
+++ work-2.6/fs/super.c
@@ -71,6 +71,7 @@ static struct super_block *alloc_super(v
 		INIT_LIST_HEAD(&s->s_instances);
 		INIT_HLIST_HEAD(&s->s_anon);
 		INIT_LIST_HEAD(&s->s_inodes);
+		INIT_LIST_HEAD(&s->s_dentry_unused);
 		s->s_dentry_stat.age_limit = 45;
 		init_rwsem(&s->s_umount);
 		mutex_init(&s->s_lock);
Index: work-2.6/include/linux/fs.h
===================================================================
--- work-2.6.orig/include/linux/fs.h
+++ work-2.6/include/linux/fs.h
@@ -847,7 +847,9 @@ struct super_block {
 	struct list_head	s_io;		/* parked for writeback */
 	struct hlist_head	s_anon;		/* anonymous dentries for (nfs) exporting */
 	struct list_head	s_files;
+	struct list_head	s_dentry_unused;
 	struct dentry_stat	s_dentry_stat;
+	unsigned long		s_dentry_unused_age;
 
 	struct block_device	*s_bdev;
 	struct list_head	s_instances;