| Message ID | 20220805134239.1257029-1-vschneid@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | rteval: Fix loads cpulist restriction | expand |
On 05/08/22 14:42, Valentin Schneider wrote: > A recent batch of commits, one of them being: > > 39115f0a826d ("rteval: Make use of systopology instead of misc in hackbench") > > has made the loads modules use CpuList.expand_cpulist() (which produces a > list(int)) instead of misc.expand_cpulist() (which produces a list(str)). > However, the bits handling restricting CPU affinity based on a user > argument still expects to handle a list(str), which results in: > > [DEBUG] [kcompile] node 0 has no available cpus, removing > [...] > [DEBUG] [hackbench] node 0 has no available cpus, removing > This is lacking some context, so here's more: This was triggered on an arm64 system (Ampere eMAG), any sort of affinity restriction suffices, e.g. $ rteval -O -D -v --loads-cpulist=2-3 I can reproduce that on my x86 laptop: $ sudo ./rteval-cmd -O -D -v --loads-cpulist=2-3 [DEBUG] [kcompile] systopology: 1 node system (8 cores per node) [DEBUG] [kcompile] node 0 has no available cpus, removing [DEBUG] [kcompile] node 0 has no available cpus, removing [DEBUG] [hackbench] node 0 has no available cpus, removing
I think the recent slew of commit is John trying to clean up the mess that David and I made, by removing duplicate functions. I'm not against using a set instead of a list in the cpulist structure, although at that point we might want to call it a cpuset :). On Fri, Aug 5, 2022 at 2:43 PM Valentin Schneider <vschneid@redhat.com> wrote: > > On 05/08/22 14:42, Valentin Schneider wrote: > > A recent batch of commits, one of them being: > > > > 39115f0a826d ("rteval: Make use of systopology instead of misc in hackbench") > > > > has made the loads modules use CpuList.expand_cpulist() (which produces a > > list(int)) instead of misc.expand_cpulist() (which produces a list(str)). > > However, the bits handling restricting CPU affinity based on a user > > argument still expects to handle a list(str), which results in: > > > > [DEBUG] [kcompile] node 0 has no available cpus, removing > > [...] > > [DEBUG] [hackbench] node 0 has no available cpus, removing > > > > This is lacking some context, so here's more: > > This was triggered on an arm64 system (Ampere eMAG), any sort of affinity > restriction suffices, e.g. > > $ rteval -O -D -v --loads-cpulist=2-3 > > I can reproduce that on my x86 laptop: > > $ sudo ./rteval-cmd -O -D -v --loads-cpulist=2-3 > [DEBUG] [kcompile] systopology: 1 node system (8 cores per node) > [DEBUG] [kcompile] node 0 has no available cpus, removing > [DEBUG] [kcompile] node 0 has no available cpus, removing > [DEBUG] [hackbench] node 0 has no available cpus, removing >
On Fri, 5 Aug 2022, Valentin Schneider wrote: > On 05/08/22 14:42, Valentin Schneider wrote: > > A recent batch of commits, one of them being: > > > > 39115f0a826d ("rteval: Make use of systopology instead of misc in hackbench") > > > > has made the loads modules use CpuList.expand_cpulist() (which produces a > > list(int)) instead of misc.expand_cpulist() (which produces a list(str)). > > However, the bits handling restricting CPU affinity based on a user > > argument still expects to handle a list(str), which results in: > > > > [DEBUG] [kcompile] node 0 has no available cpus, removing > > [...] > > [DEBUG] [hackbench] node 0 has no available cpus, removing > > > > This is lacking some context, so here's more: > > This was triggered on an arm64 system (Ampere eMAG), any sort of affinity > restriction suffices, e.g. > > $ rteval -O -D -v --loads-cpulist=2-3 > > I can reproduce that on my x86 laptop: > > $ sudo ./rteval-cmd -O -D -v --loads-cpulist=2-3 > [DEBUG] [kcompile] systopology: 1 node system (8 cores per node) > [DEBUG] [kcompile] node 0 has no available cpus, removing > [DEBUG] [kcompile] node 0 has no available cpus, removing > [DEBUG] [hackbench] node 0 has no available cpus, removing > > Thanks Signed-off-by: John Kacur <jkacur@redhat.com> re python sets for cpu (cpuset is an overloaded term) The idea isn't bad it could work. Right now I am trying reduce the number of duplicated interfaces. Even a seemingly simple change to use lists of ints instead of lists of strings of ints can uncover problems. So, I probbaly would not be so open to python sets for cpus at the moment, but could be in the future. It is probably more work than you realize though, and I would require a full solution, which means replacing the uses of the current interfaces in cyclictest (measurement module), kcompile, hackbench, stressng. You could create a personal fork and have a go at it though if you like! John
On 05/08/22 13:04, John Kacur wrote: > On Fri, 5 Aug 2022, Valentin Schneider wrote: > >> On 05/08/22 14:42, Valentin Schneider wrote: >> > A recent batch of commits, one of them being: >> > >> > 39115f0a826d ("rteval: Make use of systopology instead of misc in hackbench") >> > >> > has made the loads modules use CpuList.expand_cpulist() (which produces a >> > list(int)) instead of misc.expand_cpulist() (which produces a list(str)). >> > However, the bits handling restricting CPU affinity based on a user >> > argument still expects to handle a list(str), which results in: >> > >> > [DEBUG] [kcompile] node 0 has no available cpus, removing >> > [...] >> > [DEBUG] [hackbench] node 0 has no available cpus, removing >> > >> >> This is lacking some context, so here's more: >> >> This was triggered on an arm64 system (Ampere eMAG), any sort of affinity >> restriction suffices, e.g. >> >> $ rteval -O -D -v --loads-cpulist=2-3 >> >> I can reproduce that on my x86 laptop: >> >> $ sudo ./rteval-cmd -O -D -v --loads-cpulist=2-3 >> [DEBUG] [kcompile] systopology: 1 node system (8 cores per node) >> [DEBUG] [kcompile] node 0 has no available cpus, removing >> [DEBUG] [kcompile] node 0 has no available cpus, removing >> [DEBUG] [hackbench] node 0 has no available cpus, removing >> >> > Thanks > Signed-off-by: John Kacur <jkacur@redhat.com> > > re python sets for cpu (cpuset is an overloaded term) > The idea isn't bad it could work. > > Right now I am trying reduce the number of duplicated interfaces. > Even a seemingly simple change to use lists of ints instead of lists of > strings of ints can uncover problems. So, I probbaly would not be so open > to python sets for cpus at the moment, but could be in the future. > > It is probably more work than you realize though, and I would require a > full solution, which means replacing the uses of the current interfaces in > cyclictest (measurement module), kcompile, hackbench, stressng. You could > create a personal fork and have a go at it though if you like! > I think I'll tinker with a new class and figure out what features it would need, and then I'll sit on it and push it further down my todolist - like you said, the primary ingredient here is gonna be time :)
On Fri, 5 Aug 2022, Valentin Schneider wrote: > On 05/08/22 13:04, John Kacur wrote: > > On Fri, 5 Aug 2022, Valentin Schneider wrote: > > > >> On 05/08/22 14:42, Valentin Schneider wrote: > >> > A recent batch of commits, one of them being: > >> > > >> > 39115f0a826d ("rteval: Make use of systopology instead of misc in hackbench") > >> > > >> > has made the loads modules use CpuList.expand_cpulist() (which produces a > >> > list(int)) instead of misc.expand_cpulist() (which produces a list(str)). > >> > However, the bits handling restricting CPU affinity based on a user > >> > argument still expects to handle a list(str), which results in: > >> > > >> > [DEBUG] [kcompile] node 0 has no available cpus, removing > >> > [...] > >> > [DEBUG] [hackbench] node 0 has no available cpus, removing > >> > > >> > >> This is lacking some context, so here's more: > >> > >> This was triggered on an arm64 system (Ampere eMAG), any sort of affinity > >> restriction suffices, e.g. > >> > >> $ rteval -O -D -v --loads-cpulist=2-3 > >> > >> I can reproduce that on my x86 laptop: > >> > >> $ sudo ./rteval-cmd -O -D -v --loads-cpulist=2-3 > >> [DEBUG] [kcompile] systopology: 1 node system (8 cores per node) > >> [DEBUG] [kcompile] node 0 has no available cpus, removing > >> [DEBUG] [kcompile] node 0 has no available cpus, removing > >> [DEBUG] [hackbench] node 0 has no available cpus, removing > >> > >> > > Thanks > > Signed-off-by: John Kacur <jkacur@redhat.com> > > > > re python sets for cpu (cpuset is an overloaded term) > > The idea isn't bad it could work. > > > > Right now I am trying reduce the number of duplicated interfaces. > > Even a seemingly simple change to use lists of ints instead of lists of > > strings of ints can uncover problems. So, I probbaly would not be so open > > to python sets for cpus at the moment, but could be in the future. > > > > It is probably more work than you realize though, and I would require a > > full solution, which means replacing the uses of the current interfaces in > > cyclictest (measurement module), kcompile, hackbench, stressng. You could > > create a personal fork and have a go at it though if you like! > > > > I think I'll tinker with a new class and figure out what features it would > need, and then I'll sit on it and push it further down my todolist - like > you said, the primary ingredient here is gonna be time :) > > Also, CpuList was really just designed as a helper class to SysTopology and not really mean as a general interface. I do have plans to separate some of that functionality out into a more generic interace we can use. It might make more sense at that time, to start thinking about whether sets would work better or not. So, like anything open source, you're welcome to play with it, but you might want to chat with me a little if you're serious about it, we could maybe hash out a coherent plan. John
diff --git a/rteval/modules/loads/hackbench.py b/rteval/modules/loads/hackbench.py index 538f60f..14e60d1 100644 --- a/rteval/modules/loads/hackbench.py +++ b/rteval/modules/loads/hackbench.py @@ -76,7 +76,7 @@ class Hackbench(CommandLineLoad): self.cpus[n] = sysTop.getcpus(int(n)) # if a cpulist was specified, only allow cpus in that list on the node if self.cpulist: - self.cpus[n] = [c for c in self.cpus[n] if str(c) in expand_cpulist(self.cpulist)] + self.cpus[n] = [c for c in self.cpus[n] if c in expand_cpulist(self.cpulist)] # track largest number of cpus used on a node node_biggest = len(sysTop.getcpus(int(n))) diff --git a/rteval/modules/loads/kcompile.py b/rteval/modules/loads/kcompile.py index a544fd9..8de00cf 100644 --- a/rteval/modules/loads/kcompile.py +++ b/rteval/modules/loads/kcompile.py @@ -235,7 +235,7 @@ class Kcompile(CommandLineLoad): # if a cpulist was specified, only allow cpus in that list on the node if self.cpulist: - self.cpus[n] = [c for c in self.cpus[n] if str(c) in expand_cpulist(self.cpulist)] + self.cpus[n] = [c for c in self.cpus[n] if c in expand_cpulist(self.cpulist)] # remove nodes with no cpus available for running for node, cpus in self.cpus.items(): diff --git a/rteval/modules/loads/stressng.py b/rteval/modules/loads/stressng.py index 287f4e2..85cb473 100644 --- a/rteval/modules/loads/stressng.py +++ b/rteval/modules/loads/stressng.py @@ -68,7 +68,7 @@ class Stressng(CommandLineLoad): cpus[n] = systop.getcpus(int(n)) # if a cpulist was specified, only allow cpus in that list on the node if self.cpulist: - cpus[n] = [c for c in cpus[n] if str(c) in expand_cpulist(self.cpulist)] + cpus[n] = [c for c in cpus[n] if c in expand_cpulist(self.cpulist)] # remove nodes with no cpus available for running for node, cpu in cpus.items():
A recent batch of commits, one of them being: 39115f0a826d ("rteval: Make use of systopology instead of misc in hackbench") has made the loads modules use CpuList.expand_cpulist() (which produces a list(int)) instead of misc.expand_cpulist() (which produces a list(str)). However, the bits handling restricting CPU affinity based on a user argument still expects to handle a list(str), which results in: [DEBUG] [kcompile] node 0 has no available cpus, removing [...] [DEBUG] [hackbench] node 0 has no available cpus, removing Remove the leftover string casts. Cyclictest is unaffected. Signed-off-by: Valentin Schneider <vschneid@redhat.com> --- Staring at this made me think that we might be better served by a single Cpumask type backed by a set() and armed with string-returning methods - I find it difficult to figure for a given CPU container if it's supposed to be int or str-backed. This opens up simplifying shrinking operations into e.g.: self.cpus[n] = self.cpus & self.cpulist How much hate would I get for following up with something like that? --- rteval/modules/loads/hackbench.py | 2 +- rteval/modules/loads/kcompile.py | 2 +- rteval/modules/loads/stressng.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)