Re: [PATCH 4/5] superblock: add filesystem shrinker operations

[Andrew Morton <akpm@linux-foundation.org>]

On Tue, 25 May 2010 18:53:07 +1000
Dave Chinner <david@fromorbit.com> wrote:

> From: Dave Chinner <dchinner@redhat.com>
> 
> Now we have a per-superblock shrinker implementation, we can add a
> filesystem specific callout to it to allow filesystem internal
> caches to be shrunk by the superblock shrinker.
> 
> Rather than perpetuate the multipurpose shrinker callback API (i.e.
> nr_to_scan == 0 meaning "tell me how many objects freeable in the
> cache), two operations will be added. The first will return the
> number of objects that are freeable, the second is the actual
> shrinker call.
> 
>
> ...
>
>  static int prune_super(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask)
>  {
>  	struct super_block *sb;
> -	int count;
> +	int	fs_objects = 0;
> +	int	total_objects;
>  
>  	sb = container_of(shrink, struct super_block, s_shrink);
>  
> @@ -63,22 +64,40 @@ static int prune_super(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask)
>  		return -1;
>  	}
>  
> -	if (nr_to_scan) {
> -		/* proportion the scan between the two cache__ */
> -		int total;
> -
> -		total = sb->s_nr_dentry_unused + sb->s_nr_inodes_unused + 1;
> -		count = (nr_to_scan * sb->s_nr_dentry_unused) / total;
> +	if (sb->s_op && sb->s_op->nr_cached_objects)
> +		fs_objects = sb->s_op->nr_cached_objects(sb);
>  
> -		/* prune dcache first as icache is pinned by it */
> -		prune_dcache_sb(sb, count);
> -		prune_icache_sb(sb, nr_to_scan - count);
> +	total_objects = sb->s_nr_dentry_unused +
> +			sb->s_nr_inodes_unused + fs_objects + 1;
> +	if (nr_to_scan) {
> +		int	dentries;
> +		int	inodes;
> +
> +		/* proportion the scan between the cache__ */
> +		dentries = (nr_to_scan * sb->s_nr_dentry_unused) /
> +							total_objects;
> +		inodes = (nr_to_scan * sb->s_nr_inodes_unused) /
> +							total_objects;
> +		if (fs_objects)
> +			fs_objects = (nr_to_scan * fs_objects) /
> +							total_objects;
> +		/*
> +		 * prune the dcache first as the icache is pinned by it, then
> +		 * prune the icache, followed by the filesystem specific caches
> +		 */
> +		prune_dcache_sb(sb, dentries);
> +		prune_icache_sb(sb, inodes);
> +		if (sb->s_op && sb->s_op->free_cached_objects) {

Under which circumstances is a NULL ->free_cached_objects valid?

> +			sb->s_op->free_cached_objects(sb, fs_objects);
> +			fs_objects = sb->s_op->nr_cached_objects(sb);
> +		}
> +		total_objects = sb->s_nr_dentry_unused +
> +				sb->s_nr_inodes_unused + fs_objects;
>  	}

The return value from ->free_cached_objects() doesn't actually get
used.  Instead the code calls ->nr_cached_objects() twice.


> -	count = ((sb->s_nr_dentry_unused + sb->s_nr_inodes_unused) / 100)
> -						* sysctl_vfs_cache_pressure;
> +	total_objects = (total_objects / 100) * sysctl_vfs_cache_pressure;
>  	up_read(&sb->s_umount);
> -	return count;
> +	return total_objects;
>  }
>  
>  /**
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 5bff2dc..efcdcc6 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1590,6 +1590,17 @@ struct super_operations {
>  	ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
>  #endif
>  	int (*bdev_try_to_free_page)(struct super_block*, struct page*, gfp_t);
> +
> +	/*
> +	 * memory shrinker operations.
> +	 * ->nr_cached_objects() should return the number of freeable cached
> +	 * objects the filesystem holds.
> +	 * ->free_cache_objects() should attempt to free the number of cached
> +	 * objects indicated. It should return how many objects it attempted to
> +	 * free.
> +	 */

I'd have thought that ->free_cache_objects() would always return the
number which it was passed.  Unless someone asked it to scan more
objects than exist, perhaps.

> +	int (*nr_cached_objects)(struct super_block *);
> +	int (*free_cached_objects)(struct super_block *, int);
>  };

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs