Messages in this thread | | | Date | Fri, 25 Sep 2015 18:16:04 +0200 | From | Denys Vlasenko <> | Subject | x86_64: MAX_LOCAL_APIC way too big? |
| |
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.
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.
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 <steiner@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
| |