Change in vdsm[master]: lib: daemon: cpu affinity support using taskset

fromani at redhat.com fromani at redhat.com
Fri Sep 4 13:52:52 UTC 2015


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 <fromani at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list