per-cpu counters (Re: svn commit: r190967 - head/sys/netinet)
rwatson at FreeBSD.org
Sun Apr 12 14:28:49 UTC 2009
On Sun, 12 Apr 2009, Luigi Rizzo wrote:
> On Sun, Apr 12, 2009 at 02:06:26PM +0000, Robert Watson wrote:
>> 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 ?
Consider 3 days the minimum, rather than expected, MFC time.
> 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()
I have a project along these lines in progress, and will post the proposal to
arch@ once I've finished prototyping it. In particular, it provides common
implementations of "reset" and "report" in order to expose a single userspace
version of the structure via sysctl.
The reason to use per-subsystem macros is that there's another dimmension
here: virtualization. Some counters are per-CPU, others are per-VIMAGE, and
many are per-CPU-per-VIMAGE. Since virtualization is a work-in-progress, and
they haven't yet virtualized all the counter blocks, I figured it was best to
push all the counter updates behind subsystem-specific macros, and then
virtualization would touch only the macro definitions, not the implementation
in every place. In the sample implementation you show above, changes have to
be made to every instance of the counter when virtualization is
added/removed/changed for PIM, making per-CPU changes hard to MFC.
Robert N M Watson
University of Cambridge
More information about the svn-src-head