Re: [PATCH 03/10] Use percpu stats

[Andrew Morton]

On Wed, 11 Aug 2010 22:09:29 +0530 Nitin Gupta <ngupta@xxxxxxxxxx> wrote:

> On 08/10/2010 10:04 AM, Andrew Morton wrote:
> > On Mon,  9 Aug 2010 22:56:49 +0530 Nitin Gupta <ngupta@xxxxxxxxxx> wrote:
> > 
> >> +/*
> >> + * Individual percpu values can go negative but the sum across all CPUs
> >> + * must always be positive (we store various counts). So, return sum as
> >> + * unsigned value.
> >> + */
> >> +static u64 zram_get_stat(struct zram *zram, enum zram_stats_index idx)
> >>  {
> >> -	u64 val;
> >> -
> >> -	spin_lock(&zram->stat64_lock);
> >> -	val = *v;
> >> -	spin_unlock(&zram->stat64_lock);
> >> +	int cpu;
> >> +	s64 val = 0;
> >> +
> >> +	for_each_possible_cpu(cpu) {
> >> +		s64 temp;
> >> +		unsigned int start;
> >> +		struct zram_stats_cpu *stats;
> >> +
> >> +		stats = per_cpu_ptr(zram->stats, cpu);
> >> +		do {
> >> +			start = u64_stats_fetch_begin(&stats->syncp);
> >> +			temp = stats->count[idx];
> >> +		} while (u64_stats_fetch_retry(&stats->syncp, start));
> >> +		val += temp;
> >> +	}
> >>  
> >> +	WARN_ON(val < 0);
> >>  	return val;
> >>  }
> > 
> > That reimplements include/linux/percpu_counter.h, poorly.
> > 
> > Please see the June discussion "[PATCH v2 1/2] tmpfs: Quick token
> > library to allow scalable retrieval of tokens from token jar" for some
> > discussion.
> > 
> > 
> 
> I read the discussion you pointed out but still fail to see how percpu_counters,
> with all their overhead,

What overhead?  Send numbers.  Then extrapolate those numbers to a
machine which has 128 possible CPUs and 4 present CPUs.

> are better than simple pcpu variable used in current
> version. What is the advantage?

Firstly, they'd have saved all the time you spent duplicating them.

Secondly, getting additional users of the standard facility results in
more testing and perhaps enhancement of that facility, thus benefiting
other users too.

Thirdly, using the standard facility permits your code to leverage
enhancements which others add. 

Fourthly, they would result in a smaller kernel.

You didn't really need me to teach you the benefits of code reuse did
you?


Please do not merge this code unless there is a good reason to do so
and it has been shown that the standard facility cannot be suitably
fixed or enhanced to address the deficiency.


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