On 2015/9/26 0:16, Denys Vlasenko wrote:
> For 64-bit kernels, MAX_LOCAL_APIC is 32k:
>
> #ifdef CONFIG_X86_32
> ...
> #else
> # define MAX_IO_APICS 128
> # define MAX_LOCAL_APIC 32768
> #endif
>
> (It seems to be a bit of a misnomer, it's not a maximum
> number of APICs we support, it's the highest APIC _id_
> we support.)
>
> 32 thousand APICs? That's a lot. Especially
> considering that event with CONFIG_MAXSMP=y,
> NR_CPUS is "only" 8096.
>
> After a quick glance through code, it looks like
> such a big value causes several data arrays to be
> quite oversized:
>
> phys_cpu_present_map is 4 kbytes (one bit per apicid)
> __apicid_to_node is 64 kbytes
> apic_version is 128 kbytes (!!!)
>
> This is where they are defined:
>
> include/asm/mpspec.h
> #define PHYSID_ARRAY_SIZE BITS_TO_LONGS(MAX_LOCAL_APIC)
> struct physid_mask {
> unsigned long mask[PHYSID_ARRAY_SIZE];
> };
> typedef struct physid_mask physid_mask_t;
> apic/apic.c
> physid_mask_t phys_cpu_present_map;
>
> mm/numa.c:
> s16 __apicid_to_node[MAX_LOCAL_APIC] = {
> [0 ... MAX_LOCAL_APIC-1] = NUMA_NO_NODE
> };
>
> apic/apic.c:
> int apic_version[MAX_LOCAL_APIC];
>
>
> Maybe we can reduce MAX_LOCAL_APIC?
> Why it has to be this big in the first place?
>
> IIRC: APIC id at first was just a 8-bit quantity,
> then x2apic mode it was extended to 32 bits.
>
> On "usual" systems, apic ids simply go from zero
> to maximum logical CPU number, mirroring CPU ids.
Hi Denys,
The above assumption is risky with modern
x86 platforms. APIC ids are assigned by firmware,
and may be discrete.
>
> I imagine there are broken/unusual systems in the wild
> where apic ids are non-contiguous for some reason.
> IOW: even if NR_CPUS is low (say, 4), setting
> MAX_LOCAL_APIC ot 4 is not a good idea.
>
> I propose the following:
>
> /* For small SMP, allow non-contiguous APIC IDs.
> * We expect that big SMP machines have
> * non-broken APIC id numbering.
> */
> # define MAX_LOCAL_APIC (NR_CPUS > 256 ? NR_CPUS : 256)
>
> If this looks too risky, then how about this?
>
> # define MAX_LOCAL_APIC (NR_CPUS > 256/2 ? NR_CPUS*2 : 256)
>
>
> Why 256? We have this code in two places:
> apic_version[new_apicid] =
> GET_APIC_VERSION(apic_read(APIC_LVR));
> and
> #define GET_APIC_VERSION(x) ((x) & 0xFFu)
> IOW: apic_version[x] needs to be valid for any 8-bit x.
The above code just assume that the value stored in apic_version[]
are 8-bit. It has no explicit relationship with apic id.
Thanks!
Gerry
>
> In one place, we need to add a check that index is not out-of-bounds:
> int generic_processor_info(int apicid, int version)
> {
> ...
> apic_version[apicid] = version;
>
>
>
> MAX_IO_APICS is a bit big too, this ioapics[] array
> uses 9216 bytes:
>
> apic/io_apic.c
> static struct ioapic {
> /*
> * # of IRQ routing registers
> */
> int nr_registers;
> /*
> * Saved state during suspend/resume, or while enabling
> intr-remap.
> */
> struct IO_APIC_route_entry *saved_registers;
> /* I/O APIC config */
> struct mpc_ioapic mp_config;
> /* IO APIC gsi routing info */
> struct mp_ioapic_gsi gsi_config;
> struct ioapic_domain_cfg irqdomain_cfg;
> struct irq_domain *irqdomain;
> struct resource *iomem_res;
> } ioapics[MAX_IO_APICS];
>
> Do we really expect to have machines with 128 IOAPICS?
> We use this value even if NR_CPUS is, say, only 4.
>
> I propose:
>
> /* Minimum is 8.
> * For largish NR_CPUS, we expect to have fewer IOAPICS than CPUs.
> * No matter how big NR_CPUS is, cap IOAPICs count at 128.
> */
> # define MAX_IO_APICS (NR_CPUS < 8 ? 8 : (NR_CPUS < 128 ? NR_CPUS : 128))
>
>
> FYI: MAX_LOCAL_APIC was changed by this commit:
>
> commit a65d1d644c2b65bfb99e766e7160d764b8b2bfa4
> Author: Jack Steiner <stei...@sgi.com>
> Date: Fri Mar 28 14:12:08 2008 -0500
>
> x86: increase size of APICID
>
> Increase the number of bits in an apicid from 8 to 32.
>
> By default, MP_processor_info() gets the APICID from the
> mpc_config_processor structure. However, this structure limits
> the size of APICID to 8 bits. This patch allows the caller of
> MP_processor_info() to optionally pass a larger APICID that will
> be used instead of the one in the mpc_config_processor struct.
> ...
>
> #else
> # define MAX_IO_APICS 128
> -# define MAX_LOCAL_APIC 256
> +# define MAX_LOCAL_APIC 32768
> #endif
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
