Re: [RFC PATCH 0/7] block: improve iops limit throttle

( Sorry for sending this mail again,  I am not sure you saw that for the last one was not in pure text format.)

Hi all,

In short, Ming’s patchset was tested against kernel master and works well.  Tested-by: lining [off-list ref] 


The iops limit test result on thin-dm lv shows quite perfect and IO was much more smooth during the testing, very cool! 

The origin issue is #bz2027241 named `cgroup2: wiops limit doesn't work when doing sequential write on a thin provisioned lv`,
There are more details about this issue and patch work in this bugzilla ticket [1].

Follows my test result and test script (iops was limit to 10 in my case):
- dm thin case test result: https://pastebin.com/raw/VgvB2TFY
- dm plain case test result: https://pastebin.com/raw/UVZMYjTp
- test script: https://pastebin.com/raw/YXEDH6Ss
 
Here is my code tree info for testing:

root@ubuntu-r:~/backup/linux# git remote -v 
origin	https://github.com/torvalds/linux.git (fetch)
origin	https://github.com/torvalds/linux.git (push)

root@ubuntu-r:~/backup/linux# git branch -a
* master
  remotes/origin/master

root@ubuntu-r:~/backup/linux# git log --oneline -10
9b977519b97c (HEAD -> master) block: revert 4f1e9630afe6 ("blk-throtl: optimize IOPS throttle for large IO scenarios")
36beefdf7492 block: don't try to throttle split bio if iops limit isn't set
722ff89df455 block: throttle split bio in case of iops limit
778b7c819d8c block: don't check bio in blk_throtl_dispatch_work_fn
c803548b4623 block: allow to bypass bio check before submitting bio
936bc02492c2 block: move blk_crypto_bio_prep() out of blk-mq.c
98e2c0e19ca6 block: move submit_bio_checks() into submit_bio_noacct
455e73a07f6e (origin/master) Merge tag 'clk-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux

d9b5941bb593 Merge tag 'leds-5.17-rc1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds

4eb766f64d12 Merge tag 'devicetree-for-5.17' of 
git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux

[1] bz2027241:  https://bugzilla.redhat.com/show_bug.cgi?id=2027241
 
Thanks,
Ning

2022年1月13日 下午12:26，Ning Li [off-list ref] 写道：

Hi all,

The iops limit test result on thin-dm lv shows quite perfect, very cool!  Tested-by: lining [off-list ref]

The origin issue is #bz2027241 named `cgroup2: wiops limit doesn't work when doing sequential write on a thin provisioned lv`,
There are more details about this issue and patch work in this bugzilla ticket [1].

Follows my test result and test script (iops was limit to 10 in my case):
- dm thin case test result: https://pastebin.com/raw/VgvB2TFY
- dm plain case test result: https://pastebin.com/raw/UVZMYjTp
- test script: https://pastebin.com/raw/YXEDH6Ss
 
Here is my code tree info for testing:

root@ubuntu-r:~/backup/linux# git remote -v
origin	https://github.com/torvalds/linux.git (fetch)
origin	https://github.com/torvalds/linux.git (push)

root@ubuntu-r:~/backup/linux# git branch -a
* master
  remotes/origin/master

root@ubuntu-r:~/backup/linux# git log --oneline -10
9b977519b97c (HEAD -> master) block: revert 4f1e9630afe6 ("blk-throtl: optimize IOPS throttle for large IO scenarios")
36beefdf7492 block: don't try to throttle split bio if iops limit isn't set
722ff89df455 block: throttle split bio in case of iops limit
778b7c819d8c block: don't check bio in blk_throtl_dispatch_work_fn
c803548b4623 block: allow to bypass bio check before submitting bio
936bc02492c2 block: move blk_crypto_bio_prep() out of blk-mq.c
98e2c0e19ca6 block: move submit_bio_checks() into submit_bio_noacct
455e73a07f6e (origin/master) Merge tag 'clk-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
d9b5941bb593 Merge tag 'leds-5.17-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds
4eb766f64d12 Merge tag 'devicetree-for-5.17' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux

[1] bz2027241:  https://bugzilla.redhat.com/show_bug.cgi?id=2027241
 
Thanks,
Ning


At 2022-01-12 16:29:10, "Ning Li" [off-list ref] wrote:

Cool, I will test it later.

Thanks, 
Ning
At 2022-01-11 19:55:25, "Ming Lei" [off-list ref] wrote:

quoted

Hello Guys,

Lining reported that iops limit throttle doesn't work on dm-thin, also
iops limit throttle works bad on plain disk in case of excessive split.

Commit 4f1e9630afe6 ("blk-throtl: optimize IOPS throttle for large IO scenarios")
was for addressing this issue, but the taken approach is just to run
post-accounting, then current split bios won't be throttled actually,
so actual iops throttle result isn't good in case of excessive bio
splitting.

The 1st two patches are cleanup.

The 3rd and 4th patches add one new helper of __submit_bio_noacct() for
blk_throtl_dispatch_work_fn(), so that bios won't be throttled any more
when blk-throttle code dispatches throttled bios.

The 5th and 6th patch makes the real difference for throttling split bio wrt.
iops limit.

The last patch is to revert commit 4f1e9630afe6 ("blk-throtl: optimize IOPS
throttle for large IO scenarios").

Lining, you should get exact IOPS throttling in your dm-thin test with
this patchset, please test and feedback.


Ming Lei (7):
 block: move submit_bio_checks() into submit_bio_noacct
 block: move blk_crypto_bio_prep() out of blk-mq.c
 block: allow to bypass bio check before submitting bio
 block: don't check bio in blk_throtl_dispatch_work_fn
 block: throttle split bio in case of iops limit
 block: don't try to throttle split bio if iops limit isn't set
 block: revert 4f1e9630afe6 ("blk-throtl: optimize IOPS throttle for
   large IO scenarios")

block/blk-core.c       | 32 +++++++++-------------
block/blk-merge.c      |  2 --
block/blk-mq.c         |  3 ---
block/blk-throttle.c   | 61 +++++++++++++++---------------------------
block/blk-throttle.h   | 16 ++++++-----
include/linux/blkdev.h |  7 ++++-
6 files changed, 51 insertions(+), 70 deletions(-)

-- 
2.31.1