Francesco Romani has posted comments on this change. Change subject: scale: limit cpu usage using cpu-affinity ......................................................................
Patch Set 13: (6 comments) https://gerrit.ovirt.org/#/c/45738/13//COMMIT_MSG Commit Message: Line 26: any CPU core. Line 27: Line 28: We need a patch which is simple to backport down to 3.5, Line 29: so we use taskset just before the start of the main daemon Line 30: to set VDSM cpu affinity, and we start any child process using > We do not start child process using taskset now, please update that we do t Done Line 31: taskset to remove VDSM's cpu affinity and allow the child Line 32: process to use any cpu. Line 33: Line 34: taskset is part of util-linux, so no additional dependency https://gerrit.ovirt.org/#/c/45738/13/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 636 Line 637 Line 638 Line 639 Line 640 > I think we should change cpu affinity only if vdsm is configured to modify Neat idea indeed. Will use this. Line 625: def __del__(self): Line 626: self._poller.close() Line 627: Line 628: Line 629: _ANY_CPU = ["0-%d" % (os.sysconf('SC_NPROCESSORS_CONF') - 1)] > Probably, but this constant is unused now. Yep, I missed this. Removing. Line 630: Line 631: Line 632: def execCmd(command, sudo=False, cwd=None, data=None, raw=False, Line 633: printable=None, env=None, sync=True, nice=None, ioclass=None, https://gerrit.ovirt.org/#/c/45738/13/tests/tasksetTests.py File tests/tasksetTests.py: Line 92: # here we just need to feed taskset with any bad input. Line 93: self.assertRaises(taskset.Error, taskset.set, '', 'x') Line 94: Line 95: Line 96: # we expect that on devel/test boxes there are no offline cpu > Use sysconf like cpopen does, so we don't have to assume anything. I was about to do that, then I checked the implementation in multiprocessing, which on linux[1] indeed already uses sysconf. So I'll just drop the comment. +++ [1] wherever sys.platform not in ('win32', 'bsd', 'darwin') Line 97: def all_active_cpus(): https://gerrit.ovirt.org/#/c/45738/13/tests/utilsTests.py File tests/utilsTests.py: Line 76: class PidStatTests(TestCaseBase): Line 77: Line 78: def test(self): Line 79: args = ["sleep", "3"] Line 80: sproc = utils.execCmd(args, sync=False, resetCpuAffinity=False) > We don't need to change this, as we don't use taskset for this. Right, this is now useless. Line 81: Line 82: stats = utils.pidStat(sproc.pid) Line 83: pid = int(stats.pid) Line 84: # procName comes in the format of (procname) https://gerrit.ovirt.org/#/c/45738/13/vdsm/vdsm File vdsm/vdsm: Line 258: Line 259: def _setCpuAffinity(): Line 260: cpu_affinity = config.get('vars', 'cpu_affinity') Line 261: if cpu_affinity: Line 262: taskset.set(os.getpid(), cpu_affinity.split(','), all_tasks=True) > We should strip values, in case a user use something like "1, 2, 3". Done Line 263: Line 264: Line 265: def main(): Line 266: __assertVdsmUser() -- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yaniv Kaul <yk...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
