Commit 431b17f9 authored by Paolo Valente's avatar Paolo Valente Committed by Jens Axboe
Browse files

block, bfq: don't change ioprio class for a bfq_queue on a service tree 



On each deactivation or re-scheduling (after being served) of a
bfq_queue, BFQ invokes the function __bfq_entity_update_weight_prio(),
to perform pending updates of ioprio, weight and ioprio class for the
bfq_queue. BFQ also invokes this function on I/O-request dispatches,
to raise or lower weights more quickly when needed, thereby improving
latency. However, the entity representing the bfq_queue may be on the
active (sub)tree of a service tree when this happens, and, although
with a very low probability, the bfq_queue may happen to also have a
pending change of its ioprio class. If both conditions hold when
__bfq_entity_update_weight_prio() is invoked, then the entity moves to
a sort of hybrid state: the new service tree for the entity, as
returned by bfq_entity_service_tree(), differs from service tree on
which the entity still is. The functions that handle activations and
deactivations of entities do not cope with such a hybrid state (and
would need to become more complex to cope).

This commit addresses this issue by just making
__bfq_entity_update_weight_prio() not perform also a possible pending
change of ioprio class, when invoked on an I/O-request dispatch for a
bfq_queue. Such a change is thus postponed to when
__bfq_entity_update_weight_prio() is invoked on deactivation or
re-scheduling of the bfq_queue.

Reported-by: default avatarMarco Piazza <mpiazza@gmail.com>
Reported-by: default avatarLaurentiu Nicola <lnicola@dend.ro>
Signed-off-by: default avatarPaolo Valente <paolo.valente@linaro.org>
Tested-by: default avatarMarco Piazza <mpiazza@gmail.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 7a69f9c6

block/bfq-iosched.c

+10 −4
Original line number Diff line number Diff line
@@ -3483,11 +3483,17 @@ static void bfq_update_wr_data(struct bfq_data *bfqd, struct bfq_queue *bfqq) 
			}

		}

	}

	/* Update weight both if it must be raised and if it must be lowered */

	/*

	 * To improve latency (for this or other queues), immediately

	 * update weight both if it must be raised and if it must be

	 * lowered. Since, entity may be on some active tree here, and

	 * might have a pending change of its ioprio class, invoke

	 * next function with the last parameter unset (see the

	 * comments on the function).

	 */

	if ((entity->weight > entity->orig_weight) != (bfqq->wr_coeff > 1))

		__bfq_entity_update_weight_prio(

			bfq_entity_service_tree(entity),

			entity);

		__bfq_entity_update_weight_prio(bfq_entity_service_tree(entity),

						entity, false);

}



/*

block/bfq-iosched.h

+2 −1
Original line number Diff line number Diff line
@@ -892,7 +892,8 @@ void bfq_put_idle_entity(struct bfq_service_tree *st, 
			 struct bfq_entity *entity);

struct bfq_service_tree *

__bfq_entity_update_weight_prio(struct bfq_service_tree *old_st,

				struct bfq_entity *entity);

				struct bfq_entity *entity,

				bool update_class_too);

void bfq_bfqq_served(struct bfq_queue *bfqq, int served);

void bfq_bfqq_charge_time(struct bfq_data *bfqd, struct bfq_queue *bfqq,

			  unsigned long time_ms);

block/bfq-wf2q.c

+34 −5
Original line number Diff line number Diff line
@@ -694,10 +694,28 @@ struct bfq_service_tree *bfq_entity_service_tree(struct bfq_entity *entity) 
	return sched_data->service_tree + idx;

}





/*

 * Update weight and priority of entity. If update_class_too is true,

 * then update the ioprio_class of entity too.

 *

 * The reason why the update of ioprio_class is controlled through the

 * last parameter is as follows. Changing the ioprio class of an

 * entity implies changing the destination service trees for that

 * entity. If such a change occurred when the entity is already on one

 * of the service trees for its previous class, then the state of the

 * entity would become more complex: none of the new possible service

 * trees for the entity, according to bfq_entity_service_tree(), would

 * match any of the possible service trees on which the entity

 * is. Complex operations involving these trees, such as entity

 * activations and deactivations, should take into account this

 * additional complexity.  To avoid this issue, this function is

 * invoked with update_class_too unset in the points in the code where

 * entity may happen to be on some tree.

 */

struct bfq_service_tree *

__bfq_entity_update_weight_prio(struct bfq_service_tree *old_st,

				struct bfq_entity *entity)

				struct bfq_entity *entity,

				bool update_class_too)

{

	struct bfq_service_tree *new_st = old_st;
@@ -739,8 +757,14 @@ __bfq_entity_update_weight_prio(struct bfq_service_tree *old_st, 
				  bfq_weight_to_ioprio(entity->orig_weight);

		}



		if (bfqq)

		if (bfqq && update_class_too)

			bfqq->ioprio_class = bfqq->new_ioprio_class;



		/*

		 * Reset prio_changed only if the ioprio_class change

		 * is not pending any longer.

		 */

		if (!bfqq || bfqq->ioprio_class == bfqq->new_ioprio_class)

			entity->prio_changed = 0;



		/*
@@ -867,7 +891,12 @@ static void bfq_update_fin_time_enqueue(struct bfq_entity *entity, 
{

	struct bfq_queue *bfqq = bfq_entity_to_bfqq(entity);



	st = __bfq_entity_update_weight_prio(st, entity);

	/*

	 * When this function is invoked, entity is not in any service

	 * tree, then it is safe to invoke next function with the last

	 * parameter set (see the comments on the function).

	 */

	st = __bfq_entity_update_weight_prio(st, entity, true);

	bfq_calc_finish(entity, entity->budget);



	/*

CRA Git | Maintained and supported by SUSTech CRA and CCSE