Francesco Romani has posted comments on this change. Change subject: scale: limit cpu usage using cpu-affinity ......................................................................
Patch Set 21: (8 comments) https://gerrit.ovirt.org/#/c/45738/21/lib/vdsm/taskset.py File lib/vdsm/taskset.py: Line 42: We assume all threads of the process have the same affinity, because Line 43: this is the only usecase VDSM cares about - and requires. Line 44: Return a list of strings, each one being a cpu indices on which the Line 45: process can run. Line 46: Example: ['0', '1', '2', '3'] > frozenset of ints would be nice, but now this does not match set() which wo OK, let me see how I can accomodate both needs. Line 47: Raise taskset.Error on failure. Line 48: """ Line 49: command = [constants.EXT_TASKSET, '--pid', str(pid)] Line 50: Line 87: and return a list of strings, each one being is a cpu index. Line 88: """ Line 89: hexmask = line.rsplit(":", 1)[1].strip() Line 90: mask = int(hexmask, 16) Line 91: return sorted(str(i) for i in range(mask.bit_length()) if mask & (1 << i)) > This list will actually be sorted by itself, because we iterate on the bits Done https://gerrit.ovirt.org/#/c/45738/21/tests/tasksetTests.py File tests/tasksetTests.py: Line 54: not self.stop > could you add a comment on why Because this is a relic of a past version. self.stop.wait() should be good enough. Line 59: self.assertTrue(self.running.wait(0.5)) > If we fail to get the subprocess running within 0.5 sec, it's an environmen Done Line 57: self.proc = multiprocessing.Process(target=_run_helper) Line 58: self.proc.start() Line 59: self.assertTrue(self.running.wait(0.5)) Line 60: Line 61: self.assertEqual(taskset.get(self.proc.pid), online_cpus()) > Right, lets compare with the taskset of the parent process. Done Line 62: Line 63: @permutations(_CPU_COMBINATIONS) Line 64: def test_set_from_parent(self, cpu_list): Line 65: Line 81: def test_set_from_child(self, cpu_list): Line 82: Line 83: validate_running_with_enough_cpus(cpu_list) Line 84: Line 85: def _run_helper(): > I believe that _run_helper(cpu_list=()) can be written once, on the class l OK, but I was asked to inline it in a former review :) What could be a common middle ground? Line 86: # avoid race on startup: first do taskset, then notify Line 87: taskset.set(os.getpid(), cpu_list) Line 88: self.running.set() Line 89: while not self.stop.is_set(): Line 106: Line 107: # TODO: find a clean way to make this a decorator Line 108: def validate_running_with_enough_cpus(cpu_list): Line 109: max_available_cpu = int(online_cpus()[-1]) Line 110: max_required_cpu = int(sorted(cpu_list)[-1]) > since '10' < '2', I think you want Right, but no longer an issue since we now return frozensets() of ints. Line 111: Line 112: if (max_available_cpu < max_required_cpu): Line 113: raise SkipTest( Line 114: "This test requires at least %i available CPUs" https://gerrit.ovirt.org/#/c/45738/21/tests/testlib.py File tests/testlib.py: Line 467: Line 468: Line 469: def online_cpus(): Line 470: return sorted(str(index) for index in Line 471: range(os.sysconf('SC_NPROCESSORS_ONLN'))) > This sort will not work we convert the index to string, and it seems that u Indeed the test should (and will) use the taskset of the parent process; but still we need this only to ensure we have enough cpus to run some permutations. Will move in tasksetTests.py. -- To view, visit https://gerrit.ovirt.org/45738 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
