Change in vdsm[ovirt-3.6]: lib: daemon: autodetect online cpus for affinity

fromani at redhat.com fromani at redhat.com
Thu Dec 3 08:08:26 UTC 2015


Hello Piotr Kliczewski,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/49612

to review the following change.

Change subject: lib: daemon: autodetect online cpus for affinity
......................................................................

lib: daemon: autodetect online cpus for affinity

In commit a1d4e23 we enabled by default the cpu affinity
for VDSM, after we had confirmation this delivered important
performance benefits.

An hardcoded default setting for any cpu core is a too naive
approach, as it could break automated setups if the said CPU
is offline. And Vdsm cannot guess that.

To solve this, we add a function to learn from the kernel which
cpus are online, using a file from /sys pseudo-filesystem.
This is a simplified version of the same approach libvirt uses.
We don't leverage libvirt because it is too early in the
VDSM startup.

The layout of /sys, like /proc, is part of the kernel ABI,
so there are only small breakage concerns [1][2].
Furthermore, we already rely on /sys for other VDSM flows,
so this patch doesn't add fragility

We add a special value to the cpu_affinity tunable, to let
Vdsm select automatically best cpu. We choose the second
online core in the system, handling the case on which
only one core is active.

The admin can still select explicitely the cores on which
Vdsm should pin to, or still can disable the pinning entirely.

[1] https://www.kernel.org/doc/Documentation/ABI/README
[2] https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-system-cpu

Change-Id: Iba834f67a43ce766308036cbd079107340ed69d8
Bug-Url: https://bugzilla.redhat.com/1286462
Bug-Url: https://bugzilla.redhat.com/1279431
Backport-To: 3.6
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/49402
Reviewed-by: Nir Soffer <nsoffer at redhat.com>
Continuous-Integration: Jenkins CI
Reviewed-by: Piotr Kliczewski <piotr.kliczewski at gmail.com>
---
M lib/vdsm/config.py.in
M lib/vdsm/taskset.py
M tests/tasksetTests.py
M vdsm/vdsm
4 files changed, 102 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/49612/1

diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 9cab004..adb7dd5 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -208,11 +208,17 @@
         ('connection_stats_timeout', '3600',
             'Time in seconds defining how frequently we log transport stats'),
 
-        ('cpu_affinity', '',
-            'Comma separated whitelist of CPU cores on which VDSM is allowed '
-            'to run. The default is "", meaning VDSM can be scheduled by '
-            ' the OS to run on any core of the system. '
-            'Valid examples: "1", "0,1", "0,2,3"'),
+        ('cpu_affinity', 'auto',
+            'Use the special string value "auto" (default value) '
+            'to make Vdsm pick the first online core, starting with the '
+            'second logical core. '
+            'If only the first logical core is online, Vdsm will use it. '
+            'To explicitely select the CPU cores on which VDSM is allowed to '
+            'run, use a comma separated list of CPU cores, expressed as '
+            'integers starting from zero. '
+            'To disable the affinity, allowing Vdsm to run on all the '
+            'online cores, use the empty value. '
+            'Valid examples: "auto", "1", "0,1", ""'),
 
         ('ssl_implementation', '@SSL_IMPLEMENTATION@',
             'Specifies which ssl implementation should be used. '
diff --git a/lib/vdsm/taskset.py b/lib/vdsm/taskset.py
index 089d37b..4988b56 100644
--- a/lib/vdsm/taskset.py
+++ b/lib/vdsm/taskset.py
@@ -24,6 +24,11 @@
 from . import utils
 
 
+AUTOMATIC = "auto"
+
+_SYS_ONLINE_CPUS = "/sys/devices/system/cpu/online"
+
+
 class Error(Exception):
 
     def __init__(self, rc, out, err):
@@ -82,6 +87,25 @@
         raise Error(rc, out, err)
 
 
+def online_cpus():
+    """
+    Return a frozenset which contains identifiers of online CPUs,
+    as non-negative integers.
+    """
+    with open(_SYS_ONLINE_CPUS, 'r') as src:
+        return _cpulist_parse(src.readline())
+
+
+def pick_cpu(cpu_set):
+    """
+    Select the best CPU VDSM should pin to.
+    `cpu_set' is any iterable which produces the sequence of all
+    available CPUs, among which VDSM should pick the best one.
+    """
+    cpu_list = sorted(cpu_set)
+    return cpu_list[:2][-1]
+
+
 def _cpu_set_from_output(line):
     """
     Parse the output of taskset, in the format
@@ -91,3 +115,21 @@
     hexmask = line.rsplit(":", 1)[1].strip()
     mask = int(hexmask, 16)
     return frozenset(i for i in range(mask.bit_length()) if mask & (1 << i))
+
+
+def _cpulist_parse(cpu_range):
+    """
+    Expand the kernel cpulist syntax (e.g. 0-2,5) into a plain
+    frozenset of integers (e.g. frozenset([0,1,2,5]))
+    The input format is like the content of the special file
+    /sys/devices/system/cpu/online
+    or the output of the 'taskset' and 'lscpu' tools.
+    """
+    cpus = []
+    for item in cpu_range.split(','):
+        if '-' in item:
+            begin, end = item.split('-', 1)
+            cpus.extend(range(int(begin), int(end)+1))
+        else:
+            cpus.append(int(item))
+    return frozenset(cpus)
diff --git a/tests/tasksetTests.py b/tests/tasksetTests.py
index a002883..dcbb8a1 100644
--- a/tests/tasksetTests.py
+++ b/tests/tasksetTests.py
@@ -20,14 +20,16 @@
 
 import multiprocessing
 import os
+import tempfile
 
 from nose.plugins.skip import SkipTest
 
 from vdsm import taskset
 
-from testlib import online_cpus
+from monkeypatch import MonkeyPatchScope
 from testlib import VdsmTestCase
 from testlib import permutations, expandPermutations
+import testlib
 
 
 _CPU_COMBINATIONS = (
@@ -101,9 +103,44 @@
         self.stop.wait()
 
 
+ at expandPermutations
+class OnlineCpusFunctionsTests(VdsmTestCase):
+
+    @permutations([
+        # raw_value, cpu_set
+        ['0', set((0,))],
+        ['0,1,2,3', set(range(4))],
+        ['0-3', set(range(4))],
+        ['0-1,3', set((0, 1, 3))],
+        ['0-2,5-7', set((0, 1, 2, 5, 6, 7))],
+        # as seen on ppc64 20151130
+        ['8,16,24,32,40,48,56,64,72,80,88,96,104,112,120,128,136,144,152',
+         set((8, 16, 24, 32, 40, 48, 56, 64, 72, 80,
+              88, 96, 104, 112, 120, 128, 136, 144, 152))],
+    ])
+    def test_online_cpus(self, raw_value, cpu_set):
+
+        with tempfile.NamedTemporaryFile() as f:
+            f.write('%s\n' % raw_value)
+            f.flush()
+            with MonkeyPatchScope([(taskset, "_SYS_ONLINE_CPUS", f.name)]):
+                self.assertEqual(taskset.online_cpus(), cpu_set)
+
+    @permutations([
+        # cpu_set, expected
+        [frozenset((0,)), 0],
+        [frozenset((1,)), 1],
+        [frozenset(range(4)), 1],
+        [frozenset(range(1, 4)), 2],
+        [frozenset(range(3, 9)), 4],
+    ])
+    def test_pick_cpu(self, cpu_set, expected):
+        self.assertEqual(taskset.pick_cpu(cpu_set), expected)
+
+
 # TODO: find a clean way to make this a decorator
 def validate_running_with_enough_cpus(cpu_set):
-    max_available_cpu = sorted(online_cpus())[-1]
+    max_available_cpu = sorted(testlib.online_cpus())[-1]
     max_required_cpu = sorted(cpu_set)[-1]
 
     if max_available_cpu < max_required_cpu:
diff --git a/vdsm/vdsm b/vdsm/vdsm
index c9b6219..7db2123 100755
--- a/vdsm/vdsm
+++ b/vdsm/vdsm
@@ -265,17 +265,22 @@
 def __set_cpu_affinity():
     cpu_affinity = config.get('vars', 'cpu_affinity')
     if cpu_affinity != "":
-        cpu_set = frozenset(int(cpu.strip())
-                            for cpu in cpu_affinity.split(","))
+        online_cpus = taskset.online_cpus()
 
         log = logging.getLogger('vds')
         log.info('VDSM will run with cpu affinity: %s', cpu_set)
 
-        # too early to use the facilities from caps.py
-        if os.sysconf('SC_NPROCESSORS_ONLN') == 1:
-            log.warning('Only one cpu detected: affinity disabled')
+        if len(online_cpus) == 1:
+            log.debug('Only one cpu detected: affinity disabled')
             return
 
+        if cpu_affinity.lower() == taskset.AUTOMATIC:
+            cpu_set = frozenset((taskset.pick_cpu(online_cpus),))
+        else:
+            cpu_set = frozenset(int(cpu.strip())
+                                for cpu in cpu_affinity.split(","))
+
+        log.info('VDSM will run with cpu affinity: %s', cpu_set)
         taskset.set(os.getpid(), cpu_set, all_tasks=True)
 
 


-- 
To view, visit https://gerrit.ovirt.org/49612
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba834f67a43ce766308036cbd079107340ed69d8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>


More information about the vdsm-patches mailing list