Re: [PATCH 02/14] Basic ZVC (zoned vm counter) implementation

Christoph Lameter
2006-06-22T14:56:55Z

On Thu, 22 Jun 2006, Andrew Morton wrote:

> These patches were a bit of a disaster - what happened?

I am not sure what you did to them. You removed pieces that were not
touched by this patch. Could you undo your additional patches and tell me
what the original problem was?

> > +atomic_long_t vm_stat[NR_VM_ZONE_STAT_ITEMS];
> > +
> > +static inline void zone_page_state_add(long x, struct zone *zone,
> > + enum zone_stat_item item)
> > +{
> > + atomic_long_add(x, &zone->vm_stat[item]);
> > + atomic_long_add(x, &vm_stat[item]);
> > +}
>
> I'd have thought it'd be worth padding vm_stat out to one-per-cacheline.

On the other hand if the whole vm_stat is in one cacheline then multiple
references to counters can be satisfied from one cacheline.

> > + x = delta + *p;
> > +
> > + if (unlikely(x > STAT_THRESHOLD || x < -STAT_THRESHOLD)) {
> > + zone_page_state_add(x, zone, item);
> > + x = 0;
> > + }
> > +
> > + *p = x;
> > +}
>
> We'd get a little more efficiency with the same accuracy by doing:
>
> if (x > STAT_THRESHOLD) {
> zone_page_state_add(x + STAT_THRESHOLD/2, zone, item);
> x = -STAT_THRESHOLD/2;
> }
>
> because these things aren't random - a CPU tends to do a lot of identical
> operations in sequence, then a batch of the opposite operation happens a
> relatively long time later. Memory allocations, pagetable instantiation,
> etc. Plus some counters only go in one direction.

0 is already in the middle of the interval. The interval is from
-STAT_THRESHOLD to +STAT_THRESHOLD not from 0 to STAT_THRESHOLD.