x86_64: MAX_LOCAL_APIC way too big?

From: Denys Vlasenko
Date: Fri Sep 25 2015 - 12:16:12 EST


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@xxxxxxx>
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/