2022-03-05 12:17:07

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

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().

This patchset tries to support concurrent sync io if all the sync ios
are issued from the same cgroup:

1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;

2) Don't idle if 'num_groups_with_pending_reqs' is 1, patch 6;

3) Don't count the group if the group doesn't have pending requests,
while it's child groups may have pending requests, patch 7;

This is because, for example:
if sync ios are issued from cgroup /root/c1/c2, root, c1 and c2
will all be counted into 'num_groups_with_pending_reqs',
which makes it impossible to handle sync ios concurrently.

4) Decrease 'num_groups_with_pending_reqs' when the last queue completes
all the requests, while child groups may still have pending
requests, patch 8-10;

This is because, for example:
t1 issue sync io on root group, t2 and t3 issue sync io on the same
child group. num_groups_with_pending_reqs is 2 now.
After t1 stopped, num_groups_with_pending_reqs is still 2. sync io from
t2 and t3 still can't be handled concurrently.

fio test script: startdelay is used to avoid queue merging
[global]
filename=/dev/nvme0n1
allow_mounted_write=0
ioengine=psync
direct=1
ioscheduler=bfq
offset_increment=10g
group_reporting
rw=randwrite
bs=4k

[test1]
numjobs=1

[test2]
startdelay=1
numjobs=1

[test3]
startdelay=2
numjobs=1

[test4]
startdelay=3
numjobs=1

[test5]
startdelay=4
numjobs=1

[test6]
startdelay=5
numjobs=1

[test7]
startdelay=6
numjobs=1

[test8]
startdelay=7
numjobs=1

test result:
running fio on root cgroup
v5.17-rc6: 550 Mib/s
v5.17-rc6-patched: 550 Mib/s

running fio on non-root cgroup
v5.17-rc6: 349 Mib/s
v5.17-rc6-patched: 550 Mib/s

Yu Kuai (11):
block, bfq: add new apis to iterate bfq entities
block, bfq: apply news apis where root group is not expected
block, bfq: cleanup for __bfq_activate_requeue_entity()
block, bfq: move the increasement of 'num_groups_with_pending_reqs' to
it's caller
block, bfq: count root group into 'num_groups_with_pending_reqs'
block, bfq: do not idle if only one cgroup is activated
block, bfq: only count parent bfqg when bfqq is activated
block, bfq: record how many queues have pending requests in bfq_group
block, bfq: move forward __bfq_weights_tree_remove()
block, bfq: decrease 'num_groups_with_pending_reqs' earlier
block, bfq: cleanup bfqq_group()

block/bfq-cgroup.c | 13 +++----
block/bfq-iosched.c | 87 +++++++++++++++++++++++----------------------
block/bfq-iosched.h | 41 +++++++++++++--------
block/bfq-wf2q.c | 56 +++++++++++++++--------------
4 files changed, 106 insertions(+), 91 deletions(-)

--
2.31.1


2022-03-05 12:32:10

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 09/11] block, bfq: move forward __bfq_weights_tree_remove()

Prepare to decrease 'num_groups_with_pending_reqs' earlier.

Signed-off-by: Yu Kuai <[email protected]>
---
block/bfq-iosched.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 2a48c40b4f02..f221e9cab4d0 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -979,6 +979,19 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
{
struct bfq_entity *entity = bfqq->entity.parent;

+ /*
+ * grab a ref to prevent bfqq to be freed in
+ * __bfq_weights_tree_remove
+ */
+ bfqq->ref++;
+
+ /*
+ * remove bfqq from weights tree first, so that how many queues have
+ * pending requests in parent bfqg is updated.
+ */
+ __bfq_weights_tree_remove(bfqd, bfqq,
+ &bfqd->queue_weights_tree);
+
for_each_entity(entity) {
struct bfq_sched_data *sd = entity->my_sched_data;

@@ -1013,14 +1026,7 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
}
}

- /*
- * Next function is invoked last, because it causes bfqq to be
- * freed if the following holds: bfqq is not in service and
- * has no dispatched request. DO NOT use bfqq after the next
- * function invocation.
- */
- __bfq_weights_tree_remove(bfqd, bfqq,
- &bfqd->queue_weights_tree);
+ bfq_put_queue(bfqq);
}

/*
--
2.31.1

2022-03-05 14:58:14

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 08/11] block, bfq: record how many queues have pending requests in bfq_group

Prepare to decrease 'num_groups_with_pending_reqs' earlier.

bfqq will be inserted to weights_tree when new io is inserted to it, and
bfqq will be removed from weights_tree when all the requests are completed.
Thus use weights_tree insertion and removal to track how many queues have
pending requests.

Signed-off-by: Yu Kuai <[email protected]>
---
block/bfq-cgroup.c | 1 +
block/bfq-iosched.c | 15 +++++++++++++++
block/bfq-iosched.h | 1 +
3 files changed, 17 insertions(+)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 6cd65b5e790d..58acaf14a91d 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -557,6 +557,7 @@ static void bfq_pd_init(struct blkg_policy_data *pd)
*/
bfqg->bfqd = bfqd;
bfqg->active_entities = 0;
+ bfqg->num_entities_with_pending_reqs = 0;
bfqg->rq_pos_tree = RB_ROOT;
}

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 07027dc9dc4c..2a48c40b4f02 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -928,6 +928,13 @@ void bfq_weights_tree_add(struct bfq_data *bfqd, struct bfq_queue *bfqq,
inc_counter:
bfqq->weight_counter->num_active++;
bfqq->ref++;
+
+#ifdef CONFIG_BFQ_GROUP_IOSCHED
+ if (!entity->in_groups_with_pending_reqs) {
+ entity->in_groups_with_pending_reqs = true;
+ bfqq_group(bfqq)->num_entities_with_pending_reqs++;
+ }
+#endif
}

/*
@@ -944,6 +951,14 @@ void __bfq_weights_tree_remove(struct bfq_data *bfqd,
return;

bfqq->weight_counter->num_active--;
+
+#ifdef CONFIG_BFQ_GROUP_IOSCHED
+ if (bfqq->entity.in_groups_with_pending_reqs) {
+ bfqq->entity.in_groups_with_pending_reqs = false;
+ bfqq_group(bfqq)->num_entities_with_pending_reqs--;
+ }
+#endif
+
if (bfqq->weight_counter->num_active > 0)
goto reset_entity_pointer;

diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
index 4530ab8b42ac..5d904851519c 100644
--- a/block/bfq-iosched.h
+++ b/block/bfq-iosched.h
@@ -940,6 +940,7 @@ struct bfq_group {
struct bfq_entity *my_entity;

int active_entities;
+ int num_entities_with_pending_reqs;

struct rb_root rq_pos_tree;

--
2.31.1

2022-03-05 23:17:59

by Yu Kuai

[permalink] [raw]
Subject: [PATCH -next 04/11] block, bfq: move the increasement of 'num_groups_with_pending_reqs' to it's caller

Root group is not in service tree, thus __bfq_activate_entity() is not
needed for root_group. This will simplify counting root group into
'num_groups_with_pending_reqs'.

Signed-off-by: Yu Kuai <[email protected]>
---
block/bfq-wf2q.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/block/bfq-wf2q.c b/block/bfq-wf2q.c
index e30da27f356d..17f1d2c5b8dc 100644
--- a/block/bfq-wf2q.c
+++ b/block/bfq-wf2q.c
@@ -218,6 +218,19 @@ static bool bfq_no_longer_next_in_service(struct bfq_entity *entity)
return false;
}

+static void bfq_update_groups_with_pending_reqs(struct bfq_entity *entity)
+{
+ if (!bfq_entity_to_bfqq(entity) && /* bfq_group */
+ !entity->in_groups_with_pending_reqs) {
+ struct bfq_group *bfqg =
+ container_of(entity, struct bfq_group, entity);
+ struct bfq_data *bfqd = bfqg->bfqd;
+
+ entity->in_groups_with_pending_reqs = true;
+ bfqd->num_groups_with_pending_reqs++;
+ }
+}
+
#else /* CONFIG_BFQ_GROUP_IOSCHED */

static bool bfq_update_parent_budget(struct bfq_entity *next_in_service)
@@ -230,6 +243,10 @@ static bool bfq_no_longer_next_in_service(struct bfq_entity *entity)
return true;
}

+static void bfq_update_groups_with_pending_reqs(struct bfq_entity *entity)
+{
+}
+
#endif /* CONFIG_BFQ_GROUP_IOSCHED */

/*
@@ -984,19 +1001,6 @@ static void __bfq_activate_entity(struct bfq_entity *entity,
entity->on_st_or_in_serv = true;
}

-#ifdef CONFIG_BFQ_GROUP_IOSCHED
- if (!bfq_entity_to_bfqq(entity)) { /* bfq_group */
- struct bfq_group *bfqg =
- container_of(entity, struct bfq_group, entity);
- struct bfq_data *bfqd = bfqg->bfqd;
-
- if (!entity->in_groups_with_pending_reqs) {
- entity->in_groups_with_pending_reqs = true;
- bfqd->num_groups_with_pending_reqs++;
- }
- }
-#endif
-
bfq_update_fin_time_enqueue(entity, st, backshifted);
}

@@ -1120,6 +1124,7 @@ static void bfq_activate_requeue_entity(struct bfq_entity *entity,
bool requeue, bool expiration)
{
for_each_entity(entity) {
+ bfq_update_groups_with_pending_reqs(entity);
__bfq_activate_requeue_entity(entity, non_blocking_wait_rq);

if (!bfq_update_next_in_service(entity->sched_data, entity,
--
2.31.1

2022-03-11 23:25:08

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

friendly ping ...

?? 2022/03/05 17:11, Yu Kuai д??:
> 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().
>
> This patchset tries to support concurrent sync io if all the sync ios
> are issued from the same cgroup:
>
> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>
> 2) Don't idle if 'num_groups_with_pending_reqs' is 1, patch 6;
>
> 3) Don't count the group if the group doesn't have pending requests,
> while it's child groups may have pending requests, patch 7;
>
> This is because, for example:
> if sync ios are issued from cgroup /root/c1/c2, root, c1 and c2
> will all be counted into 'num_groups_with_pending_reqs',
> which makes it impossible to handle sync ios concurrently.
>
> 4) Decrease 'num_groups_with_pending_reqs' when the last queue completes
> all the requests, while child groups may still have pending
> requests, patch 8-10;
>
> This is because, for example:
> t1 issue sync io on root group, t2 and t3 issue sync io on the same
> child group. num_groups_with_pending_reqs is 2 now.
> After t1 stopped, num_groups_with_pending_reqs is still 2. sync io from
> t2 and t3 still can't be handled concurrently.
>
> fio test script: startdelay is used to avoid queue merging
> [global]
> filename=/dev/nvme0n1
> allow_mounted_write=0
> ioengine=psync
> direct=1
> ioscheduler=bfq
> offset_increment=10g
> group_reporting
> rw=randwrite
> bs=4k
>
> [test1]
> numjobs=1
>
> [test2]
> startdelay=1
> numjobs=1
>
> [test3]
> startdelay=2
> numjobs=1
>
> [test4]
> startdelay=3
> numjobs=1
>
> [test5]
> startdelay=4
> numjobs=1
>
> [test6]
> startdelay=5
> numjobs=1
>
> [test7]
> startdelay=6
> numjobs=1
>
> [test8]
> startdelay=7
> numjobs=1
>
> test result:
> running fio on root cgroup
> v5.17-rc6: 550 Mib/s
> v5.17-rc6-patched: 550 Mib/s
>
> running fio on non-root cgroup
> v5.17-rc6: 349 Mib/s
> v5.17-rc6-patched: 550 Mib/s
>
> Yu Kuai (11):
> block, bfq: add new apis to iterate bfq entities
> block, bfq: apply news apis where root group is not expected
> block, bfq: cleanup for __bfq_activate_requeue_entity()
> block, bfq: move the increasement of 'num_groups_with_pending_reqs' to
> it's caller
> block, bfq: count root group into 'num_groups_with_pending_reqs'
> block, bfq: do not idle if only one cgroup is activated
> block, bfq: only count parent bfqg when bfqq is activated
> block, bfq: record how many queues have pending requests in bfq_group
> block, bfq: move forward __bfq_weights_tree_remove()
> block, bfq: decrease 'num_groups_with_pending_reqs' earlier
> block, bfq: cleanup bfqq_group()
>
> block/bfq-cgroup.c | 13 +++----
> block/bfq-iosched.c | 87 +++++++++++++++++++++++----------------------
> block/bfq-iosched.h | 41 +++++++++++++--------
> block/bfq-wf2q.c | 56 +++++++++++++++--------------
> 4 files changed, 106 insertions(+), 91 deletions(-)
>

2022-03-17 05:31:50

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

friendly ping ...

在 2022/03/11 14:31, yukuai (C) 写道:
> friendly ping ...
>
> 在 2022/03/05 17:11, Yu Kuai 写道:
>> 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().
>>
>> This patchset tries to support concurrent sync io if all the sync ios
>> are issued from the same cgroup:
>>
>> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>>
>> 2) Don't idle if 'num_groups_with_pending_reqs' is 1, patch 6;
>>
>> 3) Don't count the group if the group doesn't have pending requests,
>> while it's child groups may have pending requests, patch 7;
>>
>> This is because, for example:
>> if sync ios are issued from cgroup /root/c1/c2, root, c1 and c2
>> will all be counted into 'num_groups_with_pending_reqs',
>> which makes it impossible to handle sync ios concurrently.
>>
>> 4) Decrease 'num_groups_with_pending_reqs' when the last queue completes
>> all the requests, while child groups may still have pending
>> requests, patch 8-10;
>>
>> This is because, for example:
>> t1 issue sync io on root group, t2 and t3 issue sync io on the same
>> child group. num_groups_with_pending_reqs is 2 now.
>> After t1 stopped, num_groups_with_pending_reqs is still 2. sync io from
>> t2 and t3 still can't be handled concurrently.
>>
>> fio test script: startdelay is used to avoid queue merging
>> [global]
>> filename=/dev/nvme0n1
>> allow_mounted_write=0
>> ioengine=psync
>> direct=1
>> ioscheduler=bfq
>> offset_increment=10g
>> group_reporting
>> rw=randwrite
>> bs=4k
>>
>> [test1]
>> numjobs=1
>>
>> [test2]
>> startdelay=1
>> numjobs=1
>>
>> [test3]
>> startdelay=2
>> numjobs=1
>>
>> [test4]
>> startdelay=3
>> numjobs=1
>>
>> [test5]
>> startdelay=4
>> numjobs=1
>>
>> [test6]
>> startdelay=5
>> numjobs=1
>>
>> [test7]
>> startdelay=6
>> numjobs=1
>>
>> [test8]
>> startdelay=7
>> numjobs=1
>>
>> test result:
>> running fio on root cgroup
>> v5.17-rc6:       550 Mib/s
>> v5.17-rc6-patched: 550 Mib/s
>>
>> running fio on non-root cgroup
>> v5.17-rc6:       349 Mib/s
>> v5.17-rc6-patched: 550 Mib/s
>>
>> Yu Kuai (11):
>>    block, bfq: add new apis to iterate bfq entities
>>    block, bfq: apply news apis where root group is not expected
>>    block, bfq: cleanup for __bfq_activate_requeue_entity()
>>    block, bfq: move the increasement of 'num_groups_with_pending_reqs' to
>>      it's caller
>>    block, bfq: count root group into 'num_groups_with_pending_reqs'
>>    block, bfq: do not idle if only one cgroup is activated
>>    block, bfq: only count parent bfqg when bfqq is activated
>>    block, bfq: record how many queues have pending requests in bfq_group
>>    block, bfq: move forward __bfq_weights_tree_remove()
>>    block, bfq: decrease 'num_groups_with_pending_reqs' earlier
>>    block, bfq: cleanup bfqq_group()
>>
>>   block/bfq-cgroup.c  | 13 +++----
>>   block/bfq-iosched.c | 87 +++++++++++++++++++++++----------------------
>>   block/bfq-iosched.h | 41 +++++++++++++--------
>>   block/bfq-wf2q.c    | 56 +++++++++++++++--------------
>>   4 files changed, 106 insertions(+), 91 deletions(-)
>>

2022-03-20 11:08:58

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

在 2022/03/18 20:38, Paolo Valente 写道:
> Hi,
> could you please add pointers to the thread(s) where we have already revised this series (if we have). I don't see any reference to that in this cover letter.

Hi,

Ok, sorry for that, following is the previours threads.

This is a new patchset after RFC
- Fix some term in commit messages and comments
- Add some cleanup patches

New RFC: use a new solution, and it has little relevance to
previous versions.
https://lore.kernel.org/lkml/[email protected]/T/
- as suggested by Paolo, count root group into
'num_groups_with_pending_reqs' instead of handling root group
separately.
- Change the patchset title
- New changes about when to modify 'num_groups_with_pending_reqs'

Orignal v4:
https://lore.kernel.org/lkml/[email protected]/t/
- fix a compile warning when CONFIG_BLK_CGROUP is not enabled.

Orignal v3:
https://www.spinics.net/lists/linux-block/msg74836.html
- Instead of tracking each queue in root group, tracking root group
directly just like non-root group does.
- remove patch 3,4 from these series.

Orignal v2:
https://lore.kernel.org/lkml/[email protected]/
- as suggested by Paolo, add support to track if root_group have any
pending requests, and use that to handle the situation when only one
group is activated while root group doesn't have any pending requests.
- modify commit message in patch 2

2022-03-20 16:28:36

by Paolo Valente

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

Hi,
could you please add pointers to the thread(s) where we have already revised this series (if we have). I don't see any reference to that in this cover letter.

Paolo

> Il giorno 17 mar 2022, alle ore 02:49, yukuai (C) <[email protected]> ha scritto:
>
> friendly ping ...
>
> 在 2022/03/11 14:31, yukuai (C) 写道:
>> friendly ping ...
>> 在 2022/03/05 17:11, Yu Kuai 写道:
>>> 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().
>>>
>>> This patchset tries to support concurrent sync io if all the sync ios
>>> are issued from the same cgroup:
>>>
>>> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>>>
>>> 2) Don't idle if 'num_groups_with_pending_reqs' is 1, patch 6;
>>>
>>> 3) Don't count the group if the group doesn't have pending requests,
>>> while it's child groups may have pending requests, patch 7;
>>>
>>> This is because, for example:
>>> if sync ios are issued from cgroup /root/c1/c2, root, c1 and c2
>>> will all be counted into 'num_groups_with_pending_reqs',
>>> which makes it impossible to handle sync ios concurrently.
>>>
>>> 4) Decrease 'num_groups_with_pending_reqs' when the last queue completes
>>> all the requests, while child groups may still have pending
>>> requests, patch 8-10;
>>>
>>> This is because, for example:
>>> t1 issue sync io on root group, t2 and t3 issue sync io on the same
>>> child group. num_groups_with_pending_reqs is 2 now.
>>> After t1 stopped, num_groups_with_pending_reqs is still 2. sync io from
>>> t2 and t3 still can't be handled concurrently.
>>>
>>> fio test script: startdelay is used to avoid queue merging
>>> [global]
>>> filename=/dev/nvme0n1
>>> allow_mounted_write=0
>>> ioengine=psync
>>> direct=1
>>> ioscheduler=bfq
>>> offset_increment=10g
>>> group_reporting
>>> rw=randwrite
>>> bs=4k
>>>
>>> [test1]
>>> numjobs=1
>>>
>>> [test2]
>>> startdelay=1
>>> numjobs=1
>>>
>>> [test3]
>>> startdelay=2
>>> numjobs=1
>>>
>>> [test4]
>>> startdelay=3
>>> numjobs=1
>>>
>>> [test5]
>>> startdelay=4
>>> numjobs=1
>>>
>>> [test6]
>>> startdelay=5
>>> numjobs=1
>>>
>>> [test7]
>>> startdelay=6
>>> numjobs=1
>>>
>>> [test8]
>>> startdelay=7
>>> numjobs=1
>>>
>>> test result:
>>> running fio on root cgroup
>>> v5.17-rc6: 550 Mib/s
>>> v5.17-rc6-patched: 550 Mib/s
>>>
>>> running fio on non-root cgroup
>>> v5.17-rc6: 349 Mib/s
>>> v5.17-rc6-patched: 550 Mib/s
>>>
>>> Yu Kuai (11):
>>> block, bfq: add new apis to iterate bfq entities
>>> block, bfq: apply news apis where root group is not expected
>>> block, bfq: cleanup for __bfq_activate_requeue_entity()
>>> block, bfq: move the increasement of 'num_groups_with_pending_reqs' to
>>> it's caller
>>> block, bfq: count root group into 'num_groups_with_pending_reqs'
>>> block, bfq: do not idle if only one cgroup is activated
>>> block, bfq: only count parent bfqg when bfqq is activated
>>> block, bfq: record how many queues have pending requests in bfq_group
>>> block, bfq: move forward __bfq_weights_tree_remove()
>>> block, bfq: decrease 'num_groups_with_pending_reqs' earlier
>>> block, bfq: cleanup bfqq_group()
>>>
>>> block/bfq-cgroup.c | 13 +++----
>>> block/bfq-iosched.c | 87 +++++++++++++++++++++++----------------------
>>> block/bfq-iosched.h | 41 +++++++++++++--------
>>> block/bfq-wf2q.c | 56 +++++++++++++++--------------
>>> 4 files changed, 106 insertions(+), 91 deletions(-)
>>>

2022-03-25 19:09:54

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

friendly ping ...

在 2022/03/17 9:49, yukuai (C) 写道:
> friendly ping ...
>
> 在 2022/03/11 14:31, yukuai (C) 写道:
>> friendly ping ...
>>
>> 在 2022/03/05 17:11, Yu Kuai 写道:
>>> 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().
>>>
>>> This patchset tries to support concurrent sync io if all the sync ios
>>> are issued from the same cgroup:
>>>
>>> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>>>
>>> 2) Don't idle if 'num_groups_with_pending_reqs' is 1, patch 6;
>>>
>>> 3) Don't count the group if the group doesn't have pending requests,
>>> while it's child groups may have pending requests, patch 7;
>>>
>>> This is because, for example:
>>> if sync ios are issued from cgroup /root/c1/c2, root, c1 and c2
>>> will all be counted into 'num_groups_with_pending_reqs',
>>> which makes it impossible to handle sync ios concurrently.
>>>
>>> 4) Decrease 'num_groups_with_pending_reqs' when the last queue completes
>>> all the requests, while child groups may still have pending
>>> requests, patch 8-10;
>>>
>>> This is because, for example:
>>> t1 issue sync io on root group, t2 and t3 issue sync io on the same
>>> child group. num_groups_with_pending_reqs is 2 now.
>>> After t1 stopped, num_groups_with_pending_reqs is still 2. sync io from
>>> t2 and t3 still can't be handled concurrently.
>>>
>>> fio test script: startdelay is used to avoid queue merging
>>> [global]
>>> filename=/dev/nvme0n1
>>> allow_mounted_write=0
>>> ioengine=psync
>>> direct=1
>>> ioscheduler=bfq
>>> offset_increment=10g
>>> group_reporting
>>> rw=randwrite
>>> bs=4k
>>>
>>> [test1]
>>> numjobs=1
>>>
>>> [test2]
>>> startdelay=1
>>> numjobs=1
>>>
>>> [test3]
>>> startdelay=2
>>> numjobs=1
>>>
>>> [test4]
>>> startdelay=3
>>> numjobs=1
>>>
>>> [test5]
>>> startdelay=4
>>> numjobs=1
>>>
>>> [test6]
>>> startdelay=5
>>> numjobs=1
>>>
>>> [test7]
>>> startdelay=6
>>> numjobs=1
>>>
>>> [test8]
>>> startdelay=7
>>> numjobs=1
>>>
>>> test result:
>>> running fio on root cgroup
>>> v5.17-rc6:       550 Mib/s
>>> v5.17-rc6-patched: 550 Mib/s
>>>
>>> running fio on non-root cgroup
>>> v5.17-rc6:       349 Mib/s
>>> v5.17-rc6-patched: 550 Mib/s
>>>
>>> Yu Kuai (11):
>>>    block, bfq: add new apis to iterate bfq entities
>>>    block, bfq: apply news apis where root group is not expected
>>>    block, bfq: cleanup for __bfq_activate_requeue_entity()
>>>    block, bfq: move the increasement of
>>> 'num_groups_with_pending_reqs' to
>>>      it's caller
>>>    block, bfq: count root group into 'num_groups_with_pending_reqs'
>>>    block, bfq: do not idle if only one cgroup is activated
>>>    block, bfq: only count parent bfqg when bfqq is activated
>>>    block, bfq: record how many queues have pending requests in bfq_group
>>>    block, bfq: move forward __bfq_weights_tree_remove()
>>>    block, bfq: decrease 'num_groups_with_pending_reqs' earlier
>>>    block, bfq: cleanup bfqq_group()
>>>
>>>   block/bfq-cgroup.c  | 13 +++----
>>>   block/bfq-iosched.c | 87 +++++++++++++++++++++++----------------------
>>>   block/bfq-iosched.h | 41 +++++++++++++--------
>>>   block/bfq-wf2q.c    | 56 +++++++++++++++--------------
>>>   4 files changed, 106 insertions(+), 91 deletions(-)
>>>

2022-04-01 18:32:10

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

friendly ping ...

在 2022/03/25 15:30, yukuai (C) 写道:
> friendly ping ...
>
> 在 2022/03/17 9:49, yukuai (C) 写道:
>> friendly ping ...
>>
>> 在 2022/03/11 14:31, yukuai (C) 写道:
>>> friendly ping ...
>>>
>>> 在 2022/03/05 17:11, Yu Kuai 写道:
>>>> 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().
>>>>
>>>> This patchset tries to support concurrent sync io if all the sync ios
>>>> are issued from the same cgroup:
>>>>
>>>> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>>>>
>>>> 2) Don't idle if 'num_groups_with_pending_reqs' is 1, patch 6;
>>>>
>>>> 3) Don't count the group if the group doesn't have pending requests,
>>>> while it's child groups may have pending requests, patch 7;
>>>>
>>>> This is because, for example:
>>>> if sync ios are issued from cgroup /root/c1/c2, root, c1 and c2
>>>> will all be counted into 'num_groups_with_pending_reqs',
>>>> which makes it impossible to handle sync ios concurrently.
>>>>
>>>> 4) Decrease 'num_groups_with_pending_reqs' when the last queue
>>>> completes
>>>> all the requests, while child groups may still have pending
>>>> requests, patch 8-10;
>>>>
>>>> This is because, for example:
>>>> t1 issue sync io on root group, t2 and t3 issue sync io on the same
>>>> child group. num_groups_with_pending_reqs is 2 now.
>>>> After t1 stopped, num_groups_with_pending_reqs is still 2. sync io from
>>>> t2 and t3 still can't be handled concurrently.
>>>>
>>>> fio test script: startdelay is used to avoid queue merging
>>>> [global]
>>>> filename=/dev/nvme0n1
>>>> allow_mounted_write=0
>>>> ioengine=psync
>>>> direct=1
>>>> ioscheduler=bfq
>>>> offset_increment=10g
>>>> group_reporting
>>>> rw=randwrite
>>>> bs=4k
>>>>
>>>> [test1]
>>>> numjobs=1
>>>>
>>>> [test2]
>>>> startdelay=1
>>>> numjobs=1
>>>>
>>>> [test3]
>>>> startdelay=2
>>>> numjobs=1
>>>>
>>>> [test4]
>>>> startdelay=3
>>>> numjobs=1
>>>>
>>>> [test5]
>>>> startdelay=4
>>>> numjobs=1
>>>>
>>>> [test6]
>>>> startdelay=5
>>>> numjobs=1
>>>>
>>>> [test7]
>>>> startdelay=6
>>>> numjobs=1
>>>>
>>>> [test8]
>>>> startdelay=7
>>>> numjobs=1
>>>>
>>>> test result:
>>>> running fio on root cgroup
>>>> v5.17-rc6:       550 Mib/s
>>>> v5.17-rc6-patched: 550 Mib/s
>>>>
>>>> running fio on non-root cgroup
>>>> v5.17-rc6:       349 Mib/s
>>>> v5.17-rc6-patched: 550 Mib/s
>>>>
>>>> Yu Kuai (11):
>>>>    block, bfq: add new apis to iterate bfq entities
>>>>    block, bfq: apply news apis where root group is not expected
>>>>    block, bfq: cleanup for __bfq_activate_requeue_entity()
>>>>    block, bfq: move the increasement of
>>>> 'num_groups_with_pending_reqs' to
>>>>      it's caller
>>>>    block, bfq: count root group into 'num_groups_with_pending_reqs'
>>>>    block, bfq: do not idle if only one cgroup is activated
>>>>    block, bfq: only count parent bfqg when bfqq is activated
>>>>    block, bfq: record how many queues have pending requests in
>>>> bfq_group
>>>>    block, bfq: move forward __bfq_weights_tree_remove()
>>>>    block, bfq: decrease 'num_groups_with_pending_reqs' earlier
>>>>    block, bfq: cleanup bfqq_group()
>>>>
>>>>   block/bfq-cgroup.c  | 13 +++----
>>>>   block/bfq-iosched.c | 87
>>>> +++++++++++++++++++++++----------------------
>>>>   block/bfq-iosched.h | 41 +++++++++++++--------
>>>>   block/bfq-wf2q.c    | 56 +++++++++++++++--------------
>>>>   4 files changed, 106 insertions(+), 91 deletions(-)
>>>>

2022-04-08 06:57:54

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

friendly ping ...

在 2022/04/01 11:43, yukuai (C) 写道:
> friendly ping ...
>
> 在 2022/03/25 15:30, yukuai (C) 写道:
>> friendly ping ...
>>
>> 在 2022/03/17 9:49, yukuai (C) 写道:
>>> friendly ping ...
>>>
>>> 在 2022/03/11 14:31, yukuai (C) 写道:
>>>> friendly ping ...
>>>>
>>>> 在 2022/03/05 17:11, Yu Kuai 写道:
>>>>> 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().
>>>>>
>>>>> This patchset tries to support concurrent sync io if all the sync ios
>>>>> are issued from the same cgroup:
>>>>>
>>>>> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>>>>>
>>>>> 2) Don't idle if 'num_groups_with_pending_reqs' is 1, patch 6;
>>>>>
>>>>> 3) Don't count the group if the group doesn't have pending requests,
>>>>> while it's child groups may have pending requests, patch 7;
>>>>>
>>>>> This is because, for example:
>>>>> if sync ios are issued from cgroup /root/c1/c2, root, c1 and c2
>>>>> will all be counted into 'num_groups_with_pending_reqs',
>>>>> which makes it impossible to handle sync ios concurrently.
>>>>>
>>>>> 4) Decrease 'num_groups_with_pending_reqs' when the last queue
>>>>> completes
>>>>> all the requests, while child groups may still have pending
>>>>> requests, patch 8-10;
>>>>>
>>>>> This is because, for example:
>>>>> t1 issue sync io on root group, t2 and t3 issue sync io on the same
>>>>> child group. num_groups_with_pending_reqs is 2 now.
>>>>> After t1 stopped, num_groups_with_pending_reqs is still 2. sync io
>>>>> from
>>>>> t2 and t3 still can't be handled concurrently.
>>>>>
>>>>> fio test script: startdelay is used to avoid queue merging
>>>>> [global]
>>>>> filename=/dev/nvme0n1
>>>>> allow_mounted_write=0
>>>>> ioengine=psync
>>>>> direct=1
>>>>> ioscheduler=bfq
>>>>> offset_increment=10g
>>>>> group_reporting
>>>>> rw=randwrite
>>>>> bs=4k
>>>>>
>>>>> [test1]
>>>>> numjobs=1
>>>>>
>>>>> [test2]
>>>>> startdelay=1
>>>>> numjobs=1
>>>>>
>>>>> [test3]
>>>>> startdelay=2
>>>>> numjobs=1
>>>>>
>>>>> [test4]
>>>>> startdelay=3
>>>>> numjobs=1
>>>>>
>>>>> [test5]
>>>>> startdelay=4
>>>>> numjobs=1
>>>>>
>>>>> [test6]
>>>>> startdelay=5
>>>>> numjobs=1
>>>>>
>>>>> [test7]
>>>>> startdelay=6
>>>>> numjobs=1
>>>>>
>>>>> [test8]
>>>>> startdelay=7
>>>>> numjobs=1
>>>>>
>>>>> test result:
>>>>> running fio on root cgroup
>>>>> v5.17-rc6:       550 Mib/s
>>>>> v5.17-rc6-patched: 550 Mib/s
>>>>>
>>>>> running fio on non-root cgroup
>>>>> v5.17-rc6:       349 Mib/s
>>>>> v5.17-rc6-patched: 550 Mib/s
>>>>>
>>>>> Yu Kuai (11):
>>>>>    block, bfq: add new apis to iterate bfq entities
>>>>>    block, bfq: apply news apis where root group is not expected
>>>>>    block, bfq: cleanup for __bfq_activate_requeue_entity()
>>>>>    block, bfq: move the increasement of
>>>>> 'num_groups_with_pending_reqs' to
>>>>>      it's caller
>>>>>    block, bfq: count root group into 'num_groups_with_pending_reqs'
>>>>>    block, bfq: do not idle if only one cgroup is activated
>>>>>    block, bfq: only count parent bfqg when bfqq is activated
>>>>>    block, bfq: record how many queues have pending requests in
>>>>> bfq_group
>>>>>    block, bfq: move forward __bfq_weights_tree_remove()
>>>>>    block, bfq: decrease 'num_groups_with_pending_reqs' earlier
>>>>>    block, bfq: cleanup bfqq_group()
>>>>>
>>>>>   block/bfq-cgroup.c  | 13 +++----
>>>>>   block/bfq-iosched.c | 87
>>>>> +++++++++++++++++++++++----------------------
>>>>>   block/bfq-iosched.h | 41 +++++++++++++--------
>>>>>   block/bfq-wf2q.c    | 56 +++++++++++++++--------------
>>>>>   4 files changed, 106 insertions(+), 91 deletions(-)
>>>>>

2022-04-13 15:46:11

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

On Sat 05-03-22 17:11:54, Yu Kuai wrote:
> 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().
>
> This patchset tries to support concurrent sync io if all the sync ios
> are issued from the same cgroup:
>
> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;

Seeing the complications and special casing for root_group I wonder: Won't
we be better off to create fake bfq_sched_data in bfq_data and point
root_group->sched_data there? AFAICS it would simplify the code
considerably as root_group would be just another bfq_group, no need to
special case it in various places, no games with bfqg->my_entity, etc.
Paolo, do you see any problem with that?

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2022-04-13 18:25:08

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion

?? 2022/04/13 19:12, Jan Kara д??:
> On Sat 05-03-22 17:11:54, Yu Kuai wrote:
>> 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().
>>
>> This patchset tries to support concurrent sync io if all the sync ios
>> are issued from the same cgroup:
>>
>> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>
> Seeing the complications and special casing for root_group I wonder: Won't
> we be better off to create fake bfq_sched_data in bfq_data and point
> root_group->sched_data there? AFAICS it would simplify the code

Hi,

That sounds an good idel, in this case we only need to make sure the
fake service tree will always be empty, which means we only need to
special casing bfq_active/idle_insert to the fake service tree.

Thanks,
Kuai
> considerably as root_group would be just another bfq_group, no need to
> special case it in various places, no games with bfqg->my_entity, etc.
> Paolo, do you see any problem with that?
>
> Honza
>

2022-04-26 19:03:59

by Paolo Valente

[permalink] [raw]
Subject: Re: [PATCH -next 00/11] support concurrent sync io for bfq on a specail occasion



> Il giorno 13 apr 2022, alle ore 13:12, Jan Kara <[email protected]> ha scritto:
>
> On Sat 05-03-22 17:11:54, Yu Kuai wrote:
>> 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().
>>
>> This patchset tries to support concurrent sync io if all the sync ios
>> are issued from the same cgroup:
>>
>> 1) Count root_group into 'num_groups_with_pending_reqs', patch 1-5;
>
> Seeing the complications and special casing for root_group I wonder: Won't
> we be better off to create fake bfq_sched_data in bfq_data and point
> root_group->sched_data there? AFAICS it would simplify the code
> considerably as root_group would be just another bfq_group, no need to
> special case it in various places, no games with bfqg->my_entity, etc.
> Paolo, do you see any problem with that?
>

I do see the benefits. My only concern is that then we also need to
check/change the places that rely on the assumption that we would
change.

Thanks,
Paolo

> Honza
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR