From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH 16/17] fs: Convert nr_inodes to a per-cpu counter Date: Sat, 16 Oct 2010 10:29:08 +0200 Message-ID: <1287217748.2799.68.camel@edumazet-laptop> References: <1285762729-17928-1-git-send-email-david@fromorbit.com> <1285762729-17928-17-git-send-email-david@fromorbit.com> <20100929215322.ff635d3e.akpm@linux-foundation.org> <20100930061039.GX5665@dastard> <20101016075510.GH19147@amd> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Dave Chinner , Andrew Morton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Nick Piggin Return-path: Received: from mail-wy0-f174.google.com ([74.125.82.174]:56704 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751236Ab0JPI3P (ORCPT ); Sat, 16 Oct 2010 04:29:15 -0400 In-Reply-To: <20101016075510.GH19147@amd> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Le samedi 16 octobre 2010 =C3=A0 18:55 +1100, Nick Piggin a =C3=A9crit = : > On Thu, Sep 30, 2010 at 04:10:39PM +1000, Dave Chinner wrote: > > On Wed, Sep 29, 2010 at 09:53:22PM -0700, Andrew Morton wrote: > > > On Wed, 29 Sep 2010 22:18:48 +1000 Dave Chinner wrote: > > >=20 > > > > From: Eric Dumazet > > > >=20 > > > > The number of inodes allocated does not need to be tied to the > > > > addition or removal of an inode to/from a list. If we are not t= ied > > > > to a list lock, we could update the counters when inodes are > > > > initialised or destroyed, but to do that we need to convert the > > > > counters to be per-cpu (i.e. independent of a lock). This means= that > > > > we have the freedom to change the list/locking implementation > > > > without needing to care about the counters. > > > >=20 > > > > > > > > ... > > > > > > > > +int get_nr_inodes(void) > > > > +{ > > > > + int i; > > > > + int sum =3D 0; > > > > + for_each_possible_cpu(i) > > > > + sum +=3D per_cpu(nr_inodes, i); > > > > + return sum < 0 ? 0 : sum; > > > > +} > > >=20 > > > This reimplements percpu_counter_sum_positive(), rather poorly >=20 > Why is it poorly? Nick Some people believe percpu_counter object is the right answer to such distributed counters, because the loop is done on 'online' cpus instead of 'possible' cpus. "It must be better if number of possible cpus is 4096 and only one or two cpus are online"... But if we do this loop only on rare events, like "cat /proc/sys/fs/inode-nr", then the percpu_counter() is more expensive, because percpu_add() _is_ more expensive : - Its a function call and lot of instructions/cycles per call, while this_cpu_inc(nr_inodes) is a single instruction, using no register on x86. - Its possibly accessing a shared spinlock and counter when the percpu counter reaches the batch limit. To recap : nr_inodes is not a counter that needs to be estimated in rea= l time, since we have not limit on number of inodes in the machine (limit is the memory allocator). Unless someone can prove "cat /proc/sys/fs/inode-nr" must be performed thousand of times per second on their setup, the choice I made to scale nr_inodes is better over the 'obvious percpu_counter choice' This choice was made to scale some counters in network stack some years ago, and this rocks. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html