Re: vmstat's entries type

John Baldwin wrote:
> On Saturday 29 July 2006 19:38, Michal Mertl wrote:
> > Paul Allen wrote:
> > > Surely all you need to do is a cheap crit_enter,crit_exit 
> > > while updating the 64-bit per cpu counters.  and on
> > > a 64-bit arch you skip the crit_enter,crit_exit.
> > 
> > Critical_enter/exit seem to be quite lightweight (single
> > read/modify/write of a variable).
> > 
> > > Seriously this is a bike shed.  We can summarize it thus:
> > > statistics should be maintained in 64-bit counters, these
> > > counters should be per-cpu and consistent in that context,
> > > nothing else should appear on the critical path.
> > 
> > Why do you call it a bikesched? I think that your proposal could work
> > but as nobody proposed doing the stuff with critical_* before, the
> > thread may be fruitful. 
> > 
> > Is critical_* good enough protection though? What if two threads were
> > updating the same per-CPU counter on the same CPU at the same time? With
> > 64bits counter on a 32bit architecture? I expect the cache coherency
> > issues are completely eliminated with per-CPU data, aren't they?
> 
> critical_* would prevent an interrupt from preempting the thread, so you 
> wouldn't have this case.
> 
> That said, I think just using a simple algo (like inc; jnc 1b; inc; 1:) would 
> be fine.  With the counter being per-cpu you don't even need the 'lock' 
> prefix for i386.  You would have to find similar solutions for other 32-bit 
> archs (arm, ppc, mips).

Attached please find a diff against fresh CURRENT src/sys, which
basically just wraps all modifications of the vmmeter's counters with
PCPU_CNTR{64,}_INC/ADD. The implementation of the increment is left to
the compiler but it should be easy to write anything MD instead (I
believe compiler generates rather good code (probably very similar to
your sequence above on i386)). The counters were mostly changed to
int64_t; uint should be fine too but I remember BDE said several times
unsigned types are evil :-). The macro may also be changed to give the
user compile-time chance to choose whether to prefer accuracy or speed,
e.g. use critical_enter/exit.

The resulting UP I386 kernel works and the stats seem to look OK. The
consumers have to be changed too (systat needs attached patch, sysctl
needs to be recompiled to catch the change in src/sys/sys/vmmeter.h,
other programs may have to be recompiled/changed a little too).

I would love to measure the performance impact, especially on SMP I386
but I don't have easy access to such a hardware at the moment. I think
the impact shouldn't be measuarable, although the changes in
src/sys/vm_fault/vm_fault(), where atomic_add_int has been replaced by
the simple PCPU increment, may be positive on SMP configs under 
high memory pressure.

Michal