Nir Soffer has posted comments on this change. Change subject: lib: daemon: cpu affinity support using taskset ......................................................................
Patch Set 2: (4 comments) https://gerrit.ovirt.org/#/c/45738/2/init/daemonAdapter File init/daemonAdapter: Line 69: try: Line 70: self._parse_args() Line 71: os.nice(config.getint('vars', 'vdsm_nice')) Line 72: Line 73: cpu_affinity = config.get('vars', 'cpu_affinity') Why not do this via systemd CPUAffinity? We want to kill this unneeded file, not add more features here. But worse, this change will cause all child processes to use same cpu affinity, which is not wanted. We want to limit vdsm itself, but not limit any of the child processes. We must first ensure that all child processes are run without any cpu affinity, and only then limit vdsm itself. I think the best way to ensure child process unlimited affinity is to run them with taskset 0xF..., same way we set nice and ionice. We don't want to add more features to cpopen, since it is not needed in python 3, and want to get rid of it. Line 74: if cpu_affinity: Line 75: cpu_mask = utils.cpu_mask_to_bits(cpu_affinity) Line 76: taskset = [constants.EXT_TASKSET, hex(cpu_mask).strip('lL')] Line 77: else: https://gerrit.ovirt.org/#/c/45738/2/lib/vdsm/config.py.in File lib/vdsm/config.py.in: Line 214: 'Default is empty list, meaning VDSM can be scheduled by the OS ' Line 215: 'to run on any core. ' Line 216: 'The format is list of integers from 1 to N; range syntax ' Line 217: 'like start-stop (both included) is supported. ' Line 218: 'Valid examples: "0,1", "0,2,3-5", "0,8-15,23", "0-4"') Too complicated, we need only one cpu for vdsm, so expecting list of cpu numbers is enough, ranges are not needed. The default configuration should be "1" - there is no reason to run vdsm on multiple cpus. Line 219: Line 220: ]), Line 221: Line 222: # Section: [mom] https://gerrit.ovirt.org/#/c/45738/2/lib/vdsm/constants.py.in File lib/vdsm/constants.py.in: Line 137: EXT_SU = '@SU_PATH@' Line 138: EXT_SUDO = '@SUDO_PATH@' Line 139: Line 140: EXT_TAR = '@TAR_PATH@' Line 141: EXT_TASKSET = '@TASKSET_PATH@' Why not use CommandPath? Line 142: EXT_TUNE2FS = '@TUNE2FS_PATH@' Line 143: Line 144: EXT_UMOUNT = '@UMOUNT_PATH@' Line 145: https://gerrit.ovirt.org/#/c/45738/2/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 1297: if index < CPU_MIN or index > CPU_MAX: Line 1298: raise ValueError('cpu index %i outside limits [%i, %i]', Line 1299: index, CPU_MIN, CPU_MAX) Line 1300: mask |= (1 << index) Line 1301: return mask This stuff should be in cmdutils. -- 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: 2 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: 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
