| Message ID | 20200820170624.1855825-1-Yazen.Ghannam@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/mce: Increase maximum number of banks to 64 | expand |
On Thu, Aug 20, 2020 at 05:06:24PM +0000, Yazen Ghannam wrote: > From: Akshay Gupta <Akshay.Gupta@amd.com> > > ...because future AMD systems will support up to 64 MCA banks per CPU. > > MAX_NR_BANKS is used to allocate a number of data structures, and it is > used as a ceiling for values read from MCG_CAP[Count]. Therefore, this > change will have no functional effect on existing systems with 32 or > fewer MCA banks per CPU. Of course it will, grep for MAX_NR_BANKS and look at all those bitmaps and arrays which get defined with MAX_NR_BANKS size. With your change, they will double in size. How much does vmlinux size grow with your change?
On Thu, Aug 20, 2020 at 07:15:18PM +0200, Borislav Petkov wrote: > On Thu, Aug 20, 2020 at 05:06:24PM +0000, Yazen Ghannam wrote: > > From: Akshay Gupta <Akshay.Gupta@amd.com> > > > > ...because future AMD systems will support up to 64 MCA banks per CPU. > > > > MAX_NR_BANKS is used to allocate a number of data structures, and it is > > used as a ceiling for values read from MCG_CAP[Count]. Therefore, this > > change will have no functional effect on existing systems with 32 or > > fewer MCA banks per CPU. > > Of course it will, grep for MAX_NR_BANKS and look at all those bitmaps > and arrays which get defined with MAX_NR_BANKS size. With your change, > they will double in size. > > How much does vmlinux size grow with your change? > It seems to get smaller. -rwxrwxr-x 1 yghannam yghannam 807634088 Aug 20 17:51 vmlinux-32banks -rwxrwxr-x 1 yghannam yghannam 807634072 Aug 20 17:50 vmlinux-64banks Any ideas? Maybe there's some alignment change? Or a build issue on my end? Thanks, Yazen
>> How much does vmlinux size grow with your change? >> > > It seems to get smaller. > > -rwxrwxr-x 1 yghannam yghannam 807634088 Aug 20 17:51 vmlinux-32banks > -rwxrwxr-x 1 yghannam yghannam 807634072 Aug 20 17:50 vmlinux-64banks You need to run: $ size vmlinux text data bss dec hex filename 20334755 12569682 14798924 47703361 2d7e541 vmlinux Likely the extra space is added to the third element ("bss"). That doesn't show up in the vmlinux file, but does add to memory footprint while running. -Tony
On Thu, Aug 20, 2020 at 06:15:15PM +0000, Luck, Tony wrote: > >> How much does vmlinux size grow with your change? > >> > > > > It seems to get smaller. > > > > -rwxrwxr-x 1 yghannam yghannam 807634088 Aug 20 17:51 vmlinux-32banks > > -rwxrwxr-x 1 yghannam yghannam 807634072 Aug 20 17:50 vmlinux-64banks > > You need to run: > > $ size vmlinux > text data bss dec hex filename > 20334755 12569682 14798924 47703361 2d7e541 vmlinux > > Likely the extra space is added to the third element ("bss"). That doesn't show > up in the vmlinux file, but does add to memory footprint while running. Thanks. Yeah, they're identical: text data bss dec hex filename 15710076 13519306 5398528 34627910 2106146 vmlinux-32banks 15710076 13519306 5398528 34627910 2106146 vmlinux-64banks I did a quick audit of the statically allocated data structures which use MAX_NR_BANKS. Global bitmaps: - core.c / mce_banks_ce_disabled - core.c / all_banks - core.c / valid_banks - core.c / toclear - Total: 32 new bits * 4 bitmaps = 16 new bytes Per-CPU bitmaps: - core.c / mce_poll_banks - intel.c / mce_banks_owned - Total: 32 new bits * 2 bitmaps = 8 new bytes The bitmaps are arrays of longs. So this change will only affect 32-bit execution (I assume), since there will be one additional long used. There will be no additional memory use on 64-bit execution, because the size of long is 64 bits. Global structs: - amd.c / struct smca_bank smca_banks[]: 16 bytes per bank - core.c / struct mce_bank_dev mce_bank_devs[]: 56 bytes per bank - Total: 32 new banks * (16 + 56) bytes = 2304 new bytes Per-CPU structs: - core.c / struct mce_bank mce_banks_array[]: 16 bytes per bank - Total: 32 new banks * 16 bytes = 512 new bytes 32-bit Total global size increase: 2320 bytes Total per-CPU size increase: 520 bytes 64-bit Total global size increase: 2304 bytes Total per-CPU size increase: 512 bytes Is this okay? Thanks, Yazen
On Mon, Aug 24, 2020 at 10:11:05AM -0500, Yazen Ghannam wrote: > Thanks. Yeah, they're identical: > text data bss dec hex filename > 15710076 13519306 5398528 34627910 2106146 vmlinux-32banks > 15710076 13519306 5398528 34627910 2106146 vmlinux-64banks So I went crazy here and built allmodconfigs for hypothetical CPUs which support up to 4K MCA banks: text data bss dec hex filename 24618040 27211906 35409988 87239934 5332cfe vmlinux-32banks 24618040 27211906 35409988 87239934 5332cfe vmlinux-64banks 24618040 27211906 35409988 87239934 5332cfe vmlinux-128banks For the first three - up to 128 banks, the sizes don't change. 24618000 27216002 35401796 87235798 5331cd6 vmlinux-256banks Somewhere after 128 banks, it grows the data section but by exactly 4K, i.e., one more page: 27216002 - 27211906 = 4096 The last page is not full, of course. 27216002 % 4096 = 2178 27211906 % 4096 = 2178 24618000 27220098 35401796 87239894 5332cd6 vmlinux-512banks The same happens here. 27220098 - 27216002 = 4096 27220098 % 4096 = 2178 And so on: 24618000 27228290 35393604 87239894 5332cd6 vmlinux-1024banks 24618000 27244674 35377220 87239894 5332cd6 vmlinux-2048banks 24618000 27277442 35344452 87239894 5332cd6 vmlinux-4Kbanks So what I think happens is, the first 128 banks fit within the same .data section page thus vmlinux doesn't increase. I guess there might be a .config which could cause that data increase when the 32->64 banks move won't fit in the last .data page. It depends on where those ~2K you computed and I snipped, land. But since most kernels are allmodconfig builds, I guess we should be fine. So yeah, I guess we can take this one after you summarize the findings of this thread in v2's commit message. :-) Thx.
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index 6adced6e7dd3..109af5c7f515 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -200,12 +200,8 @@ void mce_setup(struct mce *m); void mce_log(struct mce *m); DECLARE_PER_CPU(struct device *, mce_device); -/* - * Maximum banks number. - * This is the limit of the current register layout on - * Intel CPUs. - */ -#define MAX_NR_BANKS 32 +/* Maximum number of MCA banks per CPU. */ +#define MAX_NR_BANKS 64 #ifdef CONFIG_X86_MCE_INTEL void mce_intel_feature_init(struct cpuinfo_x86 *c);