2007-08-22 21:59:55

by Satyam Sharma

[permalink] [raw]
Subject: [PATCH 1/2] intel_cacheinfo: Misc section annotation fixes / cleanups

cache_sysfs_init() is only ever called at device_initcall() time. Marking it
__cpuinit is quite wrong, and wasteful when HOTPLUG_CPU=y. It must be __init.

cache_shared_cpu_map_setup() and cache_remove_shared_cpu_map(), OTOH, are
functions called from another function that is __cpuinit. But the !CONFIG_SMP
empty-body stubs of these functions are unconditionally marked __init, which
is actively wrong, and will lead to oops. But we never saw this oops, because
they always managed to get inlined in their callsites, by virtue of being
empty-body stubs! They should still be __cpuinit, of course.

assocs[], levels[] and types[] are only referenced from function that is
__cpuinit. So these are candidates for being marked __cpuinitdata.

Signed-off-by: Satyam Sharma <[email protected]>

---

arch/i386/kernel/cpu/intel_cacheinfo.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/i386/kernel/cpu/intel_cacheinfo.c b/arch/i386/kernel/cpu/intel_cacheinfo.c
index d5a456d..16499fe 100644
--- a/arch/i386/kernel/cpu/intel_cacheinfo.c
+++ b/arch/i386/kernel/cpu/intel_cacheinfo.c
@@ -170,15 +170,15 @@ union l3_cache {
unsigned val;
};

-static const unsigned short assocs[] = {
+static const unsigned short assocs[] __cpuinitdata = {
[1] = 1, [2] = 2, [4] = 4, [6] = 8,
[8] = 16, [0xa] = 32, [0xb] = 48,
[0xc] = 64,
[0xf] = 0xffff // ??
};

-static const unsigned char levels[] = { 1, 1, 2, 3 };
-static const unsigned char types[] = { 1, 2, 3, 3 };
+static const unsigned char levels[] __cpuinitdata = { 1, 1, 2, 3 };
+static const unsigned char types[] __cpuinitdata = { 1, 2, 3, 3 };

static void __cpuinit amd_cpuid4(int leaf, union _cpuid4_leaf_eax *eax,
union _cpuid4_leaf_ebx *ebx,
@@ -493,8 +493,8 @@ static void __cpuinit cache_remove_shared_cpu_map(unsigned int cpu, int index)
}
}
#else
-static void __init cache_shared_cpu_map_setup(unsigned int cpu, int index) {}
-static void __init cache_remove_shared_cpu_map(unsigned int cpu, int index) {}
+static void __cpuinit cache_shared_cpu_map_setup(unsigned int cpu, int index) {}
+static void __cpuinit cache_remove_shared_cpu_map(unsigned int cpu, int index) {}
#endif

static void free_cache_attributes(unsigned int cpu)
@@ -782,7 +782,7 @@ static struct notifier_block __cpuinitdata cacheinfo_cpu_notifier =
.notifier_call = cacheinfo_cpu_callback,
};

-static int __cpuinit cache_sysfs_init(void)
+static int __init cache_sysfs_init(void)
{
int i;


2007-08-22 22:03:20

by Satyam Sharma

[permalink] [raw]
Subject: [PATCH 2/2] intel_cacheinfo: Call cache_add_dev from cache_sysfs_init explicitly

Call cache_add_dev() from cache_sysfs_init() explicitly, instead of
referencing the CPU notifier callback directly from generic startup
code. Looks cleaner (to me at least) this way, and also makes it
possible to use other tricks to replace __cpuinit{data} annotations,
as recently discussed on this list.

Signed-off-by: Satyam Sharma <[email protected]>

---

arch/i386/kernel/cpu/intel_cacheinfo.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/i386/kernel/cpu/intel_cacheinfo.c b/arch/i386/kernel/cpu/intel_cacheinfo.c
index 16499fe..79e9c4f 100644
--- a/arch/i386/kernel/cpu/intel_cacheinfo.c
+++ b/arch/i386/kernel/cpu/intel_cacheinfo.c
@@ -792,8 +792,8 @@ static int __init cache_sysfs_init(void)
register_hotcpu_notifier(&cacheinfo_cpu_notifier);

for_each_online_cpu(i) {
- cacheinfo_cpu_callback(&cacheinfo_cpu_notifier, CPU_ONLINE,
- (void *)(long)i);
+ struct sys_device *sys_dev = get_cpu_sysdev((unsigned int)i);
+ cache_add_dev(sys_dev);
}

return 0;