Re: [PATCH] x86/apic: Use byte array apic_version[], not int array. Saves up to 96k

From: Denys Vlasenko
Date: Tue Sep 13 2016 - 14:07:51 EST




On 09/13/2016 05:33 PM, Thomas Gleixner wrote:
On Sun, 11 Sep 2016, Borislav Petkov wrote:
On Fri, Sep 09, 2016 at 10:32:04AM +0200, Denys Vlasenko wrote:
This array is [MAX_LOCAL_APIC], and MAX_LOCAL_APIC can easily be up to 32k.

This patch changes apic_version[] array elements from int to u8 -
APIC version values as of year 2016 are no larger than 0x1f on all known CPUs.
Version field in the APIC register is 8 bit wide - not likely
to overflow byte range in foreseeable future.

The "ver" argument of generic_processor_info(id,ver), which goes into apic_version[id],
is also changed from int to u8: make it obvious that assignment can't overflow.

generic_processor_info() has four callsites, none of them can put an out-of-range value
into this argument.

Right, so I dug a bit into this and found:

http://marc.info/?l=linux-kernel&m=123230551709711

and

b2b815d80a5c ("x86: put trigger in to detect mismatched apic versions")

It is from 2009 and I don't know how relevant 16-bit APIC IDs are
anymore... I guess you probably want to run this by SGI folk first.

Otherwise I was going to propose to kill that apic_version array
altogether and cache only the version of the previous CPU and compare it
to the current one to catch mismatches...

Yeah, the idea was back then to eliminate the array, but we wanted to make
sure that we don't have systems out in the wild which have different apic
versions. I really doubt that we can deal with that proper, so having a
single version entry and yelling loudly when we detect a mismatch is good
enough.

Makes sense. I'll send a patch