在 2022/08/11 9:19, Yu Kuai 写道:
Hi, Paolo
在 2022/08/10 18:49, Paolo Valente 写道:
Il giorno 27 lug 2022, alle ore 14:11, Yu Kuai
<yukuai1@xxxxxxxxxxxxxxx> ha scritto:
Hi, Paolo
hi
Are you still interested in this patchset?
Yes. Sorry for replying very late again.
Probably the last fix that you suggest is enough, but I'm a little bit
concerned that it may be a little hasty. In fact, before this fix, we
exchanged several messages, and I didn't seem to be very good at
convincing you about the need to keep into account also in-service
I/O. So, my question is: are you sure that now you have a
I'm confused here, I'm pretty aware that in-service I/O(as said pending
requests is the patchset) should be counted, as you suggested in v7, are
you still thinking that the way in this patchset is problematic?
I'll try to explain again that how to track is bfqq has pending pending
requests, please let me know if you still think there are some problems:
patch 1 support to track if bfqq has pending requests, it's
done by setting the flag 'entity->in_groups_with_pending_reqs' when the
first request is inserted to bfqq, and it's cleared when the last
request is completed. specifically the flag is set in
bfq_add_bfqq_busy() when 'bfqq->dispatched' if false, and it's cleared
both in bfq_completed_request() and bfq_del_bfqq_busy() when
'bfqq->diapatched' is false.
Hi, Paolo
Can you please have a check if patch 1 is ok?
Thanks,
Kuai
Thanks,
Kuai
clear/complete understanding of this non-trivial matter?
Consequently, are we sure that this last fix is most certainly all we
need? Of course, I will check on my own, but if you reassure me on
this point, I will feel more confident.
Thanks,
Paolo
在 2022/07/20 19:38, Yu Kuai 写道:
Hi
在 2022/07/20 19:24, Paolo VALENTE 写道:
Il giorno 12 lug 2022, alle ore 15:30, Yu Kuai
<yukuai1@xxxxxxxxxxxxxxx <mailto:yukuai1@xxxxxxxxxxxxxxx>> ha
scritto:
Hi!
I'm copying my reply with new mail address, because Paolo seems
didn't receive my reply.
在 2022/06/23 23:32, Paolo Valente 写道:
Sorry for the delay.
Il giorno 10 giu 2022, alle ore 04:17, Yu Kuai
<yukuai3@xxxxxxxxxx <mailto:yukuai3@xxxxxxxxxx>> ha scritto:
Currently, bfq can't handle sync io concurrently as long as they
are not issued from root group. This is because
'bfqd->num_groups_with_pending_reqs > 0' is always true in
bfq_asymmetric_scenario().
The way that bfqg is counted into 'num_groups_with_pending_reqs':
Before this patch:
1) root group will never be counted.
2) Count if bfqg or it's child bfqgs have pending requests.
3) Don't count if bfqg and it's child bfqgs complete all the
requests.
After this patch:
1) root group is counted.
2) Count if bfqg have pending requests.
3) Don't count if bfqg complete all the requests.
With this change, the occasion that only one group is activated
can be
detected, and next patch will support concurrent sync io in the
occasion.
Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx
<mailto:yukuai3@xxxxxxxxxx>>
Reviewed-by: Jan Kara <jack@xxxxxxx <mailto:jack@xxxxxxx>>
---
block/bfq-iosched.c | 42 ------------------------------------------
block/bfq-iosched.h | 18 +++++++++---------
block/bfq-wf2q.c | 19 ++++---------------
3 files changed, 13 insertions(+), 66 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 0ec21018daba..03b04892440c 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -970,48 +970,6 @@ void __bfq_weights_tree_remove(struct
bfq_data *bfqd,
void bfq_weights_tree_remove(struct bfq_data *bfqd,
struct bfq_queue *bfqq)
{
-struct bfq_entity *entity = bfqq->entity.parent;
-
-for_each_entity(entity) {
-struct bfq_sched_data *sd = entity->my_sched_data;
-
-if (sd->next_in_service || sd->in_service_entity) {
-/*
-* entity is still active, because either
-* next_in_service or in_service_entity is not
-* NULL (see the comments on the definition of
-* next_in_service for details on why
-* in_service_entity must be checked too).
-*
-* As a consequence, its parent entities are
-* active as well, and thus this loop must
-* stop here.
-*/
-break;
-}
-
-/*
-* The decrement of num_groups_with_pending_reqs is
-* not performed immediately upon the deactivation of
-* entity, but it is delayed to when it also happens
-* that the first leaf descendant bfqq of entity gets
-* all its pending requests completed. The following
-* instructions perform this delayed decrement, if
-* needed. See the comments on
-* num_groups_with_pending_reqs for details.
-*/
-if (entity->in_groups_with_pending_reqs) {
-entity->in_groups_with_pending_reqs = false;
-bfqd->num_groups_with_pending_reqs--;
-}
-}
With this part removed, I'm missing how you handle the following
sequence of events:
1. a queue Q becomes non busy but still has dispatched requests, so
it must not be removed from the counter of queues with pending reqs
yet
2. the last request of Q is completed with Q being still idle (non
busy). At this point Q must be removed from the counter. It
seems to
me that this case is not handled any longer
Hi, Paolo
1) At first, patch 1 support to track if bfqq has pending
requests, it's
done by setting the flag 'entity->in_groups_with_pending_reqs'
when the
first request is inserted to bfqq, and it's cleared when the last
request is completed(based on weights_tree insertion and removal).
In patch 1 I don't see the flag cleared for the request-completion
event :(
The piece of code involved is this:
static void bfq_completed_request(struct bfq_queue *bfqq, struct
bfq_data *bfqd)
{
u64 now_ns;
u32 delta_us;
bfq_update_hw_tag(bfqd);
bfqd->rq_in_driver[bfqq->actuator_idx]--;
bfqd->tot_rq_in_driver--;
bfqq->dispatched--;
if (!bfqq->dispatched && !bfq_bfqq_busy(bfqq)) {
/*
* Set budget_timeout (which we overload to store the
* time at which the queue remains with no backlog and
* no outstanding request; used by the weight-raising
* mechanism).
*/
bfqq->budget_timeout = jiffies;
bfq_weights_tree_remove(bfqd, bfqq);
}
...
Am I missing something?
I add a new api bfq_del_bfqq_in_groups_with_pending_reqs() in patch 1
to clear the flag, and it's called both from bfq_del_bfqq_busy() and
bfq_completed_request(). I think you may miss the later:
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 0d46cb728bbf..0ec21018daba 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -6263,6 +6263,7 @@ static void bfq_completed_request(struct
bfq_queue *bfqq, struct bfq_data *bfqd)
*/
bfqq->budget_timeout = jiffies;
+ bfq_del_bfqq_in_groups_with_pending_reqs(bfqq);
bfq_weights_tree_remove(bfqd, bfqq);
}
Thanks,
Kuai
Thanks,
Paolo
.
.
