[PATCH] irqbalance: fix find_best_object() for the same d->load value

Seiichi Ikarashi s.ikarashi at jp.fujitsu.com
Fri Jul 3 05:42:30 PDT 2015


find_best_object() has a problem that three or more topo_obj, which have
the same ->load value, are involved. The comparison of the ->interrupts
length are always held with the one, which is registered in best->best.
Think about the following case:

  topo_obj A, the length of ->interrupts is 3.
           B,                               1.
           C,                               2.

  and find_best_object() eats them in the order of A, B, and C.

Here, find_best_object() wrongly chooses C, instead of B, because
the length of -> interrupts of B is not compared with that of C.

To fix this, remember only the best one and compare it with the next one.

Signed-off-by: Seiichi Ikarashi <s.ikarashi at jp.fujitsu.com>

---
 placement.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/placement.c b/placement.c
index cc0d273..f476e02 100644
--- a/placement.c
+++ b/placement.c
@@ -33,7 +33,6 @@ GList *rebalance_irq_list;
 
 struct obj_placement {
 		struct topo_obj *best;
-		struct topo_obj *least_irqs;
 		uint64_t best_cost;
 		struct irq_info *info;
 };
@@ -78,12 +77,10 @@ static void find_best_object(struct topo_obj *d, void *data)
 	if (newload < best->best_cost) {
 		best->best = d;
 		best->best_cost = newload;
-		best->least_irqs = NULL;
-	}
-
-	if (newload == best->best_cost) {
-		if (g_list_length(d->interrupts) < g_list_length(best->best->interrupts))
-			best->least_irqs = d;
+	} else if (newload == best->best_cost) {
+		if (g_list_length(d->interrupts) < g_list_length(best->best->interrupts)) {
+			best->best = d;
+		}
 	}
 }
 
@@ -120,12 +117,11 @@ static void find_best_object_for_irq(struct irq_info *info, void *data)
 
 	place.info = info;
 	place.best = NULL;
-	place.least_irqs = NULL;
 	place.best_cost = ULLONG_MAX;
 
 	for_each_object(d->children, find_best_object, &place);
 
-	asign = place.least_irqs ? place.least_irqs : place.best;
+	asign = place.best;
 
 	if (asign) {
 		migrate_irq(&d->interrupts, &asign->interrupts, info);
@@ -168,12 +164,11 @@ static void place_irq_in_node(struct irq_info *info, void *data __attribute__((u
 find_placement:
 	place.best_cost = ULLONG_MAX;
 	place.best = NULL;
-	place.least_irqs = NULL;
 	place.info = info;
 
 	for_each_object(numa_nodes, find_best_object, &place);
 
-	asign = place.least_irqs ? place.least_irqs : place.best;
+	asign = place.best;
 
 	if (asign) {
 		migrate_irq(&rebalance_irq_list, &asign->interrupts, info);



More information about the irqbalance mailing list