Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758226Ab0HKRRZ (ORCPT ); Wed, 11 Aug 2010 13:17:25 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:58742 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754474Ab0HKRRY (ORCPT ); Wed, 11 Aug 2010 13:17:24 -0400 Date: Wed, 11 Aug 2010 10:18:54 -0700 From: Andrew Morton To: ngupta@vflare.org Cc: Pekka Enberg , Minchan Kim , Greg KH , Linux Driver Project , linux-mm , linux-kernel Subject: Re: [PATCH 03/10] Use percpu stats Message-Id: <20100811101854.1dc3a510.akpm@linux-foundation.org> In-Reply-To: <4C62D241.2040208@vflare.org> References: <1281374816-904-1-git-send-email-ngupta@vflare.org> <1281374816-904-4-git-send-email-ngupta@vflare.org> <20100809213431.d7699d46.akpm@linux-foundation.org> <4C62D241.2040208@vflare.org> X-Mailer: Sylpheed 2.7.1 (GTK+ 2.18.9; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2536 Lines: 79 On Wed, 11 Aug 2010 22:09:29 +0530 Nitin Gupta wrote: > On 08/10/2010 10:04 AM, Andrew Morton wrote: > > On Mon, 9 Aug 2010 22:56:49 +0530 Nitin Gupta 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/