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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
