On Sat 14-05-22 17:05:22, Yu Kuai wrote:
> 'wr_or_deserves_wr' is only used in bfq_update_bfqq_wr_on_rq_arrival(),
> which is only called from bfq_bfqq_handle_idle_busy_switch() in specific
> code branch, thus there is no need to precaculate 'wr_or_deserves_wr'
> each time bfq_bfqq_handle_idle_busy_switch() is called.
>
> Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx>
With this patch there's the same problem as with the previous one. Some of
the variables passed to bfq_update_bfqq_wr_on_rq_arrival() (in_burst,
soft_rt) would actually evaluate differently later in the function.
Honza
> ---
> block/bfq-iosched.c | 110 +++++++++++++++++++++++++-------------------
> 1 file changed, 62 insertions(+), 48 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 1e57d76c8dd3..cea8cb3f5ee2 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -1624,15 +1624,65 @@ static unsigned long bfq_smallest_from_now(void)
> return jiffies - MAX_JIFFY_OFFSET;
> }
>
> +/*
> + * bfqq deserves to be weight-raised if:
> + * - it is sync,
> + * - it does not belong to a large burst,
> + * - it has been idle for enough time or is soft real-time,
> + * - is linked to a bfq_io_cq (it is not shared in any sense),
> + * - has a default weight (otherwise we assume the user wanted
> + * to control its weight explicitly)
> + *
> + * Merged bfq_queues are kept out of weight-raising
> + * (low-latency) mechanisms. The reason is that these queues
> + * are usually created for non-interactive and
> + * non-soft-real-time tasks. Yet this is not the case for
> + * stably-merged queues. These queues are merged just because
> + * they are created shortly after each other. So they may
> + * easily serve the I/O of an interactive or soft-real time
> + * application, if the application happens to spawn multiple
> + * processes. So let also stably-merged queued enjoy weight
> + * raising.
> + */
> +static bool bfqq_wr_or_deserves_wr(struct bfq_data *bfqd,
> + struct bfq_queue *bfqq,
> + struct request *rq,
> + bool interactive, bool soft_rt)
> +{
> + if (!bfqd->low_latency)
> + return false;
> +
> + if (bfqq->wr_coeff > 1)
> + return true;
> +
> + if (!bfq_bfqq_sync(bfqq))
> + return false;
> +
> + if (!bfqq->bic && !RQ_BIC(rq)->stably_merged)
> + return false;
> +
> + if (!interactive && !soft_rt)
> + return false;
> +
> + return true;
> +}
> +
> static void bfq_update_bfqq_wr_on_rq_arrival(struct bfq_data *bfqd,
> struct bfq_queue *bfqq,
> unsigned int old_wr_coeff,
> - bool wr_or_deserves_wr,
> - bool interactive,
> - bool in_burst,
> - bool soft_rt)
> -{
> - if (old_wr_coeff == 1 && wr_or_deserves_wr) {
> + struct request *rq,
> + bool interactive)
> +{
> + bool in_burst = bfq_bfqq_in_large_burst(bfqq);
> + bool soft_rt = bfqd->bfq_wr_max_softrt_rate > 0 &&
> + !BFQQ_TOTALLY_SEEKY(bfqq) &&
> + !in_burst &&
> + time_is_before_jiffies(bfqq->soft_rt_next_start) &&
> + bfqq->dispatched == 0 &&
> + bfqq->entity.new_weight == 40;
> +
> + if (old_wr_coeff == 1 &&
> + bfqq_wr_or_deserves_wr(bfqd, bfqq, rq, interactive, soft_rt)) {
> /* start a weight-raising period */
> if (interactive) {
> bfqq->service_from_wr = 0;
> @@ -1674,9 +1724,9 @@ static void bfq_update_bfqq_wr_on_rq_arrival(struct bfq_data *bfqd,
> if (interactive) { /* update wr coeff and duration */
> bfqq->wr_coeff = bfqd->bfq_wr_coeff;
> bfqq->wr_cur_max_time = bfq_wr_duration(bfqd);
> - } else if (in_burst)
> + } else if (in_burst) {
> bfqq->wr_coeff = 1;
> - else if (soft_rt) {
> + } else if (soft_rt) {
> /*
> * The application is now or still meeting the
> * requirements for being deemed soft rt. We
> @@ -1768,44 +1818,11 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
> struct request *rq,
> bool *interactive)
> {
> - bool soft_rt, in_burst, wr_or_deserves_wr,
> - idle_for_long_time = bfq_bfqq_idle_for_long_time(bfqd, bfqq);
> + bool in_burst = bfq_bfqq_in_large_burst(bfqq);
> + bool idle_for_long_time = bfq_bfqq_idle_for_long_time(bfqd, bfqq);
>
> - /*
> - * bfqq deserves to be weight-raised if:
> - * - it is sync,
> - * - it does not belong to a large burst,
> - * - it has been idle for enough time or is soft real-time,
> - * - is linked to a bfq_io_cq (it is not shared in any sense),
> - * - has a default weight (otherwise we assume the user wanted
> - * to control its weight explicitly)
> - */
> - in_burst = bfq_bfqq_in_large_burst(bfqq);
> - soft_rt = bfqd->bfq_wr_max_softrt_rate > 0 &&
> - !BFQQ_TOTALLY_SEEKY(bfqq) &&
> - !in_burst &&
> - time_is_before_jiffies(bfqq->soft_rt_next_start) &&
> - bfqq->dispatched == 0 &&
> - bfqq->entity.new_weight == 40;
> *interactive = !in_burst && idle_for_long_time &&
> bfqq->entity.new_weight == 40;
> - /*
> - * Merged bfq_queues are kept out of weight-raising
> - * (low-latency) mechanisms. The reason is that these queues
> - * are usually created for non-interactive and
> - * non-soft-real-time tasks. Yet this is not the case for
> - * stably-merged queues. These queues are merged just because
> - * they are created shortly after each other. So they may
> - * easily serve the I/O of an interactive or soft-real time
> - * application, if the application happens to spawn multiple
> - * processes. So let also stably-merged queued enjoy weight
> - * raising.
> - */
> - wr_or_deserves_wr = bfqd->low_latency &&
> - (bfqq->wr_coeff > 1 ||
> - (bfq_bfqq_sync(bfqq) &&
> - (bfqq->bic || RQ_BIC(rq)->stably_merged) &&
> - (*interactive || soft_rt)));
>
> /*
> * If bfqq happened to be activated in a burst, but has been
> @@ -1840,11 +1857,8 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
> if (time_is_before_jiffies(bfqq->split_time +
> bfqd->bfq_wr_min_idle_time)) {
> bfq_update_bfqq_wr_on_rq_arrival(bfqd, bfqq,
> - old_wr_coeff,
> - wr_or_deserves_wr,
> - *interactive,
> - in_burst,
> - soft_rt);
> + old_wr_coeff, rq,
> + *interactive);
>
> if (old_wr_coeff != bfqq->wr_coeff)
> bfqq->entity.prio_changed = 1;
> --
> 2.31.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
