Release 1.9.2-2

Rebase to latest upstream commit (50699824c7)
Resolves:rhbz2184735

Signed-off-by: Tao Liu <ltao@redhat.com>
This commit is contained in:
Tao Liu 2023-07-18 09:24:18 +08:00
parent 185ff9cc5d
commit d7861c8775
8 changed files with 418 additions and 1 deletions

View File

@ -0,0 +1,35 @@
From 8bbc0aeca0187ad3f5f942408198bc0fc055b0f8 Mon Sep 17 00:00:00 2001
From: Tao Liu <ltao@redhat.com>
Date: Tue, 4 Jul 2023 10:04:10 +0800
Subject: [PATCH 1/7] activate_mapping: fflush the buffered data to
smp_affinity
Previously irqbalance uses the return value of fprintf() to decide whether
the modification of smp_affinity is successful or not. However it is not
reliable because fprintf() is stream buffered, a fflush() should be used to
check the buffered data been successfully written into the file before the
judgement.
This patch fixes the issue by introducing fflush() after fprintf().
Signed-off-by: Tao Liu <ltao@redhat.com>
---
activate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/activate.c b/activate.c
index 62cfd08..6d0b096 100644
--- a/activate.c
+++ b/activate.c
@@ -76,7 +76,7 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
cpumask_scnprintf(buf, PATH_MAX, applied_mask);
ret = fprintf(file, "%s", buf);
- if (ret < 0) {
+ if (ret < 0 || fflush(file)) {
log(TO_ALL, LOG_WARNING, "cannot change irq %i's affinity, add it to banned list", info->irq);
add_banned_irq(info->irq);
remove_one_irq_from_db(info->irq);
--
2.40.1

View File

@ -0,0 +1,30 @@
From 4efc1923f8fd161d131d2b8e8930bf5c9ed66668 Mon Sep 17 00:00:00 2001
From: Neil Horman <neil.horman@secedge.com>
Date: Sun, 9 Jul 2023 10:09:17 -0400
Subject: [PATCH 2/7] Revert "activate_mapping: fflush the buffered data to
smp_affinity"
This reverts commit 8bbc0aeca0187ad3f5f942408198bc0fc055b0f8.
Was causing a segfault:
https://github.com/Irqbalance/irqbalance/issues/267
---
activate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/activate.c b/activate.c
index 6d0b096..62cfd08 100644
--- a/activate.c
+++ b/activate.c
@@ -76,7 +76,7 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
cpumask_scnprintf(buf, PATH_MAX, applied_mask);
ret = fprintf(file, "%s", buf);
- if (ret < 0 || fflush(file)) {
+ if (ret < 0) {
log(TO_ALL, LOG_WARNING, "cannot change irq %i's affinity, add it to banned list", info->irq);
add_banned_irq(info->irq);
remove_one_irq_from_db(info->irq);
--
2.40.1

View File

@ -0,0 +1,144 @@
From f589bdced6e1fe885969f2833fc3cacdfb60ea79 Mon Sep 17 00:00:00 2001
From: Robin Jarry <rjarry@redhat.com>
Date: Tue, 11 Jul 2023 15:17:55 +0200
Subject: [PATCH 3/7] activate_mapping: avoid use-after-free when affinity
cannot be set
add_banned_irq appends the irq_info to the banned_irqs list.
remove_one_irq_from_db removes it from the interrupts_db and free()s it.
This leaves an invalid pointer dangling in banned_irqs *and* potentially
in rebalance_irq_list which can cause use-after-free errors.
Do not move the irq_info around. Only add a flag to indicate that this
irq's affinity cannot be managed and ignore the irq when this flag is
set.
Link: https://github.com/Irqbalance/irqbalance/issues/267
Fixes: 55c5c321c73e ("arm64: Add irq aff change check For aarch64...")
Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
activate.c | 8 +++++---
classify.c | 28 +++-------------------------
irqbalance.h | 2 --
types.h | 3 ++-
4 files changed, 10 insertions(+), 31 deletions(-)
diff --git a/activate.c b/activate.c
index 62cfd08..6f8af27 100644
--- a/activate.c
+++ b/activate.c
@@ -60,6 +60,9 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
if (!info->assigned_obj)
return;
+ if (info->flags & IRQ_FLAG_AFFINITY_UNMANAGED)
+ return;
+
/* activate only online cpus, otherwise writing to procfs returns EOVERFLOW */
cpus_and(applied_mask, cpu_online_map, info->assigned_obj->mask);
@@ -77,9 +80,8 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
cpumask_scnprintf(buf, PATH_MAX, applied_mask);
ret = fprintf(file, "%s", buf);
if (ret < 0) {
- log(TO_ALL, LOG_WARNING, "cannot change irq %i's affinity, add it to banned list", info->irq);
- add_banned_irq(info->irq);
- remove_one_irq_from_db(info->irq);
+ log(TO_ALL, LOG_WARNING, "cannot change IRQ %i affinity, will never try again\n", info->irq);
+ info->flags |= IRQ_FLAG_AFFINITY_UNMANAGED;
}
fclose(file);
info->moved = 0; /*migration is done*/
diff --git a/classify.c b/classify.c
index dac813c..1601a1e 100644
--- a/classify.c
+++ b/classify.c
@@ -257,7 +257,7 @@ static gint compare_ints(gconstpointer a, gconstpointer b)
return ai->irq - bi->irq;
}
-static void __add_banned_irq(int irq, GList **list)
+static void add_banned_irq(int irq, GList **list)
{
struct irq_info find, *new;
GList *entry;
@@ -281,14 +281,9 @@ static void __add_banned_irq(int irq, GList **list)
return;
}
-void add_banned_irq(int irq)
-{
- __add_banned_irq(irq, &banned_irqs);
-}
-
void add_cl_banned_irq(int irq)
{
- __add_banned_irq(irq, &cl_banned_irqs);
+ add_banned_irq(irq, &cl_banned_irqs);
}
gint substr_find(gconstpointer a, gconstpointer b)
@@ -392,23 +387,6 @@ get_numa_node:
return new;
}
-void remove_one_irq_from_db(int irq)
-{
- struct irq_info find, *tmp;
- GList *entry = NULL;
-
- find.irq = irq;
- entry = g_list_find_custom(interrupts_db, &find, compare_ints);
- if (!entry)
- return;
-
- tmp = entry->data;
- interrupts_db = g_list_remove(interrupts_db, tmp);
- free(tmp);
- log(TO_CONSOLE, LOG_INFO, "IRQ %d was removed from db.\n", irq);
- return;
-}
-
static void parse_user_policy_key(char *buf, int irq, struct user_irq_policy *pol)
{
char *key, *value, *end;
@@ -629,7 +607,7 @@ static void add_new_irq(char *path, struct irq_info *hint)
/* Set NULL devpath for the irq has no sysfs entries */
get_irq_user_policy(path, irq, &pol);
if ((pol.ban == 1) || check_for_irq_ban(hint, mod)) { /*FIXME*/
- __add_banned_irq(irq, &banned_irqs);
+ add_banned_irq(irq, &banned_irqs);
new = get_irq_info(irq);
} else
new = add_one_irq_to_db(path, hint, &pol);
diff --git a/irqbalance.h b/irqbalance.h
index e7f6b94..46e05ca 100644
--- a/irqbalance.h
+++ b/irqbalance.h
@@ -109,8 +109,6 @@ extern struct irq_info *get_irq_info(int irq);
extern void migrate_irq(GList **from, GList **to, struct irq_info *info);
extern void free_cl_opts(void);
extern void add_cl_banned_module(char *modname);
-extern void add_banned_irq(int irq);
-extern void remove_one_irq_from_db(int irq);
#define irq_numa_node(irq) ((irq)->numa_node)
diff --git a/types.h b/types.h
index 9693cf4..c63cfea 100644
--- a/types.h
+++ b/types.h
@@ -34,7 +34,8 @@
/*
* IRQ Internal tracking flags
*/
-#define IRQ_FLAG_BANNED 1
+#define IRQ_FLAG_BANNED (1ULL << 0)
+#define IRQ_FLAG_AFFINITY_UNMANAGED (1ULL << 1)
enum obj_type_e {
OBJ_TYPE_CPU,
--
2.40.1

View File

@ -0,0 +1,48 @@
From 470a64b190628574c28a266bdcf8960291463191 Mon Sep 17 00:00:00 2001
From: Robin Jarry <rjarry@redhat.com>
Date: Wed, 12 Jul 2023 08:51:08 +0200
Subject: [PATCH 4/7] activate_mapping: make sure to catch all errors
fprintf() is buffered and may not report an error which may be deferred
when fflush() is called (either explicitly or internally by fclose()).
Check for errors returned by fopen(), fprintf() and fclose() and add
IRQ_FLAG_AFFINITY_UNMANAGED accordingly.
Fixes: 55c5c321c73e ("arm64: Add irq aff change check For aarch64, ...")
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2184735
Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
activate.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/activate.c b/activate.c
index 6f8af27..a4112e0 100644
--- a/activate.c
+++ b/activate.c
@@ -75,16 +75,16 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
sprintf(buf, "/proc/irq/%i/smp_affinity", info->irq);
file = fopen(buf, "w");
if (!file)
- return;
+ goto error;
cpumask_scnprintf(buf, PATH_MAX, applied_mask);
ret = fprintf(file, "%s", buf);
- if (ret < 0) {
- log(TO_ALL, LOG_WARNING, "cannot change IRQ %i affinity, will never try again\n", info->irq);
- info->flags |= IRQ_FLAG_AFFINITY_UNMANAGED;
- }
- fclose(file);
+ if (fclose(file) || ret < 0)
+ goto error;
info->moved = 0; /*migration is done*/
+error:
+ log(TO_ALL, LOG_WARNING, "cannot change IRQ %i affinity, will never try again\n", info->irq);
+ info->flags |= IRQ_FLAG_AFFINITY_UNMANAGED;
}
void activate_mappings(void)
--
2.40.1

View File

@ -0,0 +1,64 @@
From 9a1fd29a82c9762c3676f613075d44a8d1fcbe82 Mon Sep 17 00:00:00 2001
From: Robin Jarry <rjarry@redhat.com>
Date: Wed, 12 Jul 2023 08:59:45 +0200
Subject: [PATCH 5/7] activate_mapping: report error reason
If a given IRQ affinity cannot be set, include strerror in the warning
message.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2184735
Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
activate.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/activate.c b/activate.c
index a4112e0..4418cda 100644
--- a/activate.c
+++ b/activate.c
@@ -25,10 +25,12 @@
* of interrupts to the kernel.
*/
#include "config.h"
+#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <stdint.h>
+#include <string.h>
#include "irqbalance.h"
@@ -48,7 +50,7 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
{
char buf[PATH_MAX];
FILE *file;
- int ret = 0;
+ int errsave, ret;
cpumask_t applied_mask;
/*
@@ -79,11 +81,18 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
cpumask_scnprintf(buf, PATH_MAX, applied_mask);
ret = fprintf(file, "%s", buf);
- if (fclose(file) || ret < 0)
+ errsave = errno;
+ if (fclose(file)) {
+ errsave = errno;
+ goto error;
+ }
+ if (ret < 0)
goto error;
info->moved = 0; /*migration is done*/
error:
- log(TO_ALL, LOG_WARNING, "cannot change IRQ %i affinity, will never try again\n", info->irq);
+ log(TO_ALL, LOG_WARNING,
+ "Cannot change IRQ %i affinity: %s. Will never try again.\n",
+ info->irq, strerror(errsave));
info->flags |= IRQ_FLAG_AFFINITY_UNMANAGED;
}
--
2.40.1

View File

@ -0,0 +1,51 @@
From eee7917ef5272691b9d4ee6341463f3c78f7f909 Mon Sep 17 00:00:00 2001
From: Robin Jarry <rjarry@redhat.com>
Date: Wed, 12 Jul 2023 17:49:13 +0200
Subject: [PATCH 6/7] activate_mapping: only blacklist irq if error is
considered permanent
Some errors reported when writing to smp_affinity are transient. For
example, when a CPU interrupt controller does not have enough room to
map the IRQ, the kernel will return "No space left on device".
This kind of situation can change over time. Do not mark the IRQ
affinity as "unmanaged". Let irqbalance try again later.
Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
activate.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/activate.c b/activate.c
index 4418cda..7353692 100644
--- a/activate.c
+++ b/activate.c
@@ -91,9 +91,23 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
info->moved = 0; /*migration is done*/
error:
log(TO_ALL, LOG_WARNING,
- "Cannot change IRQ %i affinity: %s. Will never try again.\n",
+ "Cannot change IRQ %i affinity: %s\n",
info->irq, strerror(errsave));
- info->flags |= IRQ_FLAG_AFFINITY_UNMANAGED;
+ switch (errsave) {
+ case ENOSPC: /* Specified CPU APIC is full. */
+ case EAGAIN: /* Interrupted by signal. */
+ case EBUSY: /* Affinity change already in progress. */
+ case EINVAL: /* IRQ would be bound to no CPU. */
+ case ERANGE: /* CPU in mask is offline. */
+ case ENOMEM: /* Kernel cannot allocate CPU mask. */
+ /* Do not blacklist the IRQ on transient errors. */
+ break;
+ default:
+ /* Any other error is considered permanent. */
+ info->flags |= IRQ_FLAG_AFFINITY_UNMANAGED;
+ log(TO_ALL, LOG_WARNING, "IRQ %i affinity is now unmanaged\n",
+ info->irq);
+ }
}
void activate_mappings(void)
--
2.40.1

View File

@ -0,0 +1,28 @@
From bc7794dc78474c463a26926749537f23abc4c082 Mon Sep 17 00:00:00 2001
From: Robin Jarry <rjarry@redhat.com>
Date: Thu, 13 Jul 2023 20:49:16 +0200
Subject: [PATCH 7/7] activate_mapping: avoid logging error when there is none
Add missing return statement.
Fixes: 470a64b19062 ("activate_mapping: make sure to catch all errors")
Signed-off-by: Robin Jarry <rjarry@redhat.com>
---
activate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/activate.c b/activate.c
index 7353692..548a401 100644
--- a/activate.c
+++ b/activate.c
@@ -89,6 +89,7 @@ static void activate_mapping(struct irq_info *info, void *data __attribute__((un
if (ret < 0)
goto error;
info->moved = 0; /*migration is done*/
+ return;
error:
log(TO_ALL, LOG_WARNING,
"Cannot change IRQ %i affinity: %s\n",
--
2.40.1

View File

@ -1,6 +1,6 @@
Name: irqbalance
Version: 1.9.2
Release: 1%{?dist}
Release: 2%{?dist}
Epoch: 2
Summary: IRQ balancing daemon
@ -37,6 +37,13 @@ Patch11: 0010-Revert-Fix-CPU-number-condition-in-service-file.patch
Patch12: 0011-Fix-signedness-of-error-handling.patch
Patch13: 0012-Fix-it-so-we-actually-stop-when-we-hit-an-interrupt-.patch
Patch14: 0013-procinterrupts-fix-initialisation-of-regex_t-struct.patch
Patch15: 0001-activate_mapping-fflush-the-buffered-data-to-smp_aff.patch
Patch16: 0002-Revert-activate_mapping-fflush-the-buffered-data-to-.patch
Patch17: 0003-activate_mapping-avoid-use-after-free-when-affinity-.patch
Patch18: 0004-activate_mapping-make-sure-to-catch-all-errors.patch
Patch19: 0005-activate_mapping-report-error-reason.patch
Patch20: 0006-activate_mapping-only-blacklist-irq-if-error-is-cons.patch
Patch21: 0007-activate_mapping-avoid-logging-error-when-there-is-n.patch
%description
irqbalance is a daemon that evenly distributes IRQ load across
@ -58,6 +65,13 @@ multiple CPUs for enhanced performance.
%patch12 -p1
%patch13 -p1
%patch14 -p1
%patch15 -p1
%patch16 -p1
%patch17 -p1
%patch18 -p1
%patch19 -p1
%patch20 -p1
%patch21 -p1
%build
./autogen.sh
@ -100,6 +114,9 @@ fi
/sbin/chkconfig --del irqbalance >/dev/null 2>&1 || :
%changelog
* Tue Jul 18 2023 Tao Liu <ltao@redhat.com> - 2:1.9.2-2
- Rebase to latest upstream commit (50699824c7)
* Thu May 18 2023 Tao Liu <ltao@redhat.com> - 2:1.9.2-1
- Rebase to latest upstream commit (184c95029e)