linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch -mm4] i386: __init should be __cpuinit
@ 2006-02-01  4:49 Chuck Ebbert
  2006-02-01  5:33 ` Dave Jones
  0 siblings, 1 reply; 9+ messages in thread
From: Chuck Ebbert @ 2006-02-01  4:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Ashok Raj, linux-kernel

When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
referenced from normal code.

On startup:
        arch/i386/kernel/cpu/amd.c::amd_init_cpu():
                cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;        
        amd_cpu_dev is declared __initdata and is freed

On CPU hotplug:
        arch/i386/kernel/cpu/common.c::get_cpu_vendor():
               for (i = 0; i < X86_VENDOR_NUM; i++) {
                        if (cpu_devs[i]) {
                                if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||

To fix this, change every instance of __init that seems suspicious
into __cpuinit.  When !CONFIG_HOTPLUG_CPU there is no change in .text
or .data size.  When enabled, .text += 3248 bytes; .data += 2148 bytes.

This should be safe in every case; the only drawback is the extra code and
data when CPU hotplug is enabled.

Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>

---

 arch/i386/kernel/cpu/amd.c             |    4 +--
 arch/i386/kernel/cpu/centaur.c         |   22 +++++++++----------
 arch/i386/kernel/cpu/cyrix.c           |   37 ++++++++++++++++-----------------
 arch/i386/kernel/cpu/intel_cacheinfo.c |    1 
 arch/i386/kernel/cpu/nexgen.c          |    8 +++----
 arch/i386/kernel/cpu/rise.c            |    4 +--
 6 files changed, 39 insertions(+), 37 deletions(-)

--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/amd.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/amd.c
@@ -22,7 +22,7 @@
 extern void vide(void);
 __asm__(".align 4\nvide: ret");
 
-static void __init init_amd(struct cpuinfo_x86 *c)
+static void __cpuinit init_amd(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	int mbytes = num_physpages >> (20-PAGE_SHIFT);
@@ -255,7 +255,7 @@ static unsigned int amd_size_cache(struc
 	return size;
 }
 
-static struct cpu_dev amd_cpu_dev __initdata = {
+static struct cpu_dev amd_cpu_dev __cpuinitdata = {
 	.c_vendor	= "AMD",
 	.c_ident 	= { "AuthenticAMD" },
 	.c_models = {
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/centaur.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/centaur.c
@@ -8,7 +8,7 @@
 
 #ifdef CONFIG_X86_OOSTORE
 
-static u32 __init power2(u32 x)
+static u32 __cpuinit power2(u32 x)
 {
 	u32 s=1;
 	while(s<=x)
@@ -21,7 +21,7 @@ static u32 __init power2(u32 x)
  *	Set up an actual MCR
  */
  
-static void __init centaur_mcr_insert(int reg, u32 base, u32 size, int key)
+static void __cpuinit centaur_mcr_insert(int reg, u32 base, u32 size, int key)
 {
 	u32 lo, hi;
 	
@@ -39,7 +39,7 @@ static void __init centaur_mcr_insert(in
  *	Shortcut: We know you can't put 4Gig of RAM on a winchip
  */
 
-static u32 __init ramtop(void)		/* 16388 */
+static u32 __cpuinit ramtop(void)		/* 16388 */
 {
 	int i;
 	u32 top = 0;
@@ -90,7 +90,7 @@ static u32 __init ramtop(void)		/* 16388
  *	Compute a set of MCR's to give maximum coverage
  */
 
-static int __init centaur_mcr_compute(int nr, int key)
+static int __cpuinit centaur_mcr_compute(int nr, int key)
 {
 	u32 mem = ramtop();
 	u32 root = power2(mem);
@@ -165,7 +165,7 @@ static int __init centaur_mcr_compute(in
 	return ct;
 }
 
-static void __init centaur_create_optimal_mcr(void)
+static void __cpuinit centaur_create_optimal_mcr(void)
 {
 	int i;
 	/*
@@ -188,7 +188,7 @@ static void __init centaur_create_optima
 		wrmsr(MSR_IDT_MCR0+i, 0, 0);
 }
 
-static void __init winchip2_create_optimal_mcr(void)
+static void __cpuinit winchip2_create_optimal_mcr(void)
 {
 	u32 lo, hi;
 	int i;
@@ -226,7 +226,7 @@ static void __init winchip2_create_optim
  *	Handle the MCR key on the Winchip 2.
  */
 
-static void __init winchip2_unprotect_mcr(void)
+static void __cpuinit winchip2_unprotect_mcr(void)
 {
 	u32 lo, hi;
 	u32 key;
@@ -238,7 +238,7 @@ static void __init winchip2_unprotect_mc
 	wrmsr(MSR_IDT_MCR_CTRL, lo, hi);
 }
 
-static void __init winchip2_protect_mcr(void)
+static void __cpuinit winchip2_protect_mcr(void)
 {
 	u32 lo, hi;
 	
@@ -256,7 +256,7 @@ static void __init winchip2_protect_mcr(
 #define RNG_ENABLED	(1 << 3)
 #define RNG_ENABLE	(1 << 6)	/* MSR_VIA_RNG */
 
-static void __init init_c3(struct cpuinfo_x86 *c)
+static void __cpuinit init_c3(struct cpuinfo_x86 *c)
 {
 	u32  lo, hi;
 
@@ -302,7 +302,7 @@ static void __init init_c3(struct cpuinf
 	display_cacheinfo(c);
 }
 
-static void __init init_centaur(struct cpuinfo_x86 *c)
+static void __cpuinit init_centaur(struct cpuinfo_x86 *c)
 {
 	enum {
 		ECX8=1<<1,
@@ -456,7 +456,7 @@ static unsigned int centaur_size_cache(s
 	return size;
 }
 
-static struct cpu_dev centaur_cpu_dev __initdata = {
+static struct cpu_dev centaur_cpu_dev __cpuinitdata = {
 	.c_vendor	= "Centaur",
 	.c_ident	= { "CentaurHauls" },
 	.c_init		= init_centaur,
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/cyrix.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/cyrix.c
@@ -12,7 +12,7 @@
 /*
  * Read NSC/Cyrix DEVID registers (DIR) to get more detailed info. about the CPU
  */
-static void __init do_cyrix_devid(unsigned char *dir0, unsigned char *dir1)
+static void __cpuinit do_cyrix_devid(unsigned char *dir0, unsigned char *dir1)
 {
 	unsigned char ccr2, ccr3;
 	unsigned long flags;
@@ -52,25 +52,25 @@ static void __init do_cyrix_devid(unsign
  * Actually since bugs.h doesn't even reference this perhaps someone should
  * fix the documentation ???
  */
-static unsigned char Cx86_dir0_msb __initdata = 0;
+static unsigned char Cx86_dir0_msb __cpuinitdata = 0;
 
-static char Cx86_model[][9] __initdata = {
+static char Cx86_model[][9] __cpuinitdata = {
 	"Cx486", "Cx486", "5x86 ", "6x86", "MediaGX ", "6x86MX ",
 	"M II ", "Unknown"
 };
-static char Cx486_name[][5] __initdata = {
+static char Cx486_name[][5] __cpuinitdata = {
 	"SLC", "DLC", "SLC2", "DLC2", "SRx", "DRx",
 	"SRx2", "DRx2"
 };
-static char Cx486S_name[][4] __initdata = {
+static char Cx486S_name[][4] __cpuinitdata = {
 	"S", "S2", "Se", "S2e"
 };
-static char Cx486D_name[][4] __initdata = {
+static char Cx486D_name[][4] __cpuinitdata = {
 	"DX", "DX2", "?", "?", "?", "DX4"
 };
-static char Cx86_cb[] __initdata = "?.5x Core/Bus Clock";
-static char cyrix_model_mult1[] __initdata = "12??43";
-static char cyrix_model_mult2[] __initdata = "12233445";
+static char Cx86_cb[] __cpuinitdata = "?.5x Core/Bus Clock";
+static char cyrix_model_mult1[] __cpuinitdata = "12??43";
+static char cyrix_model_mult2[] __cpuinitdata = "12233445";
 
 /*
  * Reset the slow-loop (SLOP) bit on the 686(L) which is set by some old
@@ -80,9 +80,10 @@ static char cyrix_model_mult2[] __initda
  * FIXME: our newer udelay uses the tsc. We don't need to frob with SLOP
  */
 
-extern void calibrate_delay(void) __init;
+/* is __devinit, should be __cpuinit in init/calibrate.c ??? */
+extern void calibrate_delay(void) __devinit;
 
-static void __init check_cx686_slop(struct cpuinfo_x86 *c)
+static void __cpuinit check_cx686_slop(struct cpuinfo_x86 *c)
 {
 	unsigned long flags;
 	
@@ -107,7 +108,7 @@ static void __init check_cx686_slop(stru
 }
 
 
-static void __init set_cx86_reorder(void)
+static void __cpuinit set_cx86_reorder(void)
 {
 	u8 ccr3;
 
@@ -122,7 +123,7 @@ static void __init set_cx86_reorder(void
 	setCx86(CX86_CCR3, ccr3);
 }
 
-static void __init set_cx86_memwb(void)
+static void __cpuinit set_cx86_memwb(void)
 {
 	u32 cr0;
 
@@ -137,7 +138,7 @@ static void __init set_cx86_memwb(void)
 	setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x14 );
 }
 
-static void __init set_cx86_inc(void)
+static void __cpuinit set_cx86_inc(void)
 {
 	unsigned char ccr3;
 
@@ -158,7 +159,7 @@ static void __init set_cx86_inc(void)
  *	Configure later MediaGX and/or Geode processor.
  */
 
-static void __init geode_configure(void)
+static void __cpuinit geode_configure(void)
 {
 	unsigned long flags;
 	u8 ccr3, ccr4;
@@ -191,7 +192,7 @@ static struct pci_device_id cyrix_55x0[]
 };
 #endif
 
-static void __init init_cyrix(struct cpuinfo_x86 *c)
+static void __cpuinit init_cyrix(struct cpuinfo_x86 *c)
 {
 	unsigned char dir0, dir0_msn, dir0_lsn, dir1 = 0;
 	char *buf = c->x86_model_id;
@@ -429,7 +430,7 @@ static void cyrix_identify(struct cpuinf
 	generic_identify(c);
 }
 
-static struct cpu_dev cyrix_cpu_dev __initdata = {
+static struct cpu_dev cyrix_cpu_dev __cpuinitdata = {
 	.c_vendor	= "Cyrix",
 	.c_ident 	= { "CyrixInstead" },
 	.c_init		= init_cyrix,
@@ -444,7 +445,7 @@ int __init cyrix_init_cpu(void)
 
 //early_arch_initcall(cyrix_init_cpu);
 
-static struct cpu_dev nsc_cpu_dev __initdata = {
+static struct cpu_dev nsc_cpu_dev __cpuinitdata = {
 	.c_vendor	= "NSC",
 	.c_ident 	= { "Geode by NSC" },
 	.c_init		= init_nsc,
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/nexgen.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/nexgen.c
@@ -10,7 +10,7 @@
  *	to have CPUID. (Thanks to Herbert Oppmann)
  */
  
-static int __init deep_magic_nexgen_probe(void)
+static int __cpuinit deep_magic_nexgen_probe(void)
 {
 	int ret;
 	
@@ -27,12 +27,12 @@ static int __init deep_magic_nexgen_prob
 	return  ret;
 }
 
-static void __init init_nexgen(struct cpuinfo_x86 * c)
+static void __cpuinit init_nexgen(struct cpuinfo_x86 * c)
 {
 	c->x86_cache_size = 256; /* A few had 1 MB... */
 }
 
-static void __init nexgen_identify(struct cpuinfo_x86 * c)
+static void __cpuinit nexgen_identify(struct cpuinfo_x86 * c)
 {
 	/* Detect NexGen with old hypercode */
 	if ( deep_magic_nexgen_probe() ) {
@@ -41,7 +41,7 @@ static void __init nexgen_identify(struc
 	generic_identify(c);
 }
 
-static struct cpu_dev nexgen_cpu_dev __initdata = {
+static struct cpu_dev nexgen_cpu_dev __cpuinitdata = {
 	.c_vendor	= "Nexgen",
 	.c_ident	= { "NexGenDriven" },
 	.c_models = {
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/rise.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/rise.c
@@ -5,7 +5,7 @@
 
 #include "cpu.h"
 
-static void __init init_rise(struct cpuinfo_x86 *c)
+static void __cpuinit init_rise(struct cpuinfo_x86 *c)
 {
 	printk("CPU: Rise iDragon");
 	if (c->x86_model > 2)
@@ -28,7 +28,7 @@ static void __init init_rise(struct cpui
 	set_bit(X86_FEATURE_CX8, c->x86_capability);
 }
 
-static struct cpu_dev rise_cpu_dev __initdata = {
+static struct cpu_dev rise_cpu_dev __cpuinitdata = {
 	.c_vendor	= "Rise",
 	.c_ident	= { "RiseRiseRise" },
 	.c_models = {
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/intel_cacheinfo.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/intel_cacheinfo.c
@@ -152,6 +152,7 @@ static int __cpuinit cpuid4_cache_lookup
 	return 0;
 }
 
+/* __init is safe here */
 static int __init find_num_cache_leaves(void)
 {
 	unsigned int		eax, ebx, ecx, edx;
-- 
Chuck

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
  2006-02-01  4:49 [patch -mm4] i386: __init should be __cpuinit Chuck Ebbert
@ 2006-02-01  5:33 ` Dave Jones
  2006-02-01  8:05   ` Matthew Garrett
  2006-02-02 21:34   ` Pavel Machek
  0 siblings, 2 replies; 9+ messages in thread
From: Dave Jones @ 2006-02-01  5:33 UTC (permalink / raw)
  To: Chuck Ebbert; +Cc: Andrew Morton, Ashok Raj, linux-kernel

On Tue, Jan 31, 2006 at 11:49:43PM -0500, Chuck Ebbert wrote:
 > When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
 > referenced from normal code.
 > 
 > On startup:
 >         arch/i386/kernel/cpu/amd.c::amd_init_cpu():
 >                 cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;        
 >         amd_cpu_dev is declared __initdata and is freed
 > 
 > On CPU hotplug:
 >         arch/i386/kernel/cpu/common.c::get_cpu_vendor():
 >                for (i = 0; i < X86_VENDOR_NUM; i++) {
 >                         if (cpu_devs[i]) {
 >                                 if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
 > 
 > To fix this, change every instance of __init that seems suspicious
 > into __cpuinit.  When !CONFIG_HOTPLUG_CPU there is no change in .text
 > or .data size.  When enabled, .text += 3248 bytes; .data += 2148 bytes.
 > 
 > This should be safe in every case; the only drawback is the extra code and
 > data when CPU hotplug is enabled.

Especially as for the bulk of them, those CPUs aren't hotplug capable.
(I seriously doubt we'll ever see a hotplugable cyrix for eg, which
 takes up the bulk of your diff).

How about leaving it __init on non-hotplug systems, and somehow removing
those from cpu_devs, so get_cpu_vendor() just skips them ?
NULL'ing those entries should be just a few bytes, instead of adding 5KB.

		Dave


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
  2006-02-01  5:33 ` Dave Jones
@ 2006-02-01  8:05   ` Matthew Garrett
  2006-02-01 16:03     ` Dave Jones
  2006-02-02 21:34   ` Pavel Machek
  1 sibling, 1 reply; 9+ messages in thread
From: Matthew Garrett @ 2006-02-01  8:05 UTC (permalink / raw)
  To: Dave Jones; +Cc: Andrew Morton, Ashok Raj, linux-kernel, Chuck Ebbert

Dave Jones <davej@redhat.com> wrote:

> Especially as for the bulk of them, those CPUs aren't hotplug capable.
> (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
>  takes up the bulk of your diff).

For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
executing the suspend code. I don't recall any SMP cyrix systems, but
it's potentially something to consider.

-- 
Matthew Garrett | mjg59-chiark.mail.linux-rutgers.kernel@srcf.ucam.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
  2006-02-01  8:05   ` Matthew Garrett
@ 2006-02-01 16:03     ` Dave Jones
  2006-02-01 18:56       ` Alan Cox
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Jones @ 2006-02-01 16:03 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Andrew Morton, Ashok Raj, linux-kernel, Chuck Ebbert

On Wed, Feb 01, 2006 at 08:05:51AM +0000, Matthew Garrett wrote:
 > Dave Jones <davej@redhat.com> wrote:
 > 
 > > Especially as for the bulk of them, those CPUs aren't hotplug capable.
 > > (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
 > >  takes up the bulk of your diff).
 > 
 > For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
 > executing the suspend code. I don't recall any SMP cyrix systems, but
 > it's potentially something to consider.

There weren't any.  Until AMD's Athlon MPs, Intel had the only
SMP x86 afair.

		Dave


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
  2006-02-01 16:03     ` Dave Jones
@ 2006-02-01 18:56       ` Alan Cox
  2006-02-01 19:15         ` Ashok Raj
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Cox @ 2006-02-01 18:56 UTC (permalink / raw)
  To: Dave Jones
  Cc: Matthew Garrett, Andrew Morton, Ashok Raj, linux-kernel, Chuck Ebbert

On Mer, 2006-02-01 at 11:03 -0500, Dave Jones wrote:
>  > For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
>  > executing the suspend code. I don't recall any SMP cyrix systems, but
>  > it's potentially something to consider.
> 
> There weren't any.  Until AMD's Athlon MPs, Intel had the only
> SMP x86 afair.

Several vendors demonstrated OpenMP designs including Cyrix. Nothing
production and nothing we support.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
  2006-02-01 18:56       ` Alan Cox
@ 2006-02-01 19:15         ` Ashok Raj
  0 siblings, 0 replies; 9+ messages in thread
From: Ashok Raj @ 2006-02-01 19:15 UTC (permalink / raw)
  To: Alan Cox
  Cc: Dave Jones, Matthew Garrett, Andrew Morton, Ashok Raj,
	linux-kernel, Chuck Ebbert

On Wed, Feb 01, 2006 at 06:56:22PM +0000, Alan Cox wrote:
> On Mer, 2006-02-01 at 11:03 -0500, Dave Jones wrote:
> >  > For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
> >  > executing the suspend code. I don't recall any SMP cyrix systems, but
> >  > it's potentially something to consider.
> > 
> > There weren't any.  Until AMD's Athlon MPs, Intel had the only
> > SMP x86 afair.
> 
> Several vendors demonstrated OpenMP designs including Cyrix. Nothing
> production and nothing we support.

In order to support logical cpu hotplug, you dont need any special hw, as long
as you has some SMP box. Sometimes its just a little bit more than 
moving __init to __cpuinit. A lot of these references were not changed
just speculatively to ensure when the need is there, someone will change and
test those as well.

Chuck, would it be possible for you also test cpu hotplug for the ones you 
are interested in and accompany the related changes if there are more
as separate patches? (Documentation now has a howto on cpu hotplug)

That would make it more useful for this exersise.

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
  2006-02-01  5:33 ` Dave Jones
  2006-02-01  8:05   ` Matthew Garrett
@ 2006-02-02 21:34   ` Pavel Machek
  2006-02-02 21:39     ` Dave Jones
  1 sibling, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2006-02-02 21:34 UTC (permalink / raw)
  To: Dave Jones, Chuck Ebbert, Andrew Morton, Ashok Raj, linux-kernel

Hi!

>  > When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
>  > referenced from normal code.
>  > 
>  > On startup:
>  >         arch/i386/kernel/cpu/amd.c::amd_init_cpu():
>  >                 cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;        
>  >         amd_cpu_dev is declared __initdata and is freed
>  > 
>  > On CPU hotplug:
>  >         arch/i386/kernel/cpu/common.c::get_cpu_vendor():
>  >                for (i = 0; i < X86_VENDOR_NUM; i++) {
>  >                         if (cpu_devs[i]) {
>  >                                 if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
>  > 
>  > To fix this, change every instance of __init that seems suspicious
>  > into __cpuinit.  When !CONFIG_HOTPLUG_CPU there is no change in .text
>  > or .data size.  When enabled, .text += 3248 bytes; .data += 2148 bytes.
>  > 
>  > This should be safe in every case; the only drawback is the extra code and
>  > data when CPU hotplug is enabled.
> 
> Especially as for the bulk of them, those CPUs aren't hotplug capable.
> (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
>  takes up the bulk of your diff).
> 
> How about leaving it __init on non-hotplug systems, and somehow removing
> those from cpu_devs, so get_cpu_vendor() just skips them ?
> NULL'ing those entries should be just a few bytes, instead of adding 5KB.

We use cpu hotplug system for swsusp; but unless someone makes
cyrix/SMP machine and tries to suspend it, we are ok.

							Pavel
-- 
Thanks, Sharp!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
  2006-02-02 21:34   ` Pavel Machek
@ 2006-02-02 21:39     ` Dave Jones
  0 siblings, 0 replies; 9+ messages in thread
From: Dave Jones @ 2006-02-02 21:39 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Chuck Ebbert, Andrew Morton, Ashok Raj, linux-kernel

On Thu, Feb 02, 2006 at 10:34:50PM +0100, Pavel Machek wrote:
 > Hi!
 > 
 > >  > When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
 > >  > referenced from normal code.
 > >  > 
 > >  > On startup:
 > >  >         arch/i386/kernel/cpu/amd.c::amd_init_cpu():
 > >  >                 cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;        
 > >  >         amd_cpu_dev is declared __initdata and is freed
 > >  > 
 > >  > On CPU hotplug:
 > >  >         arch/i386/kernel/cpu/common.c::get_cpu_vendor():
 > >  >                for (i = 0; i < X86_VENDOR_NUM; i++) {
 > >  >                         if (cpu_devs[i]) {
 > >  >                                 if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
 > >  > 
 > >  > To fix this, change every instance of __init that seems suspicious
 > >  > into __cpuinit.  When !CONFIG_HOTPLUG_CPU there is no change in .text
 > >  > or .data size.  When enabled, .text += 3248 bytes; .data += 2148 bytes.
 > >  > 
 > >  > This should be safe in every case; the only drawback is the extra code and
 > >  > data when CPU hotplug is enabled.
 > > 
 > > Especially as for the bulk of them, those CPUs aren't hotplug capable.
 > > (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
 > >  takes up the bulk of your diff).
 > > 
 > > How about leaving it __init on non-hotplug systems, and somehow removing
 > > those from cpu_devs, so get_cpu_vendor() just skips them ?
 > > NULL'ing those entries should be just a few bytes, instead of adding 5KB.
 > 
 > We use cpu hotplug system for swsusp; but unless someone makes
 > cyrix/SMP machine and tries to suspend it, we are ok.

As Alan mentioned, there were never any production SMP cyrix machines,
and the prototypes never worked on Linux anyway.  With Cyrix no longer
around this code is safe to assume 'UP only'.

		Dave

 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [patch -mm4] i386: __init should be __cpuinit
@ 2006-02-01  8:03 Chuck Ebbert
  0 siblings, 0 replies; 9+ messages in thread
From: Chuck Ebbert @ 2006-02-01  8:03 UTC (permalink / raw)
  To: Dave Jones; +Cc: linux-kernel, Ashok Raj, Andrew Morton

In-Reply-To: <20060201053357.GA5335@redhat.com>

On Wed, 1 Feb 2006 at 00:33:57 -0500, Dave Jones wrote:

> On Tue, Jan 31, 2006 at 11:49:43PM -0500, Chuck Ebbert wrote:
>  > To fix this, change every instance of __init that seems suspicious
>  > into __cpuinit.  When !CONFIG_HOTPLUG_CPU there is no change in .text
>  > or .data size.  When enabled, .text += 3248 bytes; .data += 2148 bytes.
>  > 
>  > This should be safe in every case; the only drawback is the extra code and
>  > data when CPU hotplug is enabled.
>
> How about leaving it __init on non-hotplug systems, and somehow removing
> those from cpu_devs, so get_cpu_vendor() just skips them ?
> NULL'ing those entries should be just a few bytes, instead of adding 5KB.

That's what I wanted to do but wasn't sure how.  Maybe e.g. like this?

--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/umc.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/umc.c
@@ -5,12 +5,12 @@
 
 /* UMC chips appear to be only either 386 or 486, so no special init takes place.
  */
-static void __cpuinit init_umc(struct cpuinfo_x86 * c)
+static void __init init_umc(struct cpuinfo_x86 * c)
 {
 
 }
 
-static struct cpu_dev umc_cpu_dev __cpuinitdata = {
+static struct cpu_dev umc_cpu_dev __initdata = {
 	.c_vendor	= "UMC",
 	.c_ident 	= { "UMC UMC UMC" },
 	.c_models = {
@@ -31,3 +31,11 @@ int __init umc_init_cpu(void)
 }
 
 //early_arch_initcall(umc_init_cpu);
+
+int __init umc_exit_cpu(void)
+{
+	cpu_devs[X86_VENDOR_UMC] = NULL;
+	return 0;
+}
+
+late_initcall(umc_exit_cpu);
-- 
Chuck

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2006-02-02 21:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-02-01  4:49 [patch -mm4] i386: __init should be __cpuinit Chuck Ebbert
2006-02-01  5:33 ` Dave Jones
2006-02-01  8:05   ` Matthew Garrett
2006-02-01 16:03     ` Dave Jones
2006-02-01 18:56       ` Alan Cox
2006-02-01 19:15         ` Ashok Raj
2006-02-02 21:34   ` Pavel Machek
2006-02-02 21:39     ` Dave Jones
2006-02-01  8:03 Chuck Ebbert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).