libct: fix two CPU mask nits from #5343 - #5392
Open
kolyshkin wants to merge 2 commits into
Open
Conversation
Since CPUAffinity.Initial/Final and LinuxMemoryPolicy.Nodes became unix.CPUSetDynamic (a slice) rather than *unix.CPUSet (a pointer), a nil check is no longer a complete "unset" test: a non-nil but empty slice is now representable, and can be produced by unmarshalling a "[]" from state.json or by a libcontainer user setting the field directly. Such a value used to pass the "!= nil" check and get handed to sched_setaffinity(2) as a NULL pointer with a zero size, which the kernel interprets as an empty mask and rejects with EINVAL, rather than resetting the affinity as intended. Use len() instead, which is both idiomatic for slices and treats nil and empty alike. While at it, drop the now-redundant nil guards in the memory policy validator (Count on a nil slice is 0). Fixes: daf934f ("libct: use CPUSetDynamic for affinity and mempolicy masks") Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
MaxCPU was documented and enforced as the highest CPU/NUMA node ID that ToCPUSet accepts, i.e. an inclusive bound, while unix.NewCPUSet takes an exclusive one. As a result, tryResetCPUAffinity's NewCPUSet(MaxCPU) mask was one ID short of what ToCPUSet can parse. This is harmless in practice, as sched_setaffinity(2) truncates the mask to cpumask_size() anyway, and no kernel is configured with anywhere near 64K CPUs. Still, having a single constant mean two different things in two different files is asking for a real off-by-one later on. Make MaxCPU exclusive to match NewCPUSet, so that NewCPUSet(MaxCPU) is correct as written, and ToCPUSet's NewCPUSet(maxID+1) can never exceed it. This restores the "ret >= max" check and the "max-1" wording used before MaxCPU was introduced, and shifts the test boundary cases by one. Fixes: daf934f ("libct: use CPUSetDynamic for affinity and mempolicy masks") Fixes: c55649b ("libct: reuse configs.MaxCPU") Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
cpu-aff-fixes
branch
from
August 11, 2026 16:23
6581114 to
9d21caa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-up fixes to #5343, found while re-reading the CPU mask changes.
libct: use len() to check for empty CPU masks
When
CPUAffinity.Initial/FinalandLinuxMemoryPolicy.Nodeswere*unix.CPUSet, a nil check was a complete "unset" test. Asunix.CPUSetDynamic(a slice) it no longer is — a non-nil zero-length slice is now representable, and{"Initial":[]}instate.jsonunmarshals to exactly that.Such a value passes the
!= nilcheck and reachessched_setaffinity(2)as a NULL pointer with a zero size, which the kernel reads as an empty mask and rejects withEINVAL, instead of resetting the affinity as intended.Only reachable via a hand-edited
state.jsonor a libcontainer user setting the field directly, butlen()is the idiomatic slice check anyway and treats nil and empty alike. Also drops the now-redundant nil guards in the memory policy validator, sinceCount()on a nil slice is already 0.setupMemoryPolicyis deliberately left as is: an emptyNodesis legitimate there, asMPOL_DEFAULT/MPOL_LOCALwant a NULL/zero-size nodemask, and the validator already rejects an emptyNodesfor the modes that require one.libct: make MaxCPU exclusive
MaxCPUwas documented and enforced as an inclusive bound (the highest IDToCPUSetaccepts), whileunix.NewCPUSettakes an exclusive one. SotryResetCPUAffinity'sNewCPUSet(MaxCPU)mask was one ID short of whatToCPUSetcan parse.Harmless in practice —
sched_setaffinity(2)truncates the mask tocpumask_size(), and no kernel is configured with anywhere near 64K CPUs — but one constant meaning two different things in two files is asking for a real off-by-one later.Making
MaxCPUexclusive meansNewCPUSet(MaxCPU)is correct as written andToCPUSet'sNewCPUSet(maxID+1)can never exceed it. This restores theret >= maxcheck andmax-1wording that predated the constant, and shifts the test boundary cases down by one.Fixes: #5388
Closes: #5389