Re: [RFC3 02/14] Basic counter functionality

Christoph Lameter
2005-12-19T17:58:09Z

On Sat, 17 Dec 2005, Marcelo Tosatti wrote:

> > +static inline s8 *diff_pointer(struct zone *zone, enum zone_stat_item item)
> > +{
> > + return &zone_pcp(zone, raw_smp_processor_id())->vm_stat_diff[item];
> > +}
> > +
> > +/*
> > + * For use when we know that interrupts are disabled.
> > + */
> > +void __mod_zone_page_state(struct zone *zone, enum zone_stat_item item, int delta)
> > +{
> > + s8 *p;
> > + long x;
> > +
> > + p = diff_pointer(zone, item);
> > + x = delta + *p;
> > +
> > + if (unlikely(x > STAT_THRESHOLD || x < -STAT_THRESHOLD)) {
> > + zone_page_state_consolidate(x, zone, item);
> > + x = 0;
> > + }
> > +
> > + *p = x;
> > +}
>
> There is no need to disable interrupts AFAICS, but only preemption
> (which could cause problems as your comment above describes). I suppose
> that these counters are not accessed at interrupt time and are not meant
> to be, right?

Some of the counters can be accessed at interrupt time and these are meant
to be right. Next rev adds another racy version of the counters that will
be used for the optional VM counters. Those counters will benefit from
inc/dec if generated by the compiler.

> Why not use preempt_disable/preempt_enable? Those would disappear
> if !CONFIG_PREEMPT, and could be faster than the interrupt
> disabling/enabling (no need to save "flags" on stack, but increment
> preempt count, which has a chance to be on cache, I guess).

On a counter by counter basis one could use the __ functions that do not
disable interrupts.

> It would also be nice to have all code related to debugging only
> counters selectable at compile time, since it might not be interesting
> data for some scenarios (but unnecessary bloat) - seems that was the
> original intent by Andrew as you noted.

That will be part of the next rev that I am currently testing.