[PATCH] fs: use approximate counter values for inodes and dentries. (was Re: [patch] fs: use fast counters for vfs caches)

[Dave Chinner <david@fromorbit.com>]

On Thu, Dec 09, 2010 at 05:16:44PM +1100, Nick Piggin wrote:
> On Thu, Dec 09, 2010 at 04:43:43PM +1100, Dave Chinner wrote:
> > On Mon, Nov 29, 2010 at 09:57:33PM +1100, Nick Piggin wrote:
> > > Hey,
> > > 
> > > What was the reason behind not using my approach to use fast per-cpu
> > > counters for inode and dentry counters, and instead using the
> > > percpu_counter lib (which is not useful unless very fast approximate
> > > access to the global counter is required, or performance is not
> > > critical, which is somewhat of an oxymoron if you're using per-counters
> > > in the first place). It is a difference between this:
> > 
> > Hi Nick - sorry for being slow to answer this - I only just found
> > this email.
> > 
> > The reason for using the generic counters is because the shrinkers
> > read the current value of the global counter on every call and hence
> > they can be read thousands of times a second. The only way to do that
> > efficiently is to use the approximately value the generic counters
> > provide.
> 
> That is not what is happening, though, so I assume that no measurements
> were done.
>
> In fact what happens now is that *both* type of counters use the crappy
> percpu counter library, and the shrinkers actually do a per-cpu loop
> over the counters to get the sum.

More likely that the overhead was hidden in the noise on the size of
machines most people test on. It certainly wasn't measurable on my
16p machine, and nobody who reviewed it at the time (ѕeveral people)
picked it up. So thanks for reviewing it - the simple fix is below.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

fs: Use approximate values for number of inodes and dentries

From: Dave Chinner <dchinner@redhat.com>

The number of inodes and dentries are percpu counters that are
summed by the shrinkers. This can result in summing across all CPUs
thousands of times per second per counter which is very inefficient.
The approximate counter value should be used instead to keep the
overhead to a minimum.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/dcache.c |    4 ++--
 fs/inode.c  |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 23702a9..a533184 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -546,7 +546,7 @@ static void prune_dcache(int count)
 {
 	struct super_block *sb, *p = NULL;
 	int w_count;
-	int unused = percpu_counter_sum_positive(&nr_dentry_unused);
+	int unused = percpu_counter_read_positive(&nr_dentry_unused);
 	int prune_ratio;
 	int pruned;
 
@@ -916,7 +916,7 @@ static int shrink_dcache_memory(struct shrinker *shrink, int nr, gfp_t gfp_mask)
 		prune_dcache(nr);
 	}
 
-	nr_unused = percpu_counter_sum_positive(&nr_dentry_unused);
+	nr_unused = percpu_counter_read_positive(&nr_dentry_unused);
 	return (nr_unused / 100) * sysctl_vfs_cache_pressure;
 }
 
diff --git a/fs/inode.c b/fs/inode.c
index ae2727a..975a651 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -109,12 +109,12 @@ static struct kmem_cache *inode_cachep __read_mostly;
 
 static inline int get_nr_inodes(void)
 {
-	return percpu_counter_sum_positive(&nr_inodes);
+	return percpu_counter_read_positive(&nr_inodes);
 }
 
 static inline int get_nr_inodes_unused(void)
 {
-	return percpu_counter_sum_positive(&nr_inodes_unused);
+	return percpu_counter_read_positive(&nr_inodes_unused);
 }
 
 int get_nr_dirty_inodes(void)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html