Re: [PATCH UPDATED] percpu: use dynamic percpu allocator as the default percpu allocator

Ingo Molnar
2009-04-01T19:06:37Z

* Christoph Lameter <cl@linux.com> wrote:

> __read_mostly should be packed as tightly as possible to increase
> the chance that one cacheline includes multiple of the critical
> variables for the hot code paths. Too much __read_mostly defeats
> its purpose.

That stance is a commonly held but quite wrong and harmful IMHO.

It stiffles the proper identification of read-mostly variables _AND_
it hurts the proper identification of critical write-often variables
as well. Not good.

The solution for critical write-often variables is what we always
used: to identify them explicitly and to place them consciously into
separate cachelines. (Or to per-cpu-ify or object-ify them where
possible/sensible.)

Then annotate everything that is read-mostly and accessed-frequently
with the __read_mostly attribute.

The rest (unannotated variables) is to be assumed "access-rarely" or
"we-dont-care", by default. This is actually 95% of the global
variables.

Yes, a spreading amount of annotations puts increasing pressure on
the places that are frequently access but not properly annotated -
but we should be happy about that: it creates the dynamics and
pressure for them to be properly annotated.

On the other hand, depending on the "put enough data bloat between
critical variables anyway, no need to care alignment" scheme is a
sloppy, fragile concept that does not lead to a reliable and
dependable end result.

It has two problems:

- Thinking that this solves false cacheline sharing reliably is
wrong: there's nothing that guarantees and enforces that slapping
a few variables between two critical variables puts them on
separate cachelines:

- Ongoing changes in code can bit-rot the
thought-to-be-large-enough distance between two critical
variables - and there's no mechanism in place.
Explicitly cacheline aligning them will preserve the
information long-term.

- There are architectures with larger cacheline sizes than
what you are developing on.

- .config variations can move variables closer or farther
apart from each other, hiding/triggering the false cacheline
sharing problem.

It is not a maintainable concept IMHO and we should not pretend
it is.

- It actually prevents true read-mostly variables from being
annotated properly. (In such a case a true read-mostly variable
bouncing around with a frequently-written variable cache line is
almost as bad in terms of MESI latencies and costs as false
cacheline sharing between two write-mostly variables.)

Architecturing the layout of variables in a knowingly random and
.config sensitive way is simply not good design and we should not
pretend it is.

We might not be able to solve the problem if not enough people care
about their variables, but we should at least not be proud of a
non-solution ;-)

Ingo