Skip to content

Commit

Permalink
block,bfq: refactor device-idling logic
Browse files Browse the repository at this point in the history 
The logic that decides whether to idle the device is scattered across
three functions. Almost all of the logic is in the function
bfq_bfqq_may_idle, but (1) part of the decision is made in
bfq_update_idle_window, and (2) the function bfq_bfqq_must_idle may
switch off idling regardless of the output of bfq_bfqq_may_idle. In
addition, both bfq_update_idle_window and bfq_bfqq_must_idle make
their decisions as a function of parameters that are used, for similar
purposes, also in bfq_bfqq_may_idle. This commit addresses these
issues by moving all the logic into bfq_bfqq_may_idle.

Signed-off-by: Paolo Valente <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
@Algodev-github @axboe
Algodev-github authored and axboe committed Aug 11, 2017
1 parent e743eb1 commit d5be3fe
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 62 deletions.
117 changes: 61 additions & 56 deletions block/bfq-iosched.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ BFQ_BFQQ_FNS(busy);
BFQ_BFQQ_FNS(wait_request);
BFQ_BFQQ_FNS(non_blocking_wait_rq);
BFQ_BFQQ_FNS(fifo_expire);
BFQ_BFQQ_FNS(idle_window);
BFQ_BFQQ_FNS(has_short_ttime);
BFQ_BFQQ_FNS(sync);
BFQ_BFQQ_FNS(IO_bound);
BFQ_BFQQ_FNS(in_large_burst);
Expand Down Expand Up @@ -731,10 +731,10 @@ bfq_bfqq_resume_state(struct bfq_queue *bfqq, struct bfq_data *bfqd,
unsigned int old_wr_coeff = bfqq->wr_coeff;
bool busy = bfq_already_existing && bfq_bfqq_busy(bfqq);

if (bic->saved_idle_window)
bfq_mark_bfqq_idle_window(bfqq);
if (bic->saved_has_short_ttime)
bfq_mark_bfqq_has_short_ttime(bfqq);
else
bfq_clear_bfqq_idle_window(bfqq);
bfq_clear_bfqq_has_short_ttime(bfqq);

if (bic->saved_IO_bound)
bfq_mark_bfqq_IO_bound(bfqq);
Expand Down Expand Up @@ -2012,7 +2012,7 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
return;

bic->saved_ttime = bfqq->ttime;
bic->saved_idle_window = bfq_bfqq_idle_window(bfqq);
bic->saved_has_short_ttime = bfq_bfqq_has_short_ttime(bfqq);
bic->saved_IO_bound = bfq_bfqq_IO_bound(bfqq);
bic->saved_in_large_burst = bfq_bfqq_in_large_burst(bfqq);
bic->was_in_burst_list = !hlist_unhashed(&bfqq->burst_list_node);
Expand Down Expand Up @@ -3038,8 +3038,8 @@ void bfq_bfqq_expire(struct bfq_data *bfqd,
}

bfq_log_bfqq(bfqd, bfqq,
"expire (%d, slow %d, num_disp %d, idle_win %d)", reason,
slow, bfqq->dispatched, bfq_bfqq_idle_window(bfqq));
"expire (%d, slow %d, num_disp %d, short_ttime %d)", reason,
slow, bfqq->dispatched, bfq_bfqq_has_short_ttime(bfqq));

/*
* Increase, decrease or leave budget unchanged according to
Expand Down Expand Up @@ -3121,6 +3121,18 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq)
if (bfqd->strict_guarantees)
return true;

/*
* Idling is performed only if slice_idle > 0. In addition, we
* do not idle if
* (a) bfqq is async
* (b) bfqq is in the idle io prio class: in this case we do
* not idle because we want to minimize the bandwidth that
* queues in this class can steal to higher-priority queues
*/
if (bfqd->bfq_slice_idle == 0 || !bfq_bfqq_sync(bfqq) ||
bfq_class_idle(bfqq))
return false;

/*
* The next variable takes into account the cases where idling
* boosts the throughput.
Expand All @@ -3142,7 +3154,7 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq)
*/
idling_boosts_thr = !bfqd->hw_tag ||
(!blk_queue_nonrot(bfqd->queue) && bfq_bfqq_IO_bound(bfqq) &&
bfq_bfqq_idle_window(bfqq));
bfq_bfqq_has_short_ttime(bfqq));

/*
* The value of the next variable,
Expand Down Expand Up @@ -3313,16 +3325,13 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq)
asymmetric_scenario && !bfq_bfqq_in_large_burst(bfqq);

/*
* We have now all the components we need to compute the return
* value of the function, which is true only if both the following
* conditions hold:
* 1) bfqq is sync, because idling make sense only for sync queues;
* 2) idling either boosts the throughput (without issues), or
* is necessary to preserve service guarantees.
* We have now all the components we need to compute the
* return value of the function, which is true only if idling
* either boosts the throughput (without issues), or is
* necessary to preserve service guarantees.
*/
return bfq_bfqq_sync(bfqq) &&
(idling_boosts_thr_without_issues ||
idling_needed_for_service_guarantees);
return idling_boosts_thr_without_issues ||
idling_needed_for_service_guarantees;
}

/*
Expand All @@ -3338,10 +3347,7 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq)
*/
static bool bfq_bfqq_must_idle(struct bfq_queue *bfqq)
{
struct bfq_data *bfqd = bfqq->bfqd;

return RB_EMPTY_ROOT(&bfqq->sort_list) && bfqd->bfq_slice_idle != 0 &&
bfq_bfqq_may_idle(bfqq);
return RB_EMPTY_ROOT(&bfqq->sort_list) && bfq_bfqq_may_idle(bfqq);
}

/*
Expand Down Expand Up @@ -3783,7 +3789,6 @@ bfq_set_next_ioprio_data(struct bfq_queue *bfqq, struct bfq_io_cq *bic)
case IOPRIO_CLASS_IDLE:
bfqq->new_ioprio_class = IOPRIO_CLASS_IDLE;
bfqq->new_ioprio = 7;
bfq_clear_bfqq_idle_window(bfqq);
break;
}

Expand Down Expand Up @@ -3843,8 +3848,14 @@ static void bfq_init_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
bfq_set_next_ioprio_data(bfqq, bic);

if (is_sync) {
/*
* No need to mark as has_short_ttime if in
* idle_class, because no device idling is performed
* for queues in idle class
*/
if (!bfq_class_idle(bfqq))
bfq_mark_bfqq_idle_window(bfqq);
/* tentatively mark as has_short_ttime */
bfq_mark_bfqq_has_short_ttime(bfqq);
bfq_mark_bfqq_sync(bfqq);
bfq_mark_bfqq_just_created(bfqq);
} else
Expand Down Expand Up @@ -3985,46 +3996,42 @@ bfq_update_io_seektime(struct bfq_data *bfqd, struct bfq_queue *bfqq,
blk_rq_sectors(rq) < BFQQ_SECT_THR_NONROT);
}

/*
* Disable idle window if the process thinks too long or seeks so much that
* it doesn't matter.
*/
static void bfq_update_idle_window(struct bfq_data *bfqd,
struct bfq_queue *bfqq,
struct bfq_io_cq *bic)
static void bfq_update_has_short_ttime(struct bfq_data *bfqd,
struct bfq_queue *bfqq,
struct bfq_io_cq *bic)
{
int enable_idle;
bool has_short_ttime = true;

/* Don't idle for async or idle io prio class. */
if (!bfq_bfqq_sync(bfqq) || bfq_class_idle(bfqq))
/*
* No need to update has_short_ttime if bfqq is async or in
* idle io prio class, or if bfq_slice_idle is zero, because
* no device idling is performed for bfqq in this case.
*/
if (!bfq_bfqq_sync(bfqq) || bfq_class_idle(bfqq) ||
bfqd->bfq_slice_idle == 0)
return;

/* Idle window just restored, statistics are meaningless. */
if (time_is_after_eq_jiffies(bfqq->split_time +
bfqd->bfq_wr_min_idle_time))
return;

enable_idle = bfq_bfqq_idle_window(bfqq);

/* Think time is infinite if no process is linked to
* bfqq. Otherwise check average think time to
* decide whether to mark as has_short_ttime
*/
if (atomic_read(&bic->icq.ioc->active_ref) == 0 ||
bfqd->bfq_slice_idle == 0 ||
(bfqd->hw_tag && BFQQ_SEEKY(bfqq) &&
bfqq->wr_coeff == 1))
enable_idle = 0;
else if (bfq_sample_valid(bfqq->ttime.ttime_samples)) {
if (bfqq->ttime.ttime_mean > bfqd->bfq_slice_idle &&
bfqq->wr_coeff == 1)
enable_idle = 0;
else
enable_idle = 1;
}
bfq_log_bfqq(bfqd, bfqq, "update_idle_window: enable_idle %d",
enable_idle);
(bfq_sample_valid(bfqq->ttime.ttime_samples) &&
bfqq->ttime.ttime_mean > bfqd->bfq_slice_idle))
has_short_ttime = false;

bfq_log_bfqq(bfqd, bfqq, "update_has_short_ttime: has_short_ttime %d",
has_short_ttime);

if (enable_idle)
bfq_mark_bfqq_idle_window(bfqq);
if (has_short_ttime)
bfq_mark_bfqq_has_short_ttime(bfqq);
else
bfq_clear_bfqq_idle_window(bfqq);
bfq_clear_bfqq_has_short_ttime(bfqq);
}

/*
Expand All @@ -4040,14 +4047,12 @@ static void bfq_rq_enqueued(struct bfq_data *bfqd, struct bfq_queue *bfqq,
bfqq->meta_pending++;

bfq_update_io_thinktime(bfqd, bfqq);
bfq_update_has_short_ttime(bfqd, bfqq, bic);
bfq_update_io_seektime(bfqd, bfqq, rq);
if (bfqq->entity.service > bfq_max_budget(bfqd) / 8 ||
!BFQQ_SEEKY(bfqq))
bfq_update_idle_window(bfqd, bfqq, bic);

bfq_log_bfqq(bfqd, bfqq,
"rq_enqueued: idle_window=%d (seeky %d)",
bfq_bfqq_idle_window(bfqq), BFQQ_SEEKY(bfqq));
"rq_enqueued: has_short_ttime=%d (seeky %d)",
bfq_bfqq_has_short_ttime(bfqq), BFQQ_SEEKY(bfqq));

bfqq->last_request_pos = blk_rq_pos(rq) + blk_rq_sectors(rq);

Expand Down
12 changes: 6 additions & 6 deletions block/bfq-iosched.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,11 @@ struct bfq_io_cq {
uint64_t blkcg_serial_nr; /* the current blkcg serial */
#endif
/*
* Snapshot of the idle window before merging; taken to
* remember this value while the queue is merged, so as to be
* able to restore it in case of split.
* Snapshot of the has_short_time flag before merging; taken
* to remember its value while the queue is merged, so as to
* be able to restore it in case of split.
*/
bool saved_idle_window;
bool saved_has_short_ttime;
/*
* Same purpose as the previous two fields for the I/O bound
* classification of a queue.
Expand Down Expand Up @@ -626,7 +626,7 @@ enum bfqq_state_flags {
* without idling the device
*/
BFQQF_fifo_expire, /* FIFO checked in this slice */
BFQQF_idle_window, /* slice idling enabled */
BFQQF_has_short_ttime, /* queue has a short think time */
BFQQF_sync, /* synchronous queue */
BFQQF_IO_bound, /*
* bfqq has timed-out at least once
Expand Down Expand Up @@ -655,7 +655,7 @@ BFQ_BFQQ_FNS(busy);
BFQ_BFQQ_FNS(wait_request);
BFQ_BFQQ_FNS(non_blocking_wait_rq);
BFQ_BFQQ_FNS(fifo_expire);
BFQ_BFQQ_FNS(idle_window);
BFQ_BFQQ_FNS(has_short_ttime);
BFQ_BFQQ_FNS(sync);
BFQ_BFQQ_FNS(IO_bound);
BFQ_BFQQ_FNS(in_large_burst);
Expand Down

0 comments on commit d5be3fe

Please sign in to comment.