| Message ID | 20220514090522.1669270-9-yukuai3@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | multiple cleanup patches for bfq | expand |
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@huawei.com> 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 >
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;
'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@huawei.com> --- block/bfq-iosched.c | 110 +++++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 48 deletions(-)