On Fri, Jul 03, 2015 at 09:42:30PM +0900, Seiichi Ikarashi wrote:
> 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);
>
applied, thanks!
Neil
