Re: [PATCH 6/9] readahead: add /debug/readahead/stats

[Jan Kara]

On Wed 14-12-11 14:36:25, Wu Fengguang wrote:
> >   This looks all inherently racy (which doesn't matter much as you suggest)
> > so I just wanted to suggest that if you used per-cpu counters you'd get
> > race-free and faster code at the cost of larger data structures and using
> > percpu_counter_add() instead of ++ (which doesn't seem like a big
> > complication to me).
> 
> OK, here is the incremental patch to use per-cpu counters :)
  Thanks! This looks better. I just thought you would use per-cpu counters
as defined in include/linux/percpu_counter.h and are used e.g. by bdi
stats. This is more standard for statistics in the kernel than using
per-cpu variables directly.

									Honza
> ---
>  mm/readahead.c |   61 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 44 insertions(+), 17 deletions(-)
> 
> --- linux-next.orig/mm/readahead.c	2011-12-14 09:50:37.000000000 +0800
> +++ linux-next/mm/readahead.c	2011-12-14 14:16:15.000000000 +0800
> @@ -68,7 +68,7 @@ enum ra_account {
>  	RA_ACCOUNT_MAX,
>  };
>  
> -static unsigned long ra_stats[RA_PATTERN_MAX][RA_ACCOUNT_MAX];
> +static DEFINE_PER_CPU(unsigned long[RA_PATTERN_ALL][RA_ACCOUNT_MAX], ra_stat);
>  
>  static void readahead_stats(struct address_space *mapping,
>  			    pgoff_t offset,
> @@ -83,38 +83,62 @@ static void readahead_stats(struct addre
>  {
>  	pgoff_t eof = ((i_size_read(mapping->host)-1) >> PAGE_CACHE_SHIFT) + 1;
>  
> -recount:
> -	ra_stats[pattern][RA_ACCOUNT_COUNT]++;
> -	ra_stats[pattern][RA_ACCOUNT_SIZE] += size;
> -	ra_stats[pattern][RA_ACCOUNT_ASYNC_SIZE] += async_size;
> -	ra_stats[pattern][RA_ACCOUNT_ACTUAL] += actual;
> +	preempt_disable();
> +
> +	__this_cpu_inc(ra_stat[pattern][RA_ACCOUNT_COUNT]);
> +	__this_cpu_add(ra_stat[pattern][RA_ACCOUNT_SIZE], size);
> +	__this_cpu_add(ra_stat[pattern][RA_ACCOUNT_ASYNC_SIZE], async_size);
> +	__this_cpu_add(ra_stat[pattern][RA_ACCOUNT_ACTUAL], actual);
>  
>  	if (start + size >= eof)
> -		ra_stats[pattern][RA_ACCOUNT_EOF]++;
> +		__this_cpu_inc(ra_stat[pattern][RA_ACCOUNT_EOF]);
>  	if (actual < size)
> -		ra_stats[pattern][RA_ACCOUNT_CACHE_HIT]++;
> +		__this_cpu_inc(ra_stat[pattern][RA_ACCOUNT_CACHE_HIT]);
>  
>  	if (actual) {
> -		ra_stats[pattern][RA_ACCOUNT_IOCOUNT]++;
> +		__this_cpu_inc(ra_stat[pattern][RA_ACCOUNT_IOCOUNT]);
>  
>  		if (start <= offset && offset < start + size)
> -			ra_stats[pattern][RA_ACCOUNT_SYNC]++;
> +			__this_cpu_inc(ra_stat[pattern][RA_ACCOUNT_SYNC]);
>  
>  		if (for_mmap)
> -			ra_stats[pattern][RA_ACCOUNT_MMAP]++;
> +			__this_cpu_inc(ra_stat[pattern][RA_ACCOUNT_MMAP]);
>  		if (for_metadata)
> -			ra_stats[pattern][RA_ACCOUNT_METADATA]++;
> +			__this_cpu_inc(ra_stat[pattern][RA_ACCOUNT_METADATA]);
>  	}
>  
> -	if (pattern != RA_PATTERN_ALL) {
> -		pattern = RA_PATTERN_ALL;
> -		goto recount;
> -	}
> +	preempt_enable();
> +}
> +
> +static void ra_stats_clear(void)
> +{
> +	int cpu;
> +	int i, j;
> +
> +	for_each_online_cpu(cpu)
> +		for (i = 0; i < RA_PATTERN_ALL; i++)
> +			for (j = 0; j < RA_ACCOUNT_MAX; j++)
> +				per_cpu(ra_stat[i][j], cpu) = 0;
> +}
> +
> +static void ra_stats_sum(unsigned long ra_stats[RA_PATTERN_MAX][RA_ACCOUNT_MAX])
> +{
> +	int cpu;
> +	int i, j;
> +
> +	for_each_online_cpu(cpu)
> +		for (i = 0; i < RA_PATTERN_ALL; i++)
> +			for (j = 0; j < RA_ACCOUNT_MAX; j++) {
> +				unsigned long n = per_cpu(ra_stat[i][j], cpu);
> +				ra_stats[i][j] += n;
> +				ra_stats[RA_PATTERN_ALL][j] += n;
> +			}
>  }
>  
>  static int readahead_stats_show(struct seq_file *s, void *_)
>  {
>  	unsigned long i;
> +	unsigned long ra_stats[RA_PATTERN_MAX][RA_ACCOUNT_MAX];
>  
>  	seq_printf(s,
>  		   "%-10s %10s %10s %10s %10s %10s %10s %10s %10s %10s %10s\n",
> @@ -122,6 +146,9 @@ static int readahead_stats_show(struct s
>  		   "io", "sync_io", "mmap_io", "meta_io",
>  		   "size", "async_size", "io_size");
>  
> +	memset(ra_stats, 0, sizeof(ra_stats));
> +	ra_stats_sum(ra_stats);
> +
>  	for (i = 0; i < RA_PATTERN_MAX; i++) {
>  		unsigned long count = ra_stats[i][RA_ACCOUNT_COUNT];
>  		unsigned long iocount = ra_stats[i][RA_ACCOUNT_IOCOUNT];
> @@ -159,7 +186,7 @@ static int readahead_stats_open(struct i
>  static ssize_t readahead_stats_write(struct file *file, const char __user *buf,
>  				     size_t size, loff_t *offset)
>  {
> -	memset(ra_stats, 0, sizeof(ra_stats));
> +	ra_stats_clear();
>  	return size;
>  }
>  
-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/