Message ID | 20240306090722.147221-1-tglozar@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | rteval: Add relative cpulists for loads | expand |
On Wed, 6 Mar 2024, tglozar@redhat.com wrote: > From: Tomas Glozar <tglozar@redhat.com> > > Relative cpulists were added for measurements in 64ce7848 ("rteval: Add > relative cpulists for measurements"). > > Add support for relative cpulists also for loads. This works the same > way as for measurements using parse_cpulist_from_config, only difference > is there is no --loads-run-on-isolcpus option. You need a better description here for people who don't know what this is about. For example, why would this be useful? There is no mention of --only-loads > > Signed-off-by: Tomas Glozar <tglozar@redhat.com> > --- > rteval-cmd | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/rteval-cmd b/rteval-cmd > index a5e8746..8572022 100755 > --- a/rteval-cmd > +++ b/rteval-cmd > @@ -340,28 +340,29 @@ if __name__ == '__main__': > ldcfg = config.GetSection('loads') > msrcfg = config.GetSection('measurement') > msrcfg_cpulist_present = msrcfg.cpulist != "" > - # Parse measurement cpulist using parse_cpulist_from_config to account for run-on-isolcpus > + ldcfg_cpulist_present = ldcfg.cpulist != "" I'm not a fan of this syntactic sugar it makes things wordier and less readable IMO for programmers. I see I let you do that for msrcfg.cpulist, but I'd prefer you put that back too > + # Parse cpulists using parse_cpulist_from_config to account for run-on-isolcpus I'm not religious about the 80 char term, but if you're already modifying a comment, that overflows to the next line, why not neaten this up? > # and relative cpusets > cpulist = parse_cpulist_from_config(msrcfg.cpulist, msrcfg.run_on_isolcpus) > if msrcfg_cpulist_present and not cpulist_utils.is_relative(msrcfg.cpulist) and msrcfg.run_on_isolcpus: > logger.log(Log.WARN, "ignoring --measurement-run-on-isolcpus, since cpulist is specified") > msrcfg.cpulist = collapse_cpulist(cpulist) > - if ldcfg.cpulist: > - ldcfg.cpulist = remove_offline(ldcfg.cpulist) > + cpulist = parse_cpulist_from_config(ldcfg.cpulist, run_on_isolcpus=False) Since run_on_isolcpus are False by default you could also just do cpulist = parse_cpulist_from_config(ldcfg.cpulist) since we know that we are reusing the interface in which run_on_isolcpus are relevant, but in this case they are not, so explitly setting them to False is confusing > + ldcfg.cpulist = collapse_cpulist(cpulist) > # if we only specified one set of cpus (loads or measurement) > # default the other to the inverse of the specified list > - if not ldcfg.cpulist and msrcfg_cpulist_present: > + if not ldcfg_cpulist_present and msrcfg_cpulist_present: > tmplist = expand_cpulist(msrcfg.cpulist) > tmplist = SysTopology().invert_cpulist(tmplist) > tmplist = cpulist_utils.online_cpulist(tmplist) > ldcfg.cpulist = collapse_cpulist(tmplist) > - if not msrcfg_cpulist_present and ldcfg.cpulist: > + if not msrcfg_cpulist_present and ldcfg_cpulist_present: > tmplist = expand_cpulist(ldcfg.cpulist) > tmplist = SysTopology().invert_cpulist(tmplist) > tmplist = cpulist_utils.online_cpulist(tmplist) > msrcfg.cpulist = collapse_cpulist(tmplist) > > - if ldcfg.cpulist: > + if ldcfg_cpulist_present: > logger.log(Log.DEBUG, f"loads cpulist: {ldcfg.cpulist}") > # if --onlyload is specified msrcfg.cpulist is unused > if msrcfg_cpulist_present and not rtevcfg.onlyload: > -- > 2.44.0 This works, but it needs to be more robust. For example rteval-cmd --measurement-run-on-isolcpus --measurement-cpulist -2,4 -d5s fails with rteval-cmd: error: argument --measurement-cpulist: expected one argument although this works rteval-cmd --measurement-run-on-isolcpus --measurement-cpulist +0,-2,4 -d5s Also, ranges are not supported, it would be nice to do -2-5 instead of -2,3,4,5 Thanks John
diff --git a/rteval-cmd b/rteval-cmd index a5e8746..8572022 100755 --- a/rteval-cmd +++ b/rteval-cmd @@ -340,28 +340,29 @@ if __name__ == '__main__': ldcfg = config.GetSection('loads') msrcfg = config.GetSection('measurement') msrcfg_cpulist_present = msrcfg.cpulist != "" - # Parse measurement cpulist using parse_cpulist_from_config to account for run-on-isolcpus + ldcfg_cpulist_present = ldcfg.cpulist != "" + # Parse cpulists using parse_cpulist_from_config to account for run-on-isolcpus # and relative cpusets cpulist = parse_cpulist_from_config(msrcfg.cpulist, msrcfg.run_on_isolcpus) if msrcfg_cpulist_present and not cpulist_utils.is_relative(msrcfg.cpulist) and msrcfg.run_on_isolcpus: logger.log(Log.WARN, "ignoring --measurement-run-on-isolcpus, since cpulist is specified") msrcfg.cpulist = collapse_cpulist(cpulist) - if ldcfg.cpulist: - ldcfg.cpulist = remove_offline(ldcfg.cpulist) + cpulist = parse_cpulist_from_config(ldcfg.cpulist, run_on_isolcpus=False) + ldcfg.cpulist = collapse_cpulist(cpulist) # if we only specified one set of cpus (loads or measurement) # default the other to the inverse of the specified list - if not ldcfg.cpulist and msrcfg_cpulist_present: + if not ldcfg_cpulist_present and msrcfg_cpulist_present: tmplist = expand_cpulist(msrcfg.cpulist) tmplist = SysTopology().invert_cpulist(tmplist) tmplist = cpulist_utils.online_cpulist(tmplist) ldcfg.cpulist = collapse_cpulist(tmplist) - if not msrcfg_cpulist_present and ldcfg.cpulist: + if not msrcfg_cpulist_present and ldcfg_cpulist_present: tmplist = expand_cpulist(ldcfg.cpulist) tmplist = SysTopology().invert_cpulist(tmplist) tmplist = cpulist_utils.online_cpulist(tmplist) msrcfg.cpulist = collapse_cpulist(tmplist) - if ldcfg.cpulist: + if ldcfg_cpulist_present: logger.log(Log.DEBUG, f"loads cpulist: {ldcfg.cpulist}") # if --onlyload is specified msrcfg.cpulist is unused if msrcfg_cpulist_present and not rtevcfg.onlyload: