per-cpu counters (Re: svn commit: r190967 - head/sys/netinet)

[Robert Watson]

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
Computer Laboratory
University of Cambridge