Commit 98fa7a3e authored by Federico Motta's avatar Federico Motta Committed by Jens Axboe

block, bfq: fix asymmetric scenarios detection

Since commit 2d29c9f8 ("block, bfq: improve asymmetric scenarios
detection"), a scenario is defined asymmetric when one of the
following conditions holds:
- active bfq_queues have different weights
- one or more group of entities (bfq_queue or other groups of entities)
  are active
bfq grants fairness and low latency also in such asymmetric scenarios,
by plugging the dispatching of I/O if the bfq_queue in service happens
to be temporarily idle. This plugging may lower throughput, so it is
important to do it only when strictly needed.

By mistake, in commit '2d29c9f8' ("block, bfq: improve asymmetric
scenarios detection") the num_active_groups counter was firstly
incremented and subsequently decremented at any entity (group or
bfq_queue) weight change.

This is useless, because only transitions from active to inactive and
vice versa matter for that counter. Unfortunately this is also
incorrect in the following case: the entity at issue is a bfq_queue
and it is under weight raising. In fact in this case there is a
spurious increment of the num_active_groups counter.

This spurious increment may cause scenarios to be wrongly detected as
asymmetric, thus causing useless plugging and loss of throughput.

This commit fixes this issue by simply removing the above useless and
wrong increments and decrements.

Fixes: 2d29c9f8 ("block, bfq: improve asymmetric scenarios detection")
Tested-by: default avatarOleksandr Natalenko <oleksandr@natalenko.name>
Signed-off-by: default avatarFederico Motta <federico@willer.it>
Signed-off-by: default avatarPaolo Valente <paolo.valente@linaro.org>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 0a40a935
...@@ -792,24 +792,18 @@ __bfq_entity_update_weight_prio(struct bfq_service_tree *old_st, ...@@ -792,24 +792,18 @@ __bfq_entity_update_weight_prio(struct bfq_service_tree *old_st,
* queue, remove the entity from its old weight counter (if * queue, remove the entity from its old weight counter (if
* there is a counter associated with the entity). * there is a counter associated with the entity).
*/ */
if (prev_weight != new_weight) { if (prev_weight != new_weight && bfqq) {
if (bfqq) {
root = &bfqd->queue_weights_tree; root = &bfqd->queue_weights_tree;
__bfq_weights_tree_remove(bfqd, bfqq, root); __bfq_weights_tree_remove(bfqd, bfqq, root);
} else
bfqd->num_active_groups--;
} }
entity->weight = new_weight; entity->weight = new_weight;
/* /*
* Add the entity, if it is not a weight-raised queue, * Add the entity, if it is not a weight-raised queue,
* to the counter associated with its new weight. * to the counter associated with its new weight.
*/ */
if (prev_weight != new_weight) { if (prev_weight != new_weight && bfqq && bfqq->wr_coeff == 1) {
if (bfqq && bfqq->wr_coeff == 1) {
/* If we get here, root has been initialized. */ /* If we get here, root has been initialized. */
bfq_weights_tree_add(bfqd, bfqq, root); bfq_weights_tree_add(bfqd, bfqq, root);
} else
bfqd->num_active_groups++;
} }
new_st->wsum += entity->weight; new_st->wsum += entity->weight;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment