Francesco Romani 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? affinity to child processes: 100% right, that's why I mentioned cpopen patch. systemd cpuaffinity: few reasons - We may need to be able to backport this on EL6.x (unlikely, but I should make this possible) - I believe we should allow the user to select the cpu core(s) (s)he want to run VDSM, hence we need some configuration file, and systemd/initscript is not ideal 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 nu yep, but first cpu is poor fit, lots of stuff defaults here and can become overcrowded. Ok about dropping range of cpus, will do in next revision. Pinning by default feels too risky change to me, I'd rather run freely as we did before. 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? no good reason, will check it. 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. will simplify as requested elsewhere, and move. -- 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
