lkml.org 
[lkml]   [2010]   [Jul]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/8] Basic zcache functionality
On 07/18/2010 02:14 PM, Eric Dumazet wrote:
> Le vendredi 16 juillet 2010 à 18:07 +0530, Nitin Gupta a écrit :
>
>> This particular patch implemets basic functionality only:
>> +static u64 zcache_get_stat(struct zcache_pool *zpool,
>> + enum zcache_pool_stats_index idx)
>> +{
>> + int cpu;
>> + s64 val = 0;
>> +
>> + for_each_possible_cpu(cpu) {
>> + unsigned int start;
>> + struct zcache_pool_stats_cpu *stats;
>> +
>> + stats = per_cpu_ptr(zpool->stats, cpu);
>> + do {
>> + start = u64_stats_fetch_begin(&stats->syncp);
>> + val += stats->count[idx];
>> + } while (u64_stats_fetch_retry(&stats->syncp, start));
>> + }
>> +
>> + BUG_ON(val < 0);
>> + return val;
>> +}
>
> Sorry this is wrong.
>
> Inside the fetch/retry block you should not do the addition to val, only
> a read of value to a temporary variable, since this might be done
> several times.
>
> You want something like :
>
> static u64 zcache_get_stat(struct zcache_pool *zpool,
> enum zcache_pool_stats_index idx)
> {
> int cpu;
> s64 temp, val = 0;
>
> for_each_possible_cpu(cpu) {
> unsigned int start;
> struct zcache_pool_stats_cpu *stats;
>
> stats = per_cpu_ptr(zpool->stats, cpu);
> do {
> start = u64_stats_fetch_begin(&stats->syncp);
> temp = stats->count[idx];
> } while (u64_stats_fetch_retry(&stats->syncp, start));
> val += temp;
> }
>
> ...
> }
>
>

Oh, my bad. Thanks for the fix!

On a side note: u64_stats_* should probably be renamed to stats64_* since
they are equally applicable for s64.


Thanks,
Nitin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2010-07-18 11:53    [W:0.085 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site