Skip to content

Commit

Permalink
irqbalance: set banned cpus list to 0
Browse files Browse the repository at this point in the history
Empty irqbalance ban list means that irqbalance will be calculated by default.
The default will exclude all isolated and nohz_full cpus from the mask
resulting in the irq’s being balanced over the reserved cpus only,
breaking the user intent.

CPU numbers which have their corresponding bits set to one in the ban mask
will not have any irq's assigned to them on rebalance.
Then we should set the mask to zero, so all cpus will participate in load balancing.
  • Loading branch information
yanirq committed Mar 16, 2024
1 parent 5985b17 commit 25cb7e8
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ SED="/usr/bin/sed"
# tunable - overridable for testing purposes
IRQBALANCE_CONF="${1:-/etc/sysconfig/irqbalance}"
CRIO_ORIG_BANNED_CPUS="${2:-/etc/sysconfig/orig_irq_banned_cpus}"
NONE=0

[ ! -f ${IRQBALANCE_CONF} ] && exit 0
[ ! -f "${IRQBALANCE_CONF}" ] && exit 0

${SED} -i '/^\s*IRQBALANCE_BANNED_CPUS\b/d' ${IRQBALANCE_CONF} || exit 0
echo "IRQBALANCE_BANNED_CPUS=" >> ${IRQBALANCE_CONF}
${SED} -i '/^\s*IRQBALANCE_BANNED_CPUS\b/d' "${IRQBALANCE_CONF}" || exit 0
# CPU numbers which have their corresponding bits set to one in this mask
# will not have any irq's assigned to them on rebalance.
# so zero means all cpus are participating in load balancing.
echo "IRQBALANCE_BANNED_CPUS=${NONE}" >> "${IRQBALANCE_CONF}"

# we now own this configuration. But CRI-O has code to restore the configuration,
# and until it gains the option to disable this restore flow, we need to make
# the configuration consistent such as the CRI-O restore will do nothing.
if [ -n ${CRIO_ORIG_BANNED_CPUS} ] && [ -f ${CRIO_ORIG_BANNED_CPUS} ]; then
true > ${CRIO_ORIG_BANNED_CPUS}
if [ -n "${CRIO_ORIG_BANNED_CPUS}" ] && [ -f "${CRIO_ORIG_BANNED_CPUS}" ]; then
echo "${NONE}" > "${CRIO_ORIG_BANNED_CPUS}"
fi
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ var _ = Describe("Assets scripts", func() {

bannedCPUs, ok := extractBannedCPUs(string(data))
Expect(ok).To(BeTrue())
Expect(bannedCPUs).To(BeEmpty())
Expect(bannedCPUs).To(Equal("0"))
})

It("should handle empty ban list", func() {
Expand All @@ -92,7 +92,7 @@ var _ = Describe("Assets scripts", func() {

bannedCPUs, ok := extractBannedCPUs(string(data))
Expect(ok).To(BeTrue())
Expect(bannedCPUs).To(BeEmpty())
Expect(bannedCPUs).To(Equal("0"))
})

It("should clear existing ban list", func() {
Expand Down Expand Up @@ -125,7 +125,7 @@ var _ = Describe("Assets scripts", func() {

updatedBannedCPUs, ok := extractBannedCPUs(string(data))
Expect(ok).To(BeTrue())
Expect(updatedBannedCPUs).To(BeEmpty())
Expect(updatedBannedCPUs).To(Equal("0"))
})
})
})
Expand Down Expand Up @@ -186,10 +186,6 @@ func getBannedCPUs(line string) (string, bool) {
items := strings.FieldsFunc(line, func(r rune) bool {
return r == '='
})
// missing value is interpreted (by shell and by us) as empty string
if len(items) == 1 {
return "", true
}
// too many values, bail out
if len(items) != 2 {
return "", false
Expand Down
17 changes: 5 additions & 12 deletions test/e2e/performanceprofile/functests/1_performance/irqbalance.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,11 @@ var _ = Describe("[performance] Checking IRQBalance settings", Ordered, func() {

origBannedCPUsFile := "/etc/sysconfig/orig_irq_banned_cpus"
By(fmt.Sprintf("Checking content of %q on node %q", origBannedCPUsFile, node.Name))
expectFileEmpty(node, origBannedCPUsFile)
fullPath := filepath.Join("/", "rootfs", origBannedCPUsFile)
out, err := nodes.ExecCommandOnNode([]string{"/usr/bin/cat", fullPath}, node)
Expect(err).ToNot(HaveOccurred())
out = strings.TrimSuffix(out, "\r\n")
Expect(out).To(Equal("0"), "file %s does not contain the expect output; expected=0 actual=%s", fullPath, out)
})
})
})
Expand Down Expand Up @@ -341,9 +345,6 @@ func getIrqBalanceBannedCPUs(node *corev1.Node) (cpuset.CPUSet, error) {
items := strings.FieldsFunc(keyValue, func(c rune) bool {
return c == '='
})
if len(items) == 1 {
return cpuset.New(), nil
}
if len(items) != 2 {
return cpuset.New(), fmt.Errorf("malformed CPU ban list in the configuration")
}
Expand Down Expand Up @@ -400,11 +401,3 @@ func unquote(s string) string {
s = strings.TrimSuffix(s, q)
return s
}

func expectFileEmpty(node *corev1.Node, path string) {
fullPath := filepath.Join("/", "rootfs", path)
out, err := nodes.ExecCommandOnNode([]string{"wc", "-c", fullPath}, node)
ExpectWithOffset(1, err).ToNot(HaveOccurred())
expected := "0 " + fullPath
ExpectWithOffset(1, out).To(Equal(expected), "file %s (%s) not empty", path, fullPath)
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ spec:
path: /usr/local/bin/set-cpus-offline.sh
user: {}
- contents:
source: data:text/plain;charset=utf-8;base64,IyEvdXNyL2Jpbi9lbnYgYmFzaApzZXQgLWV1byBwaXBlZmFpbApzZXQgLXgKCiMgY29uc3QKU0VEPSIvdXNyL2Jpbi9zZWQiCiMgdHVuYWJsZSAtIG92ZXJyaWRhYmxlIGZvciB0ZXN0aW5nIHB1cnBvc2VzCklSUUJBTEFOQ0VfQ09ORj0iJHsxOi0vZXRjL3N5c2NvbmZpZy9pcnFiYWxhbmNlfSIKQ1JJT19PUklHX0JBTk5FRF9DUFVTPSIkezI6LS9ldGMvc3lzY29uZmlnL29yaWdfaXJxX2Jhbm5lZF9jcHVzfSIKClsgISAtZiAke0lSUUJBTEFOQ0VfQ09ORn0gXSAmJiBleGl0IDAKCiR7U0VEfSAtaSAnL15ccypJUlFCQUxBTkNFX0JBTk5FRF9DUFVTXGIvZCcgJHtJUlFCQUxBTkNFX0NPTkZ9IHx8IGV4aXQgMAplY2hvICJJUlFCQUxBTkNFX0JBTk5FRF9DUFVTPSIgPj4gJHtJUlFCQUxBTkNFX0NPTkZ9CgojIHdlIG5vdyBvd24gdGhpcyBjb25maWd1cmF0aW9uLiBCdXQgQ1JJLU8gaGFzIGNvZGUgdG8gcmVzdG9yZSB0aGUgY29uZmlndXJhdGlvbiwKIyBhbmQgdW50aWwgaXQgZ2FpbnMgdGhlIG9wdGlvbiB0byBkaXNhYmxlIHRoaXMgcmVzdG9yZSBmbG93LCB3ZSBuZWVkIHRvIG1ha2UKIyB0aGUgY29uZmlndXJhdGlvbiBjb25zaXN0ZW50IHN1Y2ggYXMgdGhlIENSSS1PIHJlc3RvcmUgd2lsbCBkbyBub3RoaW5nLgppZiBbIC1uICR7Q1JJT19PUklHX0JBTk5FRF9DUFVTfSBdICYmIFsgLWYgJHtDUklPX09SSUdfQkFOTkVEX0NQVVN9IF07IHRoZW4KCXRydWUgPiAke0NSSU9fT1JJR19CQU5ORURfQ1BVU30KZmkK
source: data:text/plain;charset=utf-8;base64,IyEvdXNyL2Jpbi9lbnYgYmFzaApzZXQgLWV1byBwaXBlZmFpbApzZXQgLXgKCiMgY29uc3QKU0VEPSIvdXNyL2Jpbi9zZWQiCiMgdHVuYWJsZSAtIG92ZXJyaWRhYmxlIGZvciB0ZXN0aW5nIHB1cnBvc2VzCklSUUJBTEFOQ0VfQ09ORj0iJHsxOi0vZXRjL3N5c2NvbmZpZy9pcnFiYWxhbmNlfSIKQ1JJT19PUklHX0JBTk5FRF9DUFVTPSIkezI6LS9ldGMvc3lzY29uZmlnL29yaWdfaXJxX2Jhbm5lZF9jcHVzfSIKTk9ORT0wCgpbICEgLWYgIiR7SVJRQkFMQU5DRV9DT05GfSIgXSAmJiBleGl0IDAKCiR7U0VEfSAtaSAnL15ccypJUlFCQUxBTkNFX0JBTk5FRF9DUFVTXGIvZCcgIiR7SVJRQkFMQU5DRV9DT05GfSIgfHwgZXhpdCAwCiMgQ1BVIG51bWJlcnMgd2hpY2ggaGF2ZSB0aGVpciBjb3JyZXNwb25kaW5nIGJpdHMgc2V0IHRvIG9uZSBpbiB0aGlzIG1hc2sKIyB3aWxsIG5vdCBoYXZlIGFueSBpcnEncyBhc3NpZ25lZCB0byB0aGVtIG9uIHJlYmFsYW5jZS4KIyBzbyB6ZXJvIG1lYW5zIGFsbCBjcHVzIGFyZSBwYXJ0aWNpcGF0aW5nIGluIGxvYWQgYmFsYW5jaW5nLgplY2hvICJJUlFCQUxBTkNFX0JBTk5FRF9DUFVTPSR7Tk9ORX0iID4+ICIke0lSUUJBTEFOQ0VfQ09ORn0iCgojIHdlIG5vdyBvd24gdGhpcyBjb25maWd1cmF0aW9uLiBCdXQgQ1JJLU8gaGFzIGNvZGUgdG8gcmVzdG9yZSB0aGUgY29uZmlndXJhdGlvbiwKIyBhbmQgdW50aWwgaXQgZ2FpbnMgdGhlIG9wdGlvbiB0byBkaXNhYmxlIHRoaXMgcmVzdG9yZSBmbG93LCB3ZSBuZWVkIHRvIG1ha2UKIyB0aGUgY29uZmlndXJhdGlvbiBjb25zaXN0ZW50IHN1Y2ggYXMgdGhlIENSSS1PIHJlc3RvcmUgd2lsbCBkbyBub3RoaW5nLgppZiBbIC1uICIke0NSSU9fT1JJR19CQU5ORURfQ1BVU30iIF0gJiYgWyAtZiAiJHtDUklPX09SSUdfQkFOTkVEX0NQVVN9IiBdOyB0aGVuCgllY2hvICIke05PTkV9IiA+ICIke0NSSU9fT1JJR19CQU5ORURfQ1BVU30iCmZpCg==
verification: {}
group: {}
mode: 448
Expand Down

0 comments on commit 25cb7e8

Please sign in to comment.