Messages in this thread | | | Date | Sun, 18 Jul 2010 15:21:25 +0530 | From | Nitin Gupta <> | Subject | Re: [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/
| |