On Tue, 24 Feb 2015 18:57:49 +0100, Borislav Petkov wrote:
On Mon, Feb 23, 2015 at 06:14:25PM +0000, Sudeep Holla wrote:
- Rebased on v4.0-rc1You probably have fixed the lockdep splat but not the NULL pointer
- Fixed lockdep warning reported by Borislav
dereference which was there in the first mail I sent you. I'd suggest
you find an AMD box with an L3 and do some testing yourself before
sending out another version.
So I gave that a spin on my old PhenomII X6 box, and indeed there is a
NULL pointer dereference. I tracked it down to
__cache_amd_cpumap_setup(), where the sibling map for the L3 is
populated. When the function is called for the first CPU, subsequent
CPU's cacheinfo structures obviously haven't been initialized yet, so
the struct cpu_cacheinfo we obtain for the other CPUs is empty
(num_levels and num_leaves are 0). Dereferencing info_list + index is
then a bad idea.
I fixed it by simply skipping the sibling map setup in this case (see
the patch below). Later we revisit this code path because the most
outer loop iterates over all CPUs anyway, so the fields are valid then
and the sibling map is build up properly. Not sure if that is a proper
fix (should we check for info_list being NULL?, is num_levels the
right value?), but I don't feel like looking into this code deeper
than I already did (Sudeep should be decorated for doing that!).
Boris, can you try this on a Bulldozer class machine?
Sudeep, if Boris acknowledges this, can you merge those two lines (or
something similar) in your patch and repost it? I can give it a try
then again.
Also, when testing, do a snapshot of the sysfs cache hierarchy by
doing:
grep . -EriIn /sys/devices/system/cpu/cpu?/cache/*
before your changes and after and check for differences.
We can't allow ourselves any differences because this is an API.
But as Sudeep mentioned, this is an orthogonal issue not directly
related to this patch.
Indeed I see a regression between 3.19.0 and a clean 4.0.0-rc1 in this
respect, the diff is like:
-cpu5/cache/index3/shared_cpu_map:00000000,0000003f
+cpu5/cache/index3/shared_cpu_map:3f
(for each cpu and index)
Can someone confirm (and investigate) this?