* [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq
@ 2022-12-22 19:16 Kemeng Shi
2022-12-22 19:16 ` [PATCH RESEND v2 01/10] block, bfq: correctly raise inject limit in bfq_choose_bfqq_for_injection Kemeng Shi
` (9 more replies)
0 siblings, 10 replies; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
Hi, this series contain two patches to fix bug in request injection
mechanism and some random cleancode patches.
Thanks!
---
v2:
-improve git log.
---
Kemeng Shi (10):
block, bfq: correctly raise inject limit in
bfq_choose_bfqq_for_injection
block, bfq: remove unsed parameter reason in bfq_bfqq_is_slow
block, bfq: initialize bfqq->decrease_time_jif correctly
block, bfq: use helper macro RQ_BFQQ to get bfqq of request
block, bfq: remove unnecessary dereference to get async_bfqq
block, bfq: remove redundant bfqd->rq_in_driver > 0 check in
bfq_add_request
block, bfq: remove redundant check in bfq_put_cooperator
block, bfq: remove unnecessary goto tag in bfq_dispatch_rq_from_bfqq
block, bfq: remove unused bfq_wr_max_time in struct bfq_data
block, bfq: remove check of bfq_wr_max_softrt_rate which is always
greater than 0
block/bfq-iosched.c | 49 +++++++++++++++++----------------------------
block/bfq-iosched.h | 2 --
2 files changed, 18 insertions(+), 33 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 01/10] block, bfq: correctly raise inject limit in bfq_choose_bfqq_for_injection
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:13 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 02/10] block, bfq: remove unsed parameter reason in bfq_bfqq_is_slow Kemeng Shi
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
Function bfq_choose_bfqq_for_injection may temporarily raise inject limit
to one request if current inject_limit is 0 before search of the source
queue for injection. However the search below will reset inject limit to
bfqd->in_service_queue which is zero for raised inject limit. Then the
temporarily raised inject limit never works as expected.
Assigment limit to bfqd->in_service_queue in search is needed as limit
maybe overwriten to min_t(unsigned int, 1, limit) for condition that
a large in-flight request is on non-rotational devices in found queue.
So we need to reset limit to bfqd->in_service_queue for normal case.
Actually, we have already make sure bfqd->rq_in_driver is < limit before
search, then
-Limit is >= 1 as bfqd->rq_in_driver is >= 0. Then min_t(unsigned int,
1, limit) is always 1. So we can simply check bfqd->rq_in_driver with
1 instead of result of min_t(unsigned int, 1, limit) for larget request in
non-rotational device case to avoid overwritting limit and the bug is gone.
-For normal case, we have already check bfqd->rq_in_driver is < limit,
so we can return found bfqq unconditionally to remove unncessary check.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index a72304c728fc..1220c41fc767 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4641,12 +4641,10 @@ bfq_choose_bfqq_for_injection(struct bfq_data *bfqd)
*/
if (blk_queue_nonrot(bfqd->queue) &&
blk_rq_sectors(bfqq->next_rq) >=
- BFQQ_SECT_THR_NONROT)
- limit = min_t(unsigned int, 1, limit);
- else
- limit = in_serv_bfqq->inject_limit;
-
- if (bfqd->rq_in_driver < limit) {
+ BFQQ_SECT_THR_NONROT &&
+ bfqd->rq_in_driver >= 1)
+ continue;
+ else {
bfqd->rqs_injected = true;
return bfqq;
}
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 02/10] block, bfq: remove unsed parameter reason in bfq_bfqq_is_slow
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
2022-12-22 19:16 ` [PATCH RESEND v2 01/10] block, bfq: correctly raise inject limit in bfq_choose_bfqq_for_injection Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:14 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 03/10] block, bfq: initialize bfqq->decrease_time_jif correctly Kemeng Shi
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
Parameter reason is never used, just remove it.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 1220c41fc767..5a6d9e8c329d 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4066,8 +4066,7 @@ static void __bfq_bfqq_recalc_budget(struct bfq_data *bfqd,
* function to evaluate the I/O speed of a process.
*/
static bool bfq_bfqq_is_slow(struct bfq_data *bfqd, struct bfq_queue *bfqq,
- bool compensate, enum bfqq_expiration reason,
- unsigned long *delta_ms)
+ bool compensate, unsigned long *delta_ms)
{
ktime_t delta_ktime;
u32 delta_usecs;
@@ -4263,7 +4262,7 @@ void bfq_bfqq_expire(struct bfq_data *bfqd,
/*
* Check whether the process is slow (see bfq_bfqq_is_slow).
*/
- slow = bfq_bfqq_is_slow(bfqd, bfqq, compensate, reason, &delta);
+ slow = bfq_bfqq_is_slow(bfqd, bfqq, compensate, &delta);
/*
* As above explained, charge slow (typically seeky) and
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 03/10] block, bfq: initialize bfqq->decrease_time_jif correctly
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
2022-12-22 19:16 ` [PATCH RESEND v2 01/10] block, bfq: correctly raise inject limit in bfq_choose_bfqq_for_injection Kemeng Shi
2022-12-22 19:16 ` [PATCH RESEND v2 02/10] block, bfq: remove unsed parameter reason in bfq_bfqq_is_slow Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:30 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 04/10] block, bfq: use helper macro RQ_BFQQ to get bfqq of request Kemeng Shi
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
Inject limit is updated or reset when time_is_before_eq_jiffies(
decrease_time_jif + several msecs) or think-time state changes.
decrease_time_jif is initialized to 0 and will be set to current jiffies
when inject limit is updated or reset. If the jiffies is slightly greater
than LONG_MAX, time_is_after_eq_jiffies(0) will keep for a long time, so as
time_is_after_eq_jiffies(decrease_time_jif + several msecs). If the
think-time state never chages, then the injection will not work as expected
for long time.
To be more specific:
Function bfq_update_inject_limit maybe triggered when jiffies pasts
decrease_time_jif + msecs_to_jiffies(10) in bfq_add_request by setting
bfqd->wait_dispatch to true.
Function bfq_reset_inject_limit are called in two conditions:
1. jiffies pasts bfqq->decrease_time_jif + msecs_to_jiffies(1000) in
function bfq_add_request.
2. jiffies pasts bfqq->decrease_time_jif + msecs_to_jiffies(100) or
bfq think-time state change from short to long.
Fix this by initializing bfqq->decrease_time_jif to current jiffies
to trigger service injection soon when service injection conditions
are met.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 5a6d9e8c329d..9441fc98961a 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5491,6 +5491,8 @@ static void bfq_init_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
/* first request is almost certainly seeky */
bfqq->seek_history = 1;
+
+ bfqq->decrease_time_jif = jiffies;
}
static struct bfq_queue **bfq_async_queue_prio(struct bfq_data *bfqd,
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 04/10] block, bfq: use helper macro RQ_BFQQ to get bfqq of request
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
` (2 preceding siblings ...)
2022-12-22 19:16 ` [PATCH RESEND v2 03/10] block, bfq: initialize bfqq->decrease_time_jif correctly Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:31 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 05/10] block, bfq: remove unnecessary dereference to get async_bfqq Kemeng Shi
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
Use helper macro RQ_BFQQ to get bfqq of request.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 9441fc98961a..c3c4c83ee95f 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -6683,14 +6683,14 @@ static struct bfq_queue *bfq_init_rq(struct request *rq)
return NULL;
/*
- * Assuming that elv.priv[1] is set only if everything is set
+ * Assuming that RQ_BFQQ(rq) is set only if everything is set
* for this rq. This holds true, because this function is
* invoked only for insertion or merging, and, after such
* events, a request cannot be manipulated any longer before
* being removed from bfq.
*/
- if (rq->elv.priv[1])
- return rq->elv.priv[1];
+ if (RQ_BFQQ(rq))
+ return RQ_BFQQ(rq);
bic = icq_to_bic(rq->elv.icq);
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 05/10] block, bfq: remove unnecessary dereference to get async_bfqq
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
` (3 preceding siblings ...)
2022-12-22 19:16 ` [PATCH RESEND v2 04/10] block, bfq: use helper macro RQ_BFQQ to get bfqq of request Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:34 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 06/10] block, bfq: remove redundant bfqd->rq_in_driver > 0 check in bfq_add_request Kemeng Shi
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
The async_bfqq is assigned with bfqq->bic->bfqq[0], use it directly.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index c3c4c83ee95f..ebcafe3c4c3b 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4835,7 +4835,7 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
icq_to_bic(async_bfqq->next_rq->elv.icq) == bfqq->bic &&
bfq_serv_to_charge(async_bfqq->next_rq, async_bfqq) <=
bfq_bfqq_budget_left(async_bfqq))
- bfqq = bfqq->bic->bfqq[0];
+ bfqq = async_bfqq;
else if (bfqq->waker_bfqq &&
bfq_bfqq_busy(bfqq->waker_bfqq) &&
bfqq->waker_bfqq->next_rq &&
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 06/10] block, bfq: remove redundant bfqd->rq_in_driver > 0 check in bfq_add_request
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
` (4 preceding siblings ...)
2022-12-22 19:16 ` [PATCH RESEND v2 05/10] block, bfq: remove unnecessary dereference to get async_bfqq Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:54 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 07/10] block, bfq: remove redundant check in bfq_put_cooperator Kemeng Shi
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
The bfqd->rq_in_driver > 0 check is along with previous
"bfqd->rq_in_driver == 0 ||" check, so no need to re-check
bfqd->rq_in_driver > 0.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index ebcafe3c4c3b..7c91d16dbf6f 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2204,8 +2204,7 @@ static void bfq_add_request(struct request *rq)
*/
if (bfqq == bfqd->in_service_queue &&
(bfqd->rq_in_driver == 0 ||
- (bfqq->last_serv_time_ns > 0 &&
- bfqd->rqs_injected && bfqd->rq_in_driver > 0)) &&
+ (bfqq->last_serv_time_ns > 0 && bfqd->rqs_injected)) &&
time_is_before_eq_jiffies(bfqq->decrease_time_jif +
msecs_to_jiffies(10))) {
bfqd->last_empty_occupied_ns = ktime_get_ns();
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 07/10] block, bfq: remove redundant check in bfq_put_cooperator
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
` (5 preceding siblings ...)
2022-12-22 19:16 ` [PATCH RESEND v2 06/10] block, bfq: remove redundant bfqd->rq_in_driver > 0 check in bfq_add_request Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:57 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 08/10] block, bfq: remove unnecessary goto tag in bfq_dispatch_rq_from_bfqq Kemeng Shi
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
We have already avoided a circular list in bfq_setup_merge (see comments
in bfq_setup_merge() for details), so bfq_queue will not appear in it's
new_bfqq list. Just remove this check.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 7c91d16dbf6f..89995815dbae 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5273,8 +5273,6 @@ void bfq_put_cooperator(struct bfq_queue *bfqq)
*/
__bfqq = bfqq->new_bfqq;
while (__bfqq) {
- if (__bfqq == bfqq)
- break;
next = __bfqq->new_bfqq;
bfq_put_queue(__bfqq);
__bfqq = next;
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 08/10] block, bfq: remove unnecessary goto tag in bfq_dispatch_rq_from_bfqq
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
` (6 preceding siblings ...)
2022-12-22 19:16 ` [PATCH RESEND v2 07/10] block, bfq: remove redundant check in bfq_put_cooperator Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 15:59 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 09/10] block, bfq: remove unused bfq_wr_max_time in struct bfq_data Kemeng Shi
2022-12-22 19:16 ` [PATCH RESEND v2 10/10] block, bfq: remove check of bfq_wr_max_softrt_rate which is always greater than 0 Kemeng Shi
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
We jump to tag only for returning current rq. Return directly to
remove this tag.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 89995815dbae..195cdc6be087 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4965,7 +4965,7 @@ static struct request *bfq_dispatch_rq_from_bfqq(struct bfq_data *bfqd,
bfq_dispatch_remove(bfqd->queue, rq);
if (bfqq != bfqd->in_service_queue)
- goto return_rq;
+ return rq;
/*
* If weight raising has to terminate for bfqq, then next
@@ -4985,12 +4985,9 @@ static struct request *bfq_dispatch_rq_from_bfqq(struct bfq_data *bfqd,
* belongs to CLASS_IDLE and other queues are waiting for
* service.
*/
- if (!(bfq_tot_busy_queues(bfqd) > 1 && bfq_class_idle(bfqq)))
- goto return_rq;
+ if ((bfq_tot_busy_queues(bfqd) > 1 && bfq_class_idle(bfqq)))
+ bfq_bfqq_expire(bfqd, bfqq, false, BFQQE_BUDGET_EXHAUSTED);
- bfq_bfqq_expire(bfqd, bfqq, false, BFQQE_BUDGET_EXHAUSTED);
-
-return_rq:
return rq;
}
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 09/10] block, bfq: remove unused bfq_wr_max_time in struct bfq_data
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
` (7 preceding siblings ...)
2022-12-22 19:16 ` [PATCH RESEND v2 08/10] block, bfq: remove unnecessary goto tag in bfq_dispatch_rq_from_bfqq Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 16:00 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 10/10] block, bfq: remove check of bfq_wr_max_softrt_rate which is always greater than 0 Kemeng Shi
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
bfqd->bfq_wr_max_time is set to 0 in bfq_init_queue and is never changed.
It is only used in bfq_wr_duration when bfq_wr_max_time > 0 which never
meets, so bfqd->bfq_wr_max_time is not used actually. Just remove it.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 4 ----
block/bfq-iosched.h | 2 --
2 files changed, 6 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 195cdc6be087..91bc68fba72d 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -1068,9 +1068,6 @@ static unsigned int bfq_wr_duration(struct bfq_data *bfqd)
{
u64 dur;
- if (bfqd->bfq_wr_max_time > 0)
- return bfqd->bfq_wr_max_time;
-
dur = bfqd->rate_dur_prod;
do_div(dur, bfqd->peak_rate);
@@ -7079,7 +7076,6 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
*/
bfqd->bfq_wr_coeff = 30;
bfqd->bfq_wr_rt_max_time = msecs_to_jiffies(300);
- bfqd->bfq_wr_max_time = 0;
bfqd->bfq_wr_min_idle_time = msecs_to_jiffies(2000);
bfqd->bfq_wr_min_inter_arr_async = msecs_to_jiffies(500);
bfqd->bfq_wr_max_softrt_rate = 7000; /*
diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
index 9fa89577322d..0b1a5438046a 100644
--- a/block/bfq-iosched.h
+++ b/block/bfq-iosched.h
@@ -719,8 +719,6 @@ struct bfq_data {
* is multiplied.
*/
unsigned int bfq_wr_coeff;
- /* maximum duration of a weight-raising period (jiffies) */
- unsigned int bfq_wr_max_time;
/* Maximum weight-raising duration for soft real-time processes */
unsigned int bfq_wr_rt_max_time;
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH RESEND v2 10/10] block, bfq: remove check of bfq_wr_max_softrt_rate which is always greater than 0
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
` (8 preceding siblings ...)
2022-12-22 19:16 ` [PATCH RESEND v2 09/10] block, bfq: remove unused bfq_wr_max_time in struct bfq_data Kemeng Shi
@ 2022-12-22 19:16 ` Kemeng Shi
2023-01-02 16:05 ` Jan Kara
9 siblings, 1 reply; 21+ messages in thread
From: Kemeng Shi @ 2022-12-22 19:16 UTC (permalink / raw)
To: paolo.valente, axboe, linux-block, linux-kernel; +Cc: jack, hch, damien.lemoal
bfqd->bfq_wr_max_softrt_rate is assigned with 7000 in bfq_init_queue and
never changed. So we can remove bfqd->bfq_wr_max_softrt_rate > 0 check
which is always true.
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
block/bfq-iosched.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 91bc68fba72d..00cdd42ac02a 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -1788,8 +1788,7 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
* 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) &&
+ soft_rt = !BFQQ_TOTALLY_SEEKY(bfqq) &&
!in_burst &&
time_is_before_jiffies(bfqq->soft_rt_next_start) &&
bfqq->dispatched == 0 &&
@@ -4284,8 +4283,7 @@ void bfq_bfqq_expire(struct bfq_data *bfqd,
if (bfqd->low_latency && bfqq->wr_coeff == 1)
bfqq->last_wr_start_finish = jiffies;
- if (bfqd->low_latency && bfqd->bfq_wr_max_softrt_rate > 0 &&
- RB_EMPTY_ROOT(&bfqq->sort_list)) {
+ if (bfqd->low_latency && RB_EMPTY_ROOT(&bfqq->sort_list)) {
/*
* If we get here, and there are no outstanding
* requests, then the request pattern is isochronous
--
2.30.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 01/10] block, bfq: correctly raise inject limit in bfq_choose_bfqq_for_injection
2022-12-22 19:16 ` [PATCH RESEND v2 01/10] block, bfq: correctly raise inject limit in bfq_choose_bfqq_for_injection Kemeng Shi
@ 2023-01-02 15:13 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:13 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:32, Kemeng Shi wrote:
> Function bfq_choose_bfqq_for_injection may temporarily raise inject limit
> to one request if current inject_limit is 0 before search of the source
> queue for injection. However the search below will reset inject limit to
> bfqd->in_service_queue which is zero for raised inject limit. Then the
> temporarily raised inject limit never works as expected.
> Assigment limit to bfqd->in_service_queue in search is needed as limit
> maybe overwriten to min_t(unsigned int, 1, limit) for condition that
> a large in-flight request is on non-rotational devices in found queue.
> So we need to reset limit to bfqd->in_service_queue for normal case.
>
> Actually, we have already make sure bfqd->rq_in_driver is < limit before
> search, then
> -Limit is >= 1 as bfqd->rq_in_driver is >= 0. Then min_t(unsigned int,
> 1, limit) is always 1. So we can simply check bfqd->rq_in_driver with
> 1 instead of result of min_t(unsigned int, 1, limit) for larget request in
> non-rotational device case to avoid overwritting limit and the bug is gone.
> -For normal case, we have already check bfqd->rq_in_driver is < limit,
> so we can return found bfqq unconditionally to remove unncessary check.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
The fix looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-iosched.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index a72304c728fc..1220c41fc767 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -4641,12 +4641,10 @@ bfq_choose_bfqq_for_injection(struct bfq_data *bfqd)
> */
> if (blk_queue_nonrot(bfqd->queue) &&
> blk_rq_sectors(bfqq->next_rq) >=
> - BFQQ_SECT_THR_NONROT)
> - limit = min_t(unsigned int, 1, limit);
> - else
> - limit = in_serv_bfqq->inject_limit;
> -
> - if (bfqd->rq_in_driver < limit) {
> + BFQQ_SECT_THR_NONROT &&
> + bfqd->rq_in_driver >= 1)
> + continue;
> + else {
> bfqd->rqs_injected = true;
> return bfqq;
> }
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 02/10] block, bfq: remove unsed parameter reason in bfq_bfqq_is_slow
2022-12-22 19:16 ` [PATCH RESEND v2 02/10] block, bfq: remove unsed parameter reason in bfq_bfqq_is_slow Kemeng Shi
@ 2023-01-02 15:14 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:14 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:33, Kemeng Shi wrote:
> Parameter reason is never used, just remove it.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Sure. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-iosched.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 1220c41fc767..5a6d9e8c329d 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -4066,8 +4066,7 @@ static void __bfq_bfqq_recalc_budget(struct bfq_data *bfqd,
> * function to evaluate the I/O speed of a process.
> */
> static bool bfq_bfqq_is_slow(struct bfq_data *bfqd, struct bfq_queue *bfqq,
> - bool compensate, enum bfqq_expiration reason,
> - unsigned long *delta_ms)
> + bool compensate, unsigned long *delta_ms)
> {
> ktime_t delta_ktime;
> u32 delta_usecs;
> @@ -4263,7 +4262,7 @@ void bfq_bfqq_expire(struct bfq_data *bfqd,
> /*
> * Check whether the process is slow (see bfq_bfqq_is_slow).
> */
> - slow = bfq_bfqq_is_slow(bfqd, bfqq, compensate, reason, &delta);
> + slow = bfq_bfqq_is_slow(bfqd, bfqq, compensate, &delta);
>
> /*
> * As above explained, charge slow (typically seeky) and
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 03/10] block, bfq: initialize bfqq->decrease_time_jif correctly
2022-12-22 19:16 ` [PATCH RESEND v2 03/10] block, bfq: initialize bfqq->decrease_time_jif correctly Kemeng Shi
@ 2023-01-02 15:30 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:30 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:34, Kemeng Shi wrote:
> Inject limit is updated or reset when time_is_before_eq_jiffies(
> decrease_time_jif + several msecs) or think-time state changes.
> decrease_time_jif is initialized to 0 and will be set to current jiffies
> when inject limit is updated or reset. If the jiffies is slightly greater
> than LONG_MAX, time_is_after_eq_jiffies(0) will keep for a long time, so as
> time_is_after_eq_jiffies(decrease_time_jif + several msecs). If the
> think-time state never chages, then the injection will not work as expected
> for long time.
>
> To be more specific:
> Function bfq_update_inject_limit maybe triggered when jiffies pasts
> decrease_time_jif + msecs_to_jiffies(10) in bfq_add_request by setting
> bfqd->wait_dispatch to true.
> Function bfq_reset_inject_limit are called in two conditions:
> 1. jiffies pasts bfqq->decrease_time_jif + msecs_to_jiffies(1000) in
> function bfq_add_request.
> 2. jiffies pasts bfqq->decrease_time_jif + msecs_to_jiffies(100) or
> bfq think-time state change from short to long.
>
> Fix this by initializing bfqq->decrease_time_jif to current jiffies
> to trigger service injection soon when service injection conditions
> are met.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
I agree that initializing jiffies timer to 0 is just a bad practice and
current time is much saner choice. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-iosched.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 5a6d9e8c329d..9441fc98961a 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -5491,6 +5491,8 @@ static void bfq_init_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq,
>
> /* first request is almost certainly seeky */
> bfqq->seek_history = 1;
> +
> + bfqq->decrease_time_jif = jiffies;
> }
>
> static struct bfq_queue **bfq_async_queue_prio(struct bfq_data *bfqd,
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 04/10] block, bfq: use helper macro RQ_BFQQ to get bfqq of request
2022-12-22 19:16 ` [PATCH RESEND v2 04/10] block, bfq: use helper macro RQ_BFQQ to get bfqq of request Kemeng Shi
@ 2023-01-02 15:31 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:31 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:35, Kemeng Shi wrote:
> Use helper macro RQ_BFQQ to get bfqq of request.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Yeah, why not. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-iosched.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 9441fc98961a..c3c4c83ee95f 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -6683,14 +6683,14 @@ static struct bfq_queue *bfq_init_rq(struct request *rq)
> return NULL;
>
> /*
> - * Assuming that elv.priv[1] is set only if everything is set
> + * Assuming that RQ_BFQQ(rq) is set only if everything is set
> * for this rq. This holds true, because this function is
> * invoked only for insertion or merging, and, after such
> * events, a request cannot be manipulated any longer before
> * being removed from bfq.
> */
> - if (rq->elv.priv[1])
> - return rq->elv.priv[1];
> + if (RQ_BFQQ(rq))
> + return RQ_BFQQ(rq);
>
> bic = icq_to_bic(rq->elv.icq);
>
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 05/10] block, bfq: remove unnecessary dereference to get async_bfqq
2022-12-22 19:16 ` [PATCH RESEND v2 05/10] block, bfq: remove unnecessary dereference to get async_bfqq Kemeng Shi
@ 2023-01-02 15:34 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:34 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:36, Kemeng Shi wrote:
> The async_bfqq is assigned with bfqq->bic->bfqq[0], use it directly.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Yeah, that is a more logical choice. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-iosched.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index c3c4c83ee95f..ebcafe3c4c3b 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -4835,7 +4835,7 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
> icq_to_bic(async_bfqq->next_rq->elv.icq) == bfqq->bic &&
> bfq_serv_to_charge(async_bfqq->next_rq, async_bfqq) <=
> bfq_bfqq_budget_left(async_bfqq))
> - bfqq = bfqq->bic->bfqq[0];
> + bfqq = async_bfqq;
> else if (bfqq->waker_bfqq &&
> bfq_bfqq_busy(bfqq->waker_bfqq) &&
> bfqq->waker_bfqq->next_rq &&
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 06/10] block, bfq: remove redundant bfqd->rq_in_driver > 0 check in bfq_add_request
2022-12-22 19:16 ` [PATCH RESEND v2 06/10] block, bfq: remove redundant bfqd->rq_in_driver > 0 check in bfq_add_request Kemeng Shi
@ 2023-01-02 15:54 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:54 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:37, Kemeng Shi wrote:
> The bfqd->rq_in_driver > 0 check is along with previous
> "bfqd->rq_in_driver == 0 ||" check, so no need to re-check
> bfqd->rq_in_driver > 0.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
I have nothing against this but OTOH in this complex condition the original
version looks more readable than the new one to me.
Honza
> ---
> block/bfq-iosched.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index ebcafe3c4c3b..7c91d16dbf6f 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -2204,8 +2204,7 @@ static void bfq_add_request(struct request *rq)
> */
> if (bfqq == bfqd->in_service_queue &&
> (bfqd->rq_in_driver == 0 ||
> - (bfqq->last_serv_time_ns > 0 &&
> - bfqd->rqs_injected && bfqd->rq_in_driver > 0)) &&
> + (bfqq->last_serv_time_ns > 0 && bfqd->rqs_injected)) &&
> time_is_before_eq_jiffies(bfqq->decrease_time_jif +
> msecs_to_jiffies(10))) {
> bfqd->last_empty_occupied_ns = ktime_get_ns();
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 07/10] block, bfq: remove redundant check in bfq_put_cooperator
2022-12-22 19:16 ` [PATCH RESEND v2 07/10] block, bfq: remove redundant check in bfq_put_cooperator Kemeng Shi
@ 2023-01-02 15:57 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:57 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:38, Kemeng Shi wrote:
> We have already avoided a circular list in bfq_setup_merge (see comments
> in bfq_setup_merge() for details), so bfq_queue will not appear in it's
> new_bfqq list. Just remove this check.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-iosched.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 7c91d16dbf6f..89995815dbae 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -5273,8 +5273,6 @@ void bfq_put_cooperator(struct bfq_queue *bfqq)
> */
> __bfqq = bfqq->new_bfqq;
> while (__bfqq) {
> - if (__bfqq == bfqq)
> - break;
> next = __bfqq->new_bfqq;
> bfq_put_queue(__bfqq);
> __bfqq = next;
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 08/10] block, bfq: remove unnecessary goto tag in bfq_dispatch_rq_from_bfqq
2022-12-22 19:16 ` [PATCH RESEND v2 08/10] block, bfq: remove unnecessary goto tag in bfq_dispatch_rq_from_bfqq Kemeng Shi
@ 2023-01-02 15:59 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 15:59 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:39, Kemeng Shi wrote:
> We jump to tag only for returning current rq. Return directly to
> remove this tag.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
One nit below.
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 89995815dbae..195cdc6be087 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -4965,7 +4965,7 @@ static struct request *bfq_dispatch_rq_from_bfqq(struct bfq_data *bfqd,
> bfq_dispatch_remove(bfqd->queue, rq);
>
> if (bfqq != bfqd->in_service_queue)
> - goto return_rq;
> + return rq;
>
> /*
> * If weight raising has to terminate for bfqq, then next
> @@ -4985,12 +4985,9 @@ static struct request *bfq_dispatch_rq_from_bfqq(struct bfq_data *bfqd,
> * belongs to CLASS_IDLE and other queues are waiting for
> * service.
> */
> - if (!(bfq_tot_busy_queues(bfqd) > 1 && bfq_class_idle(bfqq)))
> - goto return_rq;
> + if ((bfq_tot_busy_queues(bfqd) > 1 && bfq_class_idle(bfqq)))
^ unnecessary brace here
Honza
> + bfq_bfqq_expire(bfqd, bfqq, false, BFQQE_BUDGET_EXHAUSTED);
>
> - bfq_bfqq_expire(bfqd, bfqq, false, BFQQE_BUDGET_EXHAUSTED);
> -
> -return_rq:
> return rq;
> }
>
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 09/10] block, bfq: remove unused bfq_wr_max_time in struct bfq_data
2022-12-22 19:16 ` [PATCH RESEND v2 09/10] block, bfq: remove unused bfq_wr_max_time in struct bfq_data Kemeng Shi
@ 2023-01-02 16:00 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 16:00 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:40, Kemeng Shi wrote:
> bfqd->bfq_wr_max_time is set to 0 in bfq_init_queue and is never changed.
> It is only used in bfq_wr_duration when bfq_wr_max_time > 0 which never
> meets, so bfqd->bfq_wr_max_time is not used actually. Just remove it.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Nice catch. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> block/bfq-iosched.c | 4 ----
> block/bfq-iosched.h | 2 --
> 2 files changed, 6 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 195cdc6be087..91bc68fba72d 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -1068,9 +1068,6 @@ static unsigned int bfq_wr_duration(struct bfq_data *bfqd)
> {
> u64 dur;
>
> - if (bfqd->bfq_wr_max_time > 0)
> - return bfqd->bfq_wr_max_time;
> -
> dur = bfqd->rate_dur_prod;
> do_div(dur, bfqd->peak_rate);
>
> @@ -7079,7 +7076,6 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
> */
> bfqd->bfq_wr_coeff = 30;
> bfqd->bfq_wr_rt_max_time = msecs_to_jiffies(300);
> - bfqd->bfq_wr_max_time = 0;
> bfqd->bfq_wr_min_idle_time = msecs_to_jiffies(2000);
> bfqd->bfq_wr_min_inter_arr_async = msecs_to_jiffies(500);
> bfqd->bfq_wr_max_softrt_rate = 7000; /*
> diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
> index 9fa89577322d..0b1a5438046a 100644
> --- a/block/bfq-iosched.h
> +++ b/block/bfq-iosched.h
> @@ -719,8 +719,6 @@ struct bfq_data {
> * is multiplied.
> */
> unsigned int bfq_wr_coeff;
> - /* maximum duration of a weight-raising period (jiffies) */
> - unsigned int bfq_wr_max_time;
>
> /* Maximum weight-raising duration for soft real-time processes */
> unsigned int bfq_wr_rt_max_time;
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH RESEND v2 10/10] block, bfq: remove check of bfq_wr_max_softrt_rate which is always greater than 0
2022-12-22 19:16 ` [PATCH RESEND v2 10/10] block, bfq: remove check of bfq_wr_max_softrt_rate which is always greater than 0 Kemeng Shi
@ 2023-01-02 16:05 ` Jan Kara
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kara @ 2023-01-02 16:05 UTC (permalink / raw)
To: Kemeng Shi
Cc: paolo.valente, axboe, linux-block, linux-kernel, jack, hch,
damien.lemoal
On Fri 23-12-22 03:16:41, Kemeng Shi wrote:
> bfqd->bfq_wr_max_softrt_rate is assigned with 7000 in bfq_init_queue and
> never changed. So we can remove bfqd->bfq_wr_max_softrt_rate > 0 check
> which is always true.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
I would just leave these checks for documentation purposes and for possible
experiments (e.g. disabling this logic by setting bfq_wr_max_softrt_rate to
0). Alternatively, we could just define a constant for this and
then we can remove all the checks, that would be a clean solution as well.
Honza
> ---
> block/bfq-iosched.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 91bc68fba72d..00cdd42ac02a 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -1788,8 +1788,7 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd,
> * 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) &&
> + soft_rt = !BFQQ_TOTALLY_SEEKY(bfqq) &&
> !in_burst &&
> time_is_before_jiffies(bfqq->soft_rt_next_start) &&
> bfqq->dispatched == 0 &&
> @@ -4284,8 +4283,7 @@ void bfq_bfqq_expire(struct bfq_data *bfqd,
> if (bfqd->low_latency && bfqq->wr_coeff == 1)
> bfqq->last_wr_start_finish = jiffies;
>
> - if (bfqd->low_latency && bfqd->bfq_wr_max_softrt_rate > 0 &&
> - RB_EMPTY_ROOT(&bfqq->sort_list)) {
> + if (bfqd->low_latency && RB_EMPTY_ROOT(&bfqq->sort_list)) {
> /*
> * If we get here, and there are no outstanding
> * requests, then the request pattern is isochronous
> --
> 2.30.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-01-02 16:05 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-22 19:16 [PATCH RESEND v2 00/10] A few bugfix and cleancode patch for bfq Kemeng Shi
2022-12-22 19:16 ` [PATCH RESEND v2 01/10] block, bfq: correctly raise inject limit in bfq_choose_bfqq_for_injection Kemeng Shi
2023-01-02 15:13 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 02/10] block, bfq: remove unsed parameter reason in bfq_bfqq_is_slow Kemeng Shi
2023-01-02 15:14 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 03/10] block, bfq: initialize bfqq->decrease_time_jif correctly Kemeng Shi
2023-01-02 15:30 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 04/10] block, bfq: use helper macro RQ_BFQQ to get bfqq of request Kemeng Shi
2023-01-02 15:31 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 05/10] block, bfq: remove unnecessary dereference to get async_bfqq Kemeng Shi
2023-01-02 15:34 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 06/10] block, bfq: remove redundant bfqd->rq_in_driver > 0 check in bfq_add_request Kemeng Shi
2023-01-02 15:54 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 07/10] block, bfq: remove redundant check in bfq_put_cooperator Kemeng Shi
2023-01-02 15:57 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 08/10] block, bfq: remove unnecessary goto tag in bfq_dispatch_rq_from_bfqq Kemeng Shi
2023-01-02 15:59 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 09/10] block, bfq: remove unused bfq_wr_max_time in struct bfq_data Kemeng Shi
2023-01-02 16:00 ` Jan Kara
2022-12-22 19:16 ` [PATCH RESEND v2 10/10] block, bfq: remove check of bfq_wr_max_softrt_rate which is always greater than 0 Kemeng Shi
2023-01-02 16:05 ` Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).