Re: [patch -mm4] i386: __init should be __cpuinit

[Dave Jones <davej@redhat.com>]

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