From: Denys Vlasenko <dvlasenk@redhat.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Jiang Liu <jiang.liu@linux.intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Len Brown <len.brown@intel.com>, X86 ML <x86@kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: x86_64: MAX_LOCAL_APIC way too big?
Date: Fri, 25 Sep 2015 18:16:04 +0200 [thread overview]
Message-ID: <56057344.2070206@redhat.com> (raw)
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
next reply other threads:[~2015-09-25 16:16 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-25 16:16 Denys Vlasenko [this message]
2015-09-25 16:56 ` x86_64: MAX_LOCAL_APIC way too big? Jiang Liu
2015-09-25 19:44 ` Denys Vlasenko
2015-09-26 6:08 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56057344.2070206@redhat.com \
--to=dvlasenk@redhat.com \
--cc=jiang.liu@linux.intel.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.