Date: Mon, 19 Dec 2005 09:58:09 -0800 (PST)
From: Christoph Lameter <clameter@engr.sgi.com>
To: Marcelo Tosatti <marcelo.tosatti@cyclades.com>
cc: linux-kernel@vger.kernel.org, akpm@osdl.org,
       Hugh Dickins <hugh@veritas.com>, Nick Piggin <nickpiggin@yahoo.com.au>,
       linux-mm@kvack.org, Andi Kleen <ak@suse.de>
Subject: Re: [RFC3 02/14] Basic counter functionality
In-Reply-To: <20051217040115.GA6975@dmt.cnet>
Message-ID: <Pine.LNX.4.62.0512190954580.20292@schroedinger.engr.sgi.com>
References: <20051215001415.31405.24898.sendpatchset@schroedinger.engr.sgi.com>
 <20051215001425.31405.74009.sendpatchset@schroedinger.engr.sgi.com>
 <20051217040115.GA6975@dmt.cnet>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2072
Lines: 56

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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/