Re: [PATCH 02/16] mm: vmstat: use s32 for vm_node_stat_diff in struct per_cpu_nodestat

[Roman Gushchin]

On Sun, Oct 20, 2019 at 10:51:10PM +0000, Christopher Lameter wrote:
> On Thu, 17 Oct 2019, Roman Gushchin wrote:
> 
> > Currently s8 type is used for per-cpu caching of per-node statistics.
> > It works fine because the overfill threshold can't exceed 125.
> >
> > But if some counters are in bytes (and the next commit in the series
> > will convert slab counters to bytes), it's not gonna work:
> > value in bytes can easily exceed s8 without exceeding the threshold
> > converted to bytes. So to avoid overfilling per-cpu caches and breaking
> > vmstats correctness, let's use s32 instead.
> 
> Actually this is inconsistenct since the other counters are all used to
> account for pages. Some of the functions in use for the page counters will
> no longer make sense. inc/dec_node_state() etc.

Actually I tried to implement what Johannes suggested earlier and convert
all counters to bytes, but it looked like it can lead to a significant
performance hit on 32bit platforms, as I'd need to replace all atomic_long_t
with atomic64_t. Also, to make the code look good, I'd need to convert
all counters to bytes (and atomic64_t): zone stats, vmevents, etc.
So I gave up relatively quickly.

Maybe it's a good long-term plan, but as now it doesn't really look
as an obviously good think to do.

I'm fully open to any suggestions here.