Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 12 Apr 2009 16:30:00 +0200
From:      Luigi Rizzo <rizzo@iet.unipi.it>
To:        Robert Watson <rwatson@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   per-cpu counters (Re: svn commit: r190967 - head/sys/netinet)
Message-ID:  <20090412143000.GB96365@onelab2.iet.unipi.it>
In-Reply-To: <200904121406.n3CE6QSH027497@svn.freebsd.org>
References:  <200904121406.n3CE6QSH027497@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Apr 12, 2009 at 02:06:26PM +0000, Robert Watson wrote:
> Author: rwatson
> Date: Sun Apr 12 14:06:26 2009
> New Revision: 190967
> URL: http://svn.freebsd.org/changeset/base/190967
> 
> Log:
>   Update stats in struct pimstat using two new macros: PIMSTAT_ADD()
>   and PIMSTAT_INC(), rather than directly manipulating the fields of
>   the structure.  This will make it easier to change the
>   implementation of these statistics, such as using per-CPU versions
>   of the data structure.
>   
>   MFC after:	3 days

i understand that the change is only a cosmetic one, but do we really
want to MFC all these counter changes now ?

The problem of per-cpu counters exist all over the kernel, so rather
than ad-hoc macros for each subsystem, it might be more interesting to
first design and discuss a global solution (using HEAD as a test
platform, if you like), and only at that stage start MFCing.

As an example, an approach i was considering is replace the
existing counters with an index in a global_counters[MAXCPU][]
array so the counter update will become something like this:

-	pimstat.pims_rcv_registers_wrongiif++;
+	global_counters[PCPU_GET(cpuid)]
+		[pimstat.pims_rcv_registers_wrongiif]++;

When you allocate an entry you also need to allocate an entry in
the global_counters, but entries have fixed size, and the modules
that you build will not depend on the MAXCPU value.

All of this is still compatible with the macros you are adding,
but I wonder why we should have many PIMSTAT_INC, IPFW_INC, NATD_INC
and so on instead of more generic COUNTER64_INC() or COUNTER32_INC()

	cheers
	luigi



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090412143000.GB96365>