linux-kernel - Re: [RFC][PATCH] x86/smp: Fix __max_logical

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160810141538.GA28551@krava>
Date:	Wed, 10 Aug 2016 16:15:38 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Andi Kleen <andi@...stfloor.org>, linux-kernel@...r.kernel.org,
	Andi Kleen <ak@...ux.intel.com>, x86@...nel.org,
	Ingo Molnar <mingo@...nel.org>,
	Frank Ramsay <framsay@...hat.com>,
	Prarit Bhargava <prarit@...hat.com>
Subject: Re: [RFC][PATCH] x86/smp: Fix __max_logical_packages value setup

On Wed, Aug 10, 2016 at 04:00:33PM +0200, Jiri Olsa wrote:
> On Wed, Aug 10, 2016 at 03:54:17PM +0200, Peter Zijlstra wrote:
> > On Wed, Aug 03, 2016 at 06:23:58PM +0200, Jiri Olsa wrote:
> 
> SNIP
> 
> > > ---
> > > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> > > index 157bf0957219..484f7d357c77 100644
> > > --- a/arch/x86/kernel/smpboot.c
> > > +++ b/arch/x86/kernel/smpboot.c
> > > @@ -340,7 +340,7 @@ static void __init smp_init_package_map(void)
> > >  		ncpus = 1;
> > >  	}
> > >  
> > > -	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
> > > +	__max_logical_packages = total_cpus;
> > 
> > This seems undesirable.. it would grow the bitmap unnecessarily big on
> > most setups.
> > 
> > Is there no way to detect the brain damage inflicted by that BIOS option
> > and fudge ncpus in that case?
> 
> yea, I have no idea.. that's why this is RFC ;-)
> 
> maybe we could also gradually allocate this bitmap
> in topology_update_package_map.. I'll check on that

it's very likely I'm missing something, but seems to me
that attached patch (untested) might work as well

jirka


---
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 157bf0957219..003208bc8e44 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -100,7 +100,6 @@ EXPORT_PER_CPU_SYMBOL(cpu_info);
 /* Logical package management. We might want to allocate that dynamically */
 static int *physical_to_logical_pkg __read_mostly;
 static unsigned long *physical_package_map __read_mostly;;
-static unsigned long *logical_package_map  __read_mostly;
 static unsigned int max_physical_pkg_id __read_mostly;
 unsigned int __max_logical_packages __read_mostly;
 EXPORT_SYMBOL(__max_logical_packages);
@@ -277,14 +276,8 @@ int topology_update_package_map(unsigned int apicid, unsigned int cpu)
 	if (test_and_set_bit(pkg, physical_package_map))
 		goto found;
 
-	new = find_first_zero_bit(logical_package_map, __max_logical_packages);
-	if (new >= __max_logical_packages) {
-		physical_to_logical_pkg[pkg] = -1;
-		pr_warn("APIC(%x) Package %u exceeds logical package map\n",
-			apicid, pkg);
-		return -ENOSPC;
-	}
-	set_bit(new, logical_package_map);
+	new = __max_logical_packages++;
+
 	pr_info("APIC(%x) Converting physical %u to logical package %u\n",
 		apicid, pkg, new);
 	physical_to_logical_pkg[pkg] = new;
@@ -340,7 +333,7 @@ static void __init smp_init_package_map(void)
 		ncpus = 1;
 	}
 
-	__max_logical_packages = DIV_ROUND_UP(total_cpus, ncpus);
+	__max_logical_packages = 0;
 
 	/*
 	 * Possibly larger than what we need as the number of apic ids per
@@ -352,10 +345,6 @@ static void __init smp_init_package_map(void)
 	memset(physical_to_logical_pkg, 0xff, size);
 	size = BITS_TO_LONGS(max_physical_pkg_id) * sizeof(unsigned long);
 	physical_package_map = kzalloc(size, GFP_KERNEL);
-	size = BITS_TO_LONGS(__max_logical_packages) * sizeof(unsigned long);
-	logical_package_map = kzalloc(size, GFP_KERNEL);
-
-	pr_info("Max logical packages: %u\n", __max_logical_packages);
 
 	for_each_present_cpu(cpu) {
 		unsigned int apicid = apic->cpu_present_to_apicid(cpu);