Fix some issues in the thread cap logic

This commit is contained in:
Panu Matilainen 2019-08-27 14:25:07 +03:00
parent dce71ded33
commit 48ff1fa7e0
2 changed files with 47 additions and 23 deletions

View File

@ -1,10 +1,8 @@
From 4c772fcf7114d6ce20800c12bc377b490d267548 Mon Sep 17 00:00:00 2001 From fa70eaa852aab38857b3c8663386e8759f121bbe Mon Sep 17 00:00:00 2001
Message-Id: <4c772fcf7114d6ce20800c12bc377b490d267548.1566558648.git.pmatilai@redhat.com> Message-Id: <fa70eaa852aab38857b3c8663386e8759f121bbe.1566904946.git.pmatilai@redhat.com>
In-Reply-To: <4d16d81a1af3e43b8392e7c335f616c9c0c6e41b.1566558648.git.pmatilai@redhat.com>
References: <4d16d81a1af3e43b8392e7c335f616c9c0c6e41b.1566558648.git.pmatilai@redhat.com>
From: Panu Matilainen <pmatilai@redhat.com> From: Panu Matilainen <pmatilai@redhat.com>
Date: Fri, 23 Aug 2019 11:17:21 +0300 Date: Fri, 23 Aug 2019 11:17:21 +0300
Subject: [PATCH 2/2] Cap number of threads on 32bit platforms, add a tunable Subject: [PATCH] Cap number of threads on 32bit platforms, add tunables
(RhBug:1729382) (RhBug:1729382)
On 32bit plaforms, address space is easily exhausted with multiple On 32bit plaforms, address space is easily exhausted with multiple
@ -13,44 +11,67 @@ Simply cap the number of threads to maximum of four on any 32bit
platform to play it safe. It's still three more than we were able to platform to play it safe. It's still three more than we were able to
use on older releases... use on older releases...
In addition, introduce a separate tunable for tweaking the maximum In addition, introduce tunables for controlling the number of threads
number of threads just in case, defaulting to max CPUs available. used similarly to what is available for number of CPUs: forced number
and a separate ceiling.
The max logic gets surprisingly tricky with the mixed OMP and rpm
tunables and different semantics. Notably, OMP does not have a nice
way to set a maximum from inside a program (this is only possible
from an environment variable so we can't use it here), so we need
to jump through a lot of hoops to figure out whether an *unlimited*
setting would go above the platform limitation. I'm not entirely
convinced this is 100% bulletproof, but then it's all going to change
and become even more complicated when we start taking memory limitations
into account...
--- ---
build/parseSpec.c | 10 +++++++++- build/parseSpec.c | 20 +++++++++++++++++---
platform.in | 3 +++ platform.in | 5 +++++
2 files changed, 12 insertions(+), 1 deletion(-) 2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/build/parseSpec.c b/build/parseSpec.c diff --git a/build/parseSpec.c b/build/parseSpec.c
index 737a1233c..ab3ba9cae 100644 index 737a1233c..ed5c3ef63 100644
--- a/build/parseSpec.c --- a/build/parseSpec.c
+++ b/build/parseSpec.c +++ b/build/parseSpec.c
@@ -1035,7 +1035,15 @@ static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags, @@ -1035,9 +1035,23 @@ static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags,
#ifdef ENABLE_OPENMP #ifdef ENABLE_OPENMP
/* Set number of OMP threads centrally */ /* Set number of OMP threads centrally */
- int ncpus = rpmExpandNumeric("%{?_smp_build_ncpus}"); - int ncpus = rpmExpandNumeric("%{?_smp_build_ncpus}");
+ int ncpus = rpmExpandNumeric("%{?_smp_nthreads_max}"); - if (ncpus > 0)
- omp_set_num_threads(ncpus);
+ int nthreads = rpmExpandNumeric("%{?_smp_build_nthreads}");
+ int nthreads_max = rpmExpandNumeric("%{?_smp_nthreads_max}");
+ if (nthreads <= 0)
+ nthreads = omp_get_max_threads();
+ if (nthreads_max > 0 && nthreads > nthreads_max)
+ nthreads = nthreads_max;
+#if __WORDSIZE == 32 +#if __WORDSIZE == 32
+ /* On 32bit platforms, address space shortage is an issue. Play safe. */ + /* On 32bit platforms, address space shortage is an issue. Play safe. */
+ if (ncpus <= 0 || ncpus > 4) { + int platlimit = 4;
+ ncpus = 4; + if (nthreads > platlimit) {
+ nthreads = platlimit;
+ rpmlog(RPMLOG_DEBUG, + rpmlog(RPMLOG_DEBUG,
+ "limiting number of threads to %d due to platform\n", ncpus); + "limiting number of threads to %d due to platform\n", platlimit);
+ } + }
+#endif +#endif
if (ncpus > 0) + if (nthreads > 0)
omp_set_num_threads(ncpus); + omp_set_num_threads(nthreads);
#endif #endif
if (spec->clean == NULL) {
diff --git a/platform.in b/platform.in diff --git a/platform.in b/platform.in
index db6d2382f..c70f44193 100644 index db6d2382f..61f5e18a0 100644
--- a/platform.in --- a/platform.in
+++ b/platform.in +++ b/platform.in
@@ -59,6 +59,9 @@ @@ -59,6 +59,11 @@
%_smp_mflags -j%{_smp_build_ncpus} %_smp_mflags -j%{_smp_build_ncpus}
+# Maximum number of CPU's to use for threads +# Maximum number of threads to use when building, 0 for unlimited
+%_smp_nthreads_max %{_smp_build_ncpus} +#%_smp_nthreads_max 0
+
+%_smp_build_nthreads %{_smp_build_ncpus}
+ +
#============================================================================== #==============================================================================
# ---- Build policy macros. # ---- Build policy macros.

View File

@ -21,7 +21,7 @@
%global rpmver 4.15.0 %global rpmver 4.15.0
%global snapver beta %global snapver beta
%global rel 5 %global rel 6
%global srcver %{version}%{?snapver:-%{snapver}} %global srcver %{version}%{?snapver:-%{snapver}}
%global srcdir %{?snapver:testing}%{!?snapver:%{name}-%(echo %{version} | cut -d'.' -f1-2).x} %global srcdir %{?snapver:testing}%{!?snapver:%{name}-%(echo %{version} | cut -d'.' -f1-2).x}
@ -547,6 +547,9 @@ make check || (cat tests/rpmtests.log; exit 0)
%doc doc/librpm/html/* %doc doc/librpm/html/*
%changelog %changelog
* Tue Aug 27 2019 Panu Matilainen <pmatilai@redhat.com> - 4.15.0-0.beta.6
- Fix some issues in the thread cap logic
* Mon Aug 26 2019 Panu Matilainen <pmatilai@redhat.com> - 4.15.0-0.beta.5 * Mon Aug 26 2019 Panu Matilainen <pmatilai@redhat.com> - 4.15.0-0.beta.5
- Re-enable test-suite, temporarily disabled during alpha troubleshooting - Re-enable test-suite, temporarily disabled during alpha troubleshooting