Re: [PATCH 04/10] lib: percpu_counter_mod64

[Andrew Morton <akpm@linux-foundation.org>]

On Sat, 21 Apr 2007 13:02:26 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> > > +	cpu = get_cpu();
> > > +	pcount = per_cpu_ptr(fbc->counters, cpu);
> > > +	count = *pcount + amount;
> > > +	if (count >= FBC_BATCH || count <= -FBC_BATCH) {
> > > +		spin_lock(&fbc->lock);
> > > +		fbc->count += count;
> > > +		*pcount = 0;
> > > +		spin_unlock(&fbc->lock);
> > > +	} else {
> > > +		*pcount = count;
> > > +	}
> > > +	put_cpu();
> > > +}
> > > +EXPORT_SYMBOL(percpu_counter_mod64);
> > 
> > Bloaty.  Surely we won't be needing this on 32-bit kernels?  Even monster
> > PAE has only 64,000,000 pages and won't be using deltas of more than 4
> > gigapages?
> > 
> > <Does even 64-bit need to handle 4 gigapages in a single hit?  /me suspects
> > another changelog bug>
> 
> Yeah, /me chastises himself for that...
> 
> This is because percpu_counter is s64 instead of the native long; I need
> to halve the counter at some point (bdi_writeout_norm) and do that by
> subtracting half the current value.

ah, the mysterious bdi_writeout_norm().

I don't think it's possible to precisely halve a percpu_counter - there has
to be some error involved.  I guess that's acceptable within the
inscrutable bdi_writeout_norm().

otoh, there's a chance that the attempt to halve the counter will take the
counter negative, due to races.  Does the elusive bdi_writeout_norm()
handle that?  If not, it should.  If it does, then there should be comments
around the places where this is being handled, because it is subtle, and unobvious,
and others might break it by accident.

> If percpu_counter_mod is limited to s32 this might not always work
> (although in practice it might just fit).
-
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/