Re: Heads up: moving some uvmexp stat to being per-cpu

[Andrew Doran <ad%homeworld.netbsd.org@localhost>]

On Tue, Dec 14, 2010 at 08:49:14PM -0800, Matt Thomas wrote:
> I have a fairly large but mostly simple patch which changes the stats 
> collected in
> uvmexp for faults, intrs, softs, syscalls, and traps from 32 bit to 64 bits 
> and
> puts them in cpu_data (in cpu_info).  This makes more accurate and a little 
> cheaper
> to update on 64bit systems.
> 
> I've had to modify some assembly from architectures I'm not really proficient 
> at
> (m68k, sparc, sparc64, sh3, i386, amd64) but I think did so correctly.  I 
> would
> appreciate some reviews of those changes.  I've verified that all kernels 
> still
> build (except those that didn't before the changes).
> 
> This change also remove the include of <uvm/uvm_extern.h> from files where it
> isn't needed.
> 
> The diffs are at http://www.netbsd.org/uvmexp-diff.txt 
> 
> Have fun reading them!

I did!

I have three concerns around atomicity and concurrency.

- curcpu()->ci_data.cpu_.... we can be preempted in the middle of this type
  of dereference.  So sometimes we could end up with the counter going
  backwards.  Admittedly there are many places where we are counting,
  where interrupts are off.

- 64 bit increment isn't atomic on a 32-bit or RISCy platform.  So the
  same sort of problem as above.

- In uvm where we tally the counters, we can read them mid-update so
  maybe we'll only see half the updated value.  The reason I had the 
  periodic update in the clock interrupt was to avoid this problem.

So all that said, maybe we don't care about the above problems but I wanted
to point them out.  As long as there is not a "really bad" case then
perhaps it's OK.