blkcg: Before starting a new slice, firstly count bps/iops limit in func tg_may_dispatch.
Subsystems (2)
PATCH 1/1 blkcg: Before starting a new slice, firstly count bps/iops limit in func tg_may_dispatch.
In func tg_may_dispatch,
Quoted 2 lines
if (
throtl_slice_used(td, tg, rw))throtl_start_new_slice(td, tg, rw);
...
Quoted 2 lines
if (
tg_with_in_bps_limit(td, tg, bio, &bps_wait)&& tg_with_in_iops_limit(td, tg, bio, &iops_wait)) {
In funcs tg_with_in_(bps/iops)_limit, it used the slice_start to count. So if privious slice expired, it start a new slice.This can cause hung task.
The next steps can repeat this bug.
1:echo "8:48 10240" > blkio.throttle.write_bps_devic 2:dd if=/dev/zero of=/dev/sdd bs=1M count=1 oflag=direct
Using the blktrace, the messages about throttle:
root@kernel:/mnt/programs# blktrace -d /dev/sdd -a notify -o -|blkparse -i - 8,48 1 0 0.000000000 0 m N throtl / [W] new slice start=4294854679 end=4294854779 jiffies=4294854679 8,48 1 0 0.000000966 0 m N throtl / [W] extend slice start=4294854679 end=4294905900 jiffies=4294854679 8,48 1 0 0.000002553 0 m N throtl / [W] bio. bdisp=0 sz=524288 bps=10240 iodisp=0 iops=4294967295 queued=0/0 8,48 1 0 0.000004788 0 m N throtl schedule work. delay=51200 jiffies=4294854679 8,48 1 0 51.304698681 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 51.304701979 0 m N throtl / [W] new slice start=4294905984 end=4294906084 jiffies=4294905984 8,48 1 0 51.304703329 0 m N throtl / [W] extend slice start=4294905984 end=4294957200 jiffies=4294905984 8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984 8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 102.632700544 0 m N throtl / [W] new slice start=4294957312 end=4294957412 jiffies=4294957312 8,48 1 0 102.632701922 0 m N throtl / [W] extend slice start=4294957312 end=4295008600 jiffies=4294957312 8,48 1 0 102.632704016 0 m N throtl schedule work. delay=51200 jiffies=4294957312 8,48 1 0 153.960696503 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 153.960699797 0 m N throtl / [W] new slice start=4295008640 end=4295008740 jiffies=4295008640 8,48 1 0 153.960701153 0 m N throtl / [W] extend slice start=4295008640 end=4295059900 jiffies=4295008640 8,48 1 0 153.960703218 0 m N throtl schedule work. delay=51200 jiffies=4295008640 8,48 1 0 205.288697067 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 205.288700268 0 m N throtl / [W] new slice start=4295059968 end=4295060068 jiffies=4295059968 8,48 1 0 205.288701630 0 m N throtl / [W] extend slice start=4295059968 end=4295111200 jiffies=4295059968 8,48 1 0 205.288703784 0 m N throtl schedule work. delay=51200 jiffies=4295059968 8,48 1 0 256.616696184 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 256.616699266 0 m N throtl / [W] new slice start=4295111296 end=4295111396 jiffies=4295111296 8,48 1 0 256.616700574 0 m N throtl / [W] extend slice start=4295111296 end=4295162500 jiffies=4295111296 8,48 1 0 256.616702701 0 m N throtl schedule work. delay=51200 jiffies=4295111296
--- block/blk-throttle.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
block/blk-throttle.c +10 -5
@@ -645,6 +645,15 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, }/* +* If privious slice expired,then start new slice. +* But counting bps and iops limit need privious slice info +* which ->slice_start. +*/ +if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) + &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) +if (wait) + *wait =0 ; +/* * If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now.
@@ -656,12 +665,8 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg,throtl_extend_slice (td, tg, rw, jiffies + throtl_slice); } -if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) - &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) { -if (wait) - *wait =0 ; +if (!(bps_wait || iops_wait))return 1 ; - } max_wait =max (bps_wait, iops_wait);
Signature · 5 lines
1.7.9.5
Jianpeng Ma
Thanks!�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay� 蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��� �鹅�⒏?朓�Jens Axboe
maintainerQuoted 70 lines · majianpeng
In func
tg_may_dispatch,Quoted 2 lines
if (
throtl_slice_used(td, tg, rw))throtl_start_new_slice(td, tg, rw);...
Quoted 2 lines
if (
tg_with_in_bps_limit(td, tg, bio, &bps_wait)&& tg_with_in_iops_limit(td, tg, bio, &iops_wait)) {In funcs tg_with_in_(bps/iops)_limit, it used the
slice_startto count. So if privious slice expired, it start a new slice.This can cause hung task.The next steps can repeat this bug.
1:echo "8:48 10240" > blkio.throttle.write_bps_devic 2:dd if=/dev/zero of=/dev/sdd bs=1M count=1 oflag=directUsing the blktrace, the messages about throttle:
root@kernel:/mnt/programs# blktrace -d /dev/sdd -a notify -o -|blkparse -i - 8,48 1 0 0.000000000 0 m N throtl / [W] new slice start=4294854679 end=4294854779 jiffies=4294854679 8,48 1 0 0.000000966 0 m N throtl / [W] extend slice start=4294854679 end=4294905900 jiffies=4294854679 8,48 1 0 0.000002553 0 m N throtl / [W] bio. bdisp=0 sz=524288 bps=10240 iodisp=0 iops=4294967295 queued=0/0 8,48 1 0 0.000004788 0 m N throtl schedule work. delay=51200 jiffies=4294854679 8,48 1 0 51.304698681 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 51.304701979 0 m N throtl / [W] new slice start=4294905984 end=4294906084 jiffies=4294905984 8,48 1 0 51.304703329 0 m N throtl / [W] extend slice start=4294905984 end=4294957200 jiffies=4294905984 8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984 8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 102.632700544 0 m N throtl / [W] new slice start=4294957312 end=4294957412 jiffies=4294957312 8,48 1 0 102.632701922 0 m N throtl / [W] extend slice start=4294957312 end=4295008600 jiffies=4294957312 8,48 1 0 102.632704016 0 m N throtl schedule work. delay=51200 jiffies=4294957312 8,48 1 0 153.960696503 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 153.960699797 0 m N throtl / [W] new slice start=4295008640 end=4295008740 jiffies=4295008640 8,48 1 0 153.960701153 0 m N throtl / [W] extend slice start=4295008640 end=4295059900 jiffies=4295008640 8,48 1 0 153.960703218 0 m N throtl schedule work. delay=51200 jiffies=4295008640 8,48 1 0 205.288697067 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 205.288700268 0 m N throtl / [W] new slice start=4295059968 end=4295060068 jiffies=4295059968 8,48 1 0 205.288701630 0 m N throtl / [W] extend slice start=4295059968 end=4295111200 jiffies=4295059968 8,48 1 0 205.288703784 0 m N throtl schedule work. delay=51200 jiffies=4295059968 8,48 1 0 256.616696184 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 256.616699266 0 m N throtl / [W] new slice start=4295111296 end=4295111396 jiffies=4295111296 8,48 1 0 256.616700574 0 m N throtl / [W] extend slice start=4295111296 end=4295162500 jiffies=4295111296 8,48 1 0 256.616702701 0 m N throtl schedule work. delay=51200 jiffies=4295111296Signed-off-by Jianpeng Ma <[email protected]>--- block/blk-throttle.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)block/blk-throttle.c +9
-0@@ -645,6 +645,15 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, }/* +* If privious slice expired,then start new slice. +* But counting bps and iops limit need privious slice info +* which ->slice_start. +*/ +if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) + &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) +if (wait) + *wait =0 ; +/* * If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. @@ -656,12 +665,8 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg,throtl_extend_slice (td, tg, rw, jiffies + throtl_slice); }
block/blk-throttle.c static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, · line 671
Quoted 10 lines
block/blk-throttle.c +1
-5-if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) - &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) { -if (wait) - *wait =0 ; +if (!(bps_wait || iops_wait))return 1 ; - } max_wait =max (bps_wait, iops_wait);
Looks pretty sane to me. Vivek?
Signature · 1 line
Jens Axboe
Quoted 80 lines · Jens Axboe
Quoted 79 lines · majianpeng
In func
tg_may_dispatch,Quoted 2 lines
if (
throtl_slice_used(td, tg, rw))throtl_start_new_slice(td, tg, rw);...
Quoted 2 lines
if (
tg_with_in_bps_limit(td, tg, bio, &bps_wait)&& tg_with_in_iops_limit(td, tg, bio, &iops_wait)) {In funcs tg_with_in_(bps/iops)_limit, it used the
slice_startto count. So if privious slice expired, it start a new slice.This can cause hung task.The next steps can repeat this bug.
1:echo "8:48 10240" > blkio.throttle.write_bps_devic 2:dd if=/dev/zero of=/dev/sdd bs=1M count=1 oflag=directUsing the blktrace, the messages about throttle:
root@kernel:/mnt/programs# blktrace -d /dev/sdd -a notify -o -|blkparse -i - 8,48 1 0 0.000000000 0 m N throtl / [W] new slice start=4294854679 end=4294854779 jiffies=4294854679 8,48 1 0 0.000000966 0 m N throtl / [W] extend slice start=4294854679 end=4294905900 jiffies=4294854679 8,48 1 0 0.000002553 0 m N throtl / [W] bio. bdisp=0 sz=524288 bps=10240 iodisp=0 iops=4294967295 queued=0/0 8,48 1 0 0.000004788 0 m N throtl schedule work. delay=51200 jiffies=4294854679 8,48 1 0 51.304698681 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 51.304701979 0 m N throtl / [W] new slice start=4294905984 end=4294906084 jiffies=4294905984 8,48 1 0 51.304703329 0 m N throtl / [W] extend slice start=4294905984 end=4294957200 jiffies=4294905984 8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984 8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 102.632700544 0 m N throtl / [W] new slice start=4294957312 end=4294957412 jiffies=4294957312 8,48 1 0 102.632701922 0 m N throtl / [W] extend slice start=4294957312 end=4295008600 jiffies=4294957312 8,48 1 0 102.632704016 0 m N throtl schedule work. delay=51200 jiffies=4294957312 8,48 1 0 153.960696503 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 153.960699797 0 m N throtl / [W] new slice start=4295008640 end=4295008740 jiffies=4295008640 8,48 1 0 153.960701153 0 m N throtl / [W] extend slice start=4295008640 end=4295059900 jiffies=4295008640 8,48 1 0 153.960703218 0 m N throtl schedule work. delay=51200 jiffies=4295008640 8,48 1 0 205.288697067 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 205.288700268 0 m N throtl / [W] new slice start=4295059968 end=4295060068 jiffies=4295059968 8,48 1 0 205.288701630 0 m N throtl / [W] extend slice start=4295059968 end=4295111200 jiffies=4295059968 8,48 1 0 205.288703784 0 m N throtl schedule work. delay=51200 jiffies=4295059968 8,48 1 0 256.616696184 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 256.616699266 0 m N throtl / [W] new slice start=4295111296 end=4295111396 jiffies=4295111296 8,48 1 0 256.616700574 0 m N throtl / [W] extend slice start=4295111296 end=4295162500 jiffies=4295111296 8,48 1 0 256.616702701 0 m N throtl schedule work. delay=51200 jiffies=4295111296Signed-off-by Jianpeng Ma <[email protected]>--- block/blk-throttle.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)block/blk-throttle.c +10
-5@@ -645,6 +645,15 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, }/* +* If privious slice expired,then start new slice. +* But counting bps and iops limit need privious slice info +* which ->slice_start. +*/ +if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) + &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) +if (wait) + *wait =0 ; +/* * If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. @@ -656,12 +665,8 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg,throtl_extend_slice (td, tg, rw, jiffies + throtl_slice); } -if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) - &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) { -if (wait) - *wait =0 ; +if (!(bps_wait || iops_wait))return 1 ; - }
Quoted 3 lines
block/blk-throttle.c static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, · line 671
Quoted 1 line
block/blk-throttle.c +0
-0max_wait =max (bps_wait, iops_wait);Looks pretty sane to me. Vivek?
Hi Jens,
This fix will introduce other side affects. And that is when an group has been idle for few seconds and a new IO gets queued in, we will not start a new slice and allow dispatch equivalent of those idle seconds before we throttle the group. So keeping group idle will become an incentive.
I have attached a patch which might work better. majianpeng, can you please give it a try.
Having said that it is strange that workqueue thread is triggering so late. Looking at timestamp of traces attached.
We scheduled a delayed work. Following is trace.
8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984
Now a worker should execute after 51.2 seconds (delay=51200).
8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1
But worker executed at delay of 51.328 seconds (102.632 - 51.304). That is a delay of around 128 jiffies (51.328 - 51.2). That kind of seems odd. AFAIK, workers are fairly quick to execute after expiry. And it is because of this excessive delay that we think slice has expired.
May be it is something new. CCing Tejun, if he has seen anything like this and may be it is a known issue.
Even if it is a worker thread issue, I think applying below patch makes sense.
Thanks
Vivek
blk-throttle: Do not start a new slice if IO is pending
It might happen that we queue an IO (because it overshot the rate) and then wait for certain jiffies to pass. We will set slice_end accordingly and wait for that time to elapse and worker thread will kick in, again evaluate whether group can dispatch or not. Now it might happen that current time is after slice_end and tg_may_dispatch() will start a new slice.
This will result in IO not being dispatched and be put back on wait again. And this will repeat, resulting in hang.
Do not start a new slice if an IO is pending in that group in the direction being queired. Instead extend the slice. New slice is supposed to start when a group has not been doing IO for some time and a new IO shows up. In that case we want do discard history and start a new slice.
--- block/blk-throttle.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
block/blk-throttle.c +7 -1
@@ -648,8 +648,14 @@ static bool tg_may_dispatch(struct throt* If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. +* +* Start a new slice only if there is no bio queued in that direction. +* That bio is waiting to be dispatched and slice needs to be +* extended. It might happen that bio waited to be dispatched but +* workqueue execution got little late it might restart a new slice +* instead of taking all the waited time into account. */ -if (throtl_slice_used (td, tg, rw)) +if (throtl_slice_used (td, tg, rw) && !tg->nr_queued[rw])throtl_start_new_slice (td, tg, rw);else {if (time_before (tg->slice_end[rw], jiffies + throtl_slice))
Quoted 155 lines · Jens Axboe
Quoted 80 lines · majianpeng
Quoted 79 lines
In func
tg_may_dispatch,Quoted 2 lines
if (
throtl_slice_used(td, tg, rw))throtl_start_new_slice(td, tg, rw);...
Quoted 2 lines
if (
tg_with_in_bps_limit(td, tg, bio, &bps_wait)&& tg_with_in_iops_limit(td, tg, bio, &iops_wait)) {In funcs tg_with_in_(bps/iops)_limit, it used the
slice_startto count. So if privious slice expired, it start a new slice.This can cause hung task.The next steps can repeat this bug.
1:echo "8:48 10240" > blkio.throttle.write_bps_devic 2:dd if=/dev/zero of=/dev/sdd bs=1M count=1 oflag=directUsing the blktrace, the messages about throttle:
root@kernel:/mnt/programs# blktrace -d /dev/sdd -a notify -o -|blkparse -i - 8,48 1 0 0.000000000 0 m N throtl / [W] new slice start=4294854679 end=4294854779 jiffies=4294854679 8,48 1 0 0.000000966 0 m N throtl / [W] extend slice start=4294854679 end=4294905900 jiffies=4294854679 8,48 1 0 0.000002553 0 m N throtl / [W] bio. bdisp=0 sz=524288 bps=10240 iodisp=0 iops=4294967295 queued=0/0 8,48 1 0 0.000004788 0 m N throtl schedule work. delay=51200 jiffies=4294854679 8,48 1 0 51.304698681 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 51.304701979 0 m N throtl / [W] new slice start=4294905984 end=4294906084 jiffies=4294905984 8,48 1 0 51.304703329 0 m N throtl / [W] extend slice start=4294905984 end=4294957200 jiffies=4294905984 8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984 8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 102.632700544 0 m N throtl / [W] new slice start=4294957312 end=4294957412 jiffies=4294957312 8,48 1 0 102.632701922 0 m N throtl / [W] extend slice start=4294957312 end=4295008600 jiffies=4294957312 8,48 1 0 102.632704016 0 m N throtl schedule work. delay=51200 jiffies=4294957312 8,48 1 0 153.960696503 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 153.960699797 0 m N throtl / [W] new slice start=4295008640 end=4295008740 jiffies=4295008640 8,48 1 0 153.960701153 0 m N throtl / [W] extend slice start=4295008640 end=4295059900 jiffies=4295008640 8,48 1 0 153.960703218 0 m N throtl schedule work. delay=51200 jiffies=4295008640 8,48 1 0 205.288697067 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 205.288700268 0 m N throtl / [W] new slice start=4295059968 end=4295060068 jiffies=4295059968 8,48 1 0 205.288701630 0 m N throtl / [W] extend slice start=4295059968 end=4295111200 jiffies=4295059968 8,48 1 0 205.288703784 0 m N throtl schedule work. delay=51200 jiffies=4295059968 8,48 1 0 256.616696184 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 256.616699266 0 m N throtl / [W] new slice start=4295111296 end=4295111396 jiffies=4295111296 8,48 1 0 256.616700574 0 m N throtl / [W] extend slice start=4295111296 end=4295162500 jiffies=4295111296 8,48 1 0 256.616702701 0 m N throtl schedule work. delay=51200 jiffies=4295111296Signed-off-by Jianpeng Ma <[email protected]>--- block/blk-throttle.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)block/blk-throttle.c +10
-5@@ -645,6 +645,15 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, }/* +* If privious slice expired,then start new slice. +* But counting bps and iops limit need privious slice info +* which ->slice_start. +*/ +if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) + &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) +if (wait) + *wait =0 ; +/* * If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. @@ -656,12 +665,8 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg,throtl_extend_slice (td, tg, rw, jiffies + throtl_slice); } -if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) - &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) { -if (wait) - *wait =0 ; +if (!(bps_wait || iops_wait))return 1 ; - }Quoted 3 lines
block/blk-throttle.c static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, · line 671
Quoted 1 line
block/blk-throttle.c +0
-0max_wait =max (bps_wait, iops_wait);Looks pretty sane to me. Vivek?
Hi Jens,
This fix will introduce other side affects. And that is when an group has been idle for few seconds and a new IO gets queued in, we will not start a new slice and allow dispatch equivalent of those idle seconds before we throttle the group. So keeping group idle will become an incentive.
I have attached a patch which might work better. majianpeng, can you please give it a try.
Having said that it is strange that workqueue thread is triggering so late. Looking at timestamp of traces attached.
We scheduled a delayed work. Following is trace.
8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984
Now a worker should execute after 51.2 seconds (delay=51200).
8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1
But worker executed at delay of 51.328 seconds (102.632 - 51.304). That is a delay of around 128 jiffies (51.328 - 51.2). That kind of seems odd. AFAIK, workers are fairly quick to execute after expiry. And it is because of this excessive delay that we think slice has expired.
May be it is something new. CCing Tejun, if he has seen anything like this and may be it is a known issue.
Even if it is a worker thread issue, I think applying below patch makes sense.
Thanks
Vivekblk-throttle: Do not start a new slice if IO is pending
It might happen that we queue an IO (because it overshot the rate) and then wait for certain jiffies to pass. We will set
slice_endaccordingly and wait for that time to elapse and worker thread will kick in, again evaluate whether group can dispatch or not. Now it might happen that current time is afterslice_endandtg_may_dispatch()will start a new slice.This will result in IO not being dispatched and be put back on wait again. And this will repeat, resulting in hang.
Do not start a new slice if an IO is pending in that group in the direction being queired. Instead extend the slice. New slice is supposed to start when a group has not been doing IO for some time and a new IO shows up. In that case we want do discard history and start a new slice.
Signed-off-by Vivek Goyal <[email protected]>---
block/blk-throttle.c| 8 +++++++-1 file changed, 7 insertions(+), 1 deletion(-)Index:
linux-2.6/block/blk-throttle.c
===================================================================block/blk-throttle.c +2
-0@@ -648,8 +648,14 @@ static bool tg_may_dispatch(struct throt* If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. +* +* Start a new slice only if there is no bio queued in that direction.
block/blk-throttle.c static bool tg_may_dispatch(struct throt · line 661
Quoted 10 lines
block/blk-throttle.c +5
-1+* That bio is waiting to be dispatched and slice needs to be +* extended. It might happen that bio waited to be dispatched but +* workqueue execution got little late it might restart a new slice +* instead of taking all the waited time into account. */ -if (throtl_slice_used (td, tg, rw)) +if (throtl_slice_used (td, tg, rw) && !tg->nr_queued[rw])throtl_start_new_slice (td, tg, rw);else {if (time_before (tg->slice_end[rw], jiffies + throtl_slice))
Hi vivek,
Your patch is ok.But i had a question:
What's condition tg->nr_queued[rw] = 0, but bio is not null?
Thanks! Jianpeng�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay� 蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��� �鹅�⒏?朓�
On Mon, Jan 14, 2013 at 10:15:58AM +0800, majianpeng wrote:
[..]
Quoted 15 lines
Quoted 15 lines
Index:
linux-2.6/block/blk-throttle.c
===================================================================block/blk-throttle.c +6
-0@@ -648,8 +648,14 @@ static bool tg_may_dispatch(struct throt* If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. +* +* Start a new slice only if there is no bio queued in that direction. +* That bio is waiting to be dispatched and slice needs to be +* extended. It might happen that bio waited to be dispatched but +* workqueue execution got little late it might restart a new slice +* instead of taking all the waited time into account. */
Quoted 8 lines
block/blk-throttle.c static bool tg_may_dispatch(struct throt · line 661
Quoted 5 lines
block/blk-throttle.c +1
-1-if (throtl_slice_used (td, tg, rw)) +if (throtl_slice_used (td, tg, rw) && !tg->nr_queued[rw])throtl_start_new_slice (td, tg, rw);else {if (time_before (tg->slice_end[rw], jiffies + throtl_slice))Hi vivek,
Your patch is ok.But i had a question:
What's condition tg->nr_queued[rw] = 0, but bio is not null?
When a new bio is about to be queued in an empty group (look at blk_throtl_bio). At that time, tg->nr_queued[rw] might be 0 if no other bio is already queued.
Thanks
Vivek
Quoted 27 lines · majianpeng
[..]
Quoted 15 lines
Quoted 15 lines
Index:
linux-2.6/block/blk-throttle.c
===================================================================block/blk-throttle.c +6
-0@@ -648,8 +648,14 @@ static bool tg_may_dispatch(struct throt* If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. +* +* Start a new slice only if there is no bio queued in that direction. +* That bio is waiting to be dispatched and slice needs to be +* extended. It might happen that bio waited to be dispatched but +* workqueue execution got little late it might restart a new slice +* instead of taking all the waited time into account. */ Quoted 8 lines
block/blk-throttle.c static bool tg_may_dispatch(struct throt · line 661
Quoted 5 lines
block/blk-throttle.c +1
-1-if (throtl_slice_used (td, tg, rw)) +if (throtl_slice_used (td, tg, rw) && !tg->nr_queued[rw])throtl_start_new_slice (td, tg, rw);else {if (time_before (tg->slice_end[rw], jiffies + throtl_slice))Hi vivek,
Your patch is ok.But i had a question:
What's condition tg->nr_queued[rw] = 0, but bio is not null?
Quoted 6 lines
When a new bio is about to be queued in an empty group (look at
blk_throtl_bio). At that time, tg->nr_queued[rw] might be 0 if no other bio is already queued.Thanks
Vivek
I see. Thanks your time!
Jianpeng Thanks!�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay� 蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��� �鹅�⒏?朓�
Quoted 155 lines · Jens Axboe
Quoted 80 lines · majianpeng
Quoted 79 lines
In func
tg_may_dispatch,Quoted 2 lines
if (
throtl_slice_used(td, tg, rw))throtl_start_new_slice(td, tg, rw);...
Quoted 2 lines
if (
tg_with_in_bps_limit(td, tg, bio, &bps_wait)&& tg_with_in_iops_limit(td, tg, bio, &iops_wait)) {In funcs tg_with_in_(bps/iops)_limit, it used the
slice_startto count. So if privious slice expired, it start a new slice.This can cause hung task.The next steps can repeat this bug.
1:echo "8:48 10240" > blkio.throttle.write_bps_devic 2:dd if=/dev/zero of=/dev/sdd bs=1M count=1 oflag=directUsing the blktrace, the messages about throttle:
root@kernel:/mnt/programs# blktrace -d /dev/sdd -a notify -o -|blkparse -i - 8,48 1 0 0.000000000 0 m N throtl / [W] new slice start=4294854679 end=4294854779 jiffies=4294854679 8,48 1 0 0.000000966 0 m N throtl / [W] extend slice start=4294854679 end=4294905900 jiffies=4294854679 8,48 1 0 0.000002553 0 m N throtl / [W] bio. bdisp=0 sz=524288 bps=10240 iodisp=0 iops=4294967295 queued=0/0 8,48 1 0 0.000004788 0 m N throtl schedule work. delay=51200 jiffies=4294854679 8,48 1 0 51.304698681 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 51.304701979 0 m N throtl / [W] new slice start=4294905984 end=4294906084 jiffies=4294905984 8,48 1 0 51.304703329 0 m N throtl / [W] extend slice start=4294905984 end=4294957200 jiffies=4294905984 8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984 8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 102.632700544 0 m N throtl / [W] new slice start=4294957312 end=4294957412 jiffies=4294957312 8,48 1 0 102.632701922 0 m N throtl / [W] extend slice start=4294957312 end=4295008600 jiffies=4294957312 8,48 1 0 102.632704016 0 m N throtl schedule work. delay=51200 jiffies=4294957312 8,48 1 0 153.960696503 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 153.960699797 0 m N throtl / [W] new slice start=4295008640 end=4295008740 jiffies=4295008640 8,48 1 0 153.960701153 0 m N throtl / [W] extend slice start=4295008640 end=4295059900 jiffies=4295008640 8,48 1 0 153.960703218 0 m N throtl schedule work. delay=51200 jiffies=4295008640 8,48 1 0 205.288697067 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 205.288700268 0 m N throtl / [W] new slice start=4295059968 end=4295060068 jiffies=4295059968 8,48 1 0 205.288701630 0 m N throtl / [W] extend slice start=4295059968 end=4295111200 jiffies=4295059968 8,48 1 0 205.288703784 0 m N throtl schedule work. delay=51200 jiffies=4295059968 8,48 1 0 256.616696184 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 256.616699266 0 m N throtl / [W] new slice start=4295111296 end=4295111396 jiffies=4295111296 8,48 1 0 256.616700574 0 m N throtl / [W] extend slice start=4295111296 end=4295162500 jiffies=4295111296 8,48 1 0 256.616702701 0 m N throtl schedule work. delay=51200 jiffies=4295111296Signed-off-by Jianpeng Ma <[email protected]>--- block/blk-throttle.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)block/blk-throttle.c +10
-5@@ -645,6 +645,15 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, }/* +* If privious slice expired,then start new slice. +* But counting bps and iops limit need privious slice info +* which ->slice_start. +*/ +if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) + &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) +if (wait) + *wait =0 ; +/* * If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. @@ -656,12 +665,8 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg,throtl_extend_slice (td, tg, rw, jiffies + throtl_slice); } -if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) - &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) { -if (wait) - *wait =0 ; +if (!(bps_wait || iops_wait))return 1 ; - }Quoted 3 lines
block/blk-throttle.c static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, · line 671
Quoted 1 line
block/blk-throttle.c +0
-0max_wait =max (bps_wait, iops_wait);Looks pretty sane to me. Vivek?
Hi Jens,
This fix will introduce other side affects. And that is when an group has been idle for few seconds and a new IO gets queued in, we will not start a new slice and allow dispatch equivalent of those idle seconds before we throttle the group. So keeping group idle will become an incentive.
I have attached a patch which might work better. majianpeng, can you please give it a try.
Having said that it is strange that workqueue thread is triggering so late. Looking at timestamp of traces attached.
We scheduled a delayed work. Following is trace.
8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984
Now a worker should execute after 51.2 seconds (delay=51200).
8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1
But worker executed at delay of 51.328 seconds (102.632 - 51.304). That is a delay of around 128 jiffies (51.328 - 51.2). That kind of seems odd. AFAIK, workers are fairly quick to execute after expiry. And it is because of this excessive delay that we think slice has expired.
May be it is something new. CCing Tejun, if he has seen anything like this and may be it is a known issue.
Even if it is a worker thread issue, I think applying below patch makes sense.
Thanks
Vivekblk-throttle: Do not start a new slice if IO is pending
It might happen that we queue an IO (because it overshot the rate) and then wait for certain jiffies to pass. We will set
slice_endaccordingly and wait for that time to elapse and worker thread will kick in, again evaluate whether group can dispatch or not. Now it might happen that current time is afterslice_endandtg_may_dispatch()will start a new slice.This will result in IO not being dispatched and be put back on wait again. And this will repeat, resulting in hang.
Do not start a new slice if an IO is pending in that group in the direction being queired. Instead extend the slice. New slice is supposed to start when a group has not been doing IO for some time and a new IO shows up. In that case we want do discard history and start a new slice.
Signed-off-by Vivek Goyal <[email protected]>---
block/blk-throttle.c| 8 +++++++-1 file changed, 7 insertions(+), 1 deletion(-)Index:
linux-2.6/block/blk-throttle.c
===================================================================block/blk-throttle.c +2
-0@@ -648,8 +648,14 @@ static bool tg_may_dispatch(struct throt* If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. +* +* Start a new slice only if there is no bio queued in that direction.
block/blk-throttle.c static bool tg_may_dispatch(struct throt · line 661
Quoted 10 lines
block/blk-throttle.c +5
-1+* That bio is waiting to be dispatched and slice needs to be +* extended. It might happen that bio waited to be dispatched but +* workqueue execution got little late it might restart a new slice +* instead of taking all the waited time into account. */ -if (throtl_slice_used (td, tg, rw)) +if (throtl_slice_used (td, tg, rw) && !tg->nr_queued[rw])throtl_start_new_slice (td, tg, rw);else {if (time_before (tg->slice_end[rw], jiffies + throtl_slice))
Hi vivek,
Your patch is ok.But i had a question:
What's condition tg->nr_queued[rw] = 0, but bio is not null?
Thanks! Jianpeng�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay� 蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��� �鹅�⒏?朓�
[maybe you received more, I'm very sorry for that]
Quoted 155 lines · Jens Axboe
Quoted 80 lines · majianpeng
Quoted 79 lines
In func
tg_may_dispatch,Quoted 2 lines
if (
throtl_slice_used(td, tg, rw))throtl_start_new_slice(td, tg, rw);...
Quoted 2 lines
if (
tg_with_in_bps_limit(td, tg, bio, &bps_wait)&& tg_with_in_iops_limit(td, tg, bio, &iops_wait)) {In funcs tg_with_in_(bps/iops)_limit, it used the
slice_startto count. So if privious slice expired, it start a new slice.This can cause hung task.The next steps can repeat this bug.
1:echo "8:48 10240" > blkio.throttle.write_bps_devic 2:dd if=/dev/zero of=/dev/sdd bs=1M count=1 oflag=directUsing the blktrace, the messages about throttle:
root@kernel:/mnt/programs# blktrace -d /dev/sdd -a notify -o -|blkparse -i - 8,48 1 0 0.000000000 0 m N throtl / [W] new slice start=4294854679 end=4294854779 jiffies=4294854679 8,48 1 0 0.000000966 0 m N throtl / [W] extend slice start=4294854679 end=4294905900 jiffies=4294854679 8,48 1 0 0.000002553 0 m N throtl / [W] bio. bdisp=0 sz=524288 bps=10240 iodisp=0 iops=4294967295 queued=0/0 8,48 1 0 0.000004788 0 m N throtl schedule work. delay=51200 jiffies=4294854679 8,48 1 0 51.304698681 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 51.304701979 0 m N throtl / [W] new slice start=4294905984 end=4294906084 jiffies=4294905984 8,48 1 0 51.304703329 0 m N throtl / [W] extend slice start=4294905984 end=4294957200 jiffies=4294905984 8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984 8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 102.632700544 0 m N throtl / [W] new slice start=4294957312 end=4294957412 jiffies=4294957312 8,48 1 0 102.632701922 0 m N throtl / [W] extend slice start=4294957312 end=4295008600 jiffies=4294957312 8,48 1 0 102.632704016 0 m N throtl schedule work. delay=51200 jiffies=4294957312 8,48 1 0 153.960696503 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 153.960699797 0 m N throtl / [W] new slice start=4295008640 end=4295008740 jiffies=4295008640 8,48 1 0 153.960701153 0 m N throtl / [W] extend slice start=4295008640 end=4295059900 jiffies=4295008640 8,48 1 0 153.960703218 0 m N throtl schedule work. delay=51200 jiffies=4295008640 8,48 1 0 205.288697067 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 205.288700268 0 m N throtl / [W] new slice start=4295059968 end=4295060068 jiffies=4295059968 8,48 1 0 205.288701630 0 m N throtl / [W] extend slice start=4295059968 end=4295111200 jiffies=4295059968 8,48 1 0 205.288703784 0 m N throtl schedule work. delay=51200 jiffies=4295059968 8,48 1 0 256.616696184 0 m N throtl dispatch nr_queued=1 read=0 write=1 8,48 1 0 256.616699266 0 m N throtl / [W] new slice start=4295111296 end=4295111396 jiffies=4295111296 8,48 1 0 256.616700574 0 m N throtl / [W] extend slice start=4295111296 end=4295162500 jiffies=4295111296 8,48 1 0 256.616702701 0 m N throtl schedule work. delay=51200 jiffies=4295111296Signed-off-by Jianpeng Ma <[email protected]>--- block/blk-throttle.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)block/blk-throttle.c +10
-5@@ -645,6 +645,15 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, }/* +* If privious slice expired,then start new slice. +* But counting bps and iops limit need privious slice info +* which ->slice_start. +*/ +if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) + &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) +if (wait) + *wait =0 ; +/* * If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. @@ -656,12 +665,8 @@ static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg,throtl_extend_slice (td, tg, rw, jiffies + throtl_slice); } -if (tg_with_in_bps_limit (td, tg, bio, &bps_wait) - &&tg_with_in_iops_limit (td, tg, bio, &iops_wait)) { -if (wait) - *wait =0 ; +if (!(bps_wait || iops_wait))return 1 ; - }Quoted 3 lines
block/blk-throttle.c static bool tg_may_dispatch(struct throtl_data *td, struct throtl_grp *tg, · line 671
Quoted 1 line
block/blk-throttle.c +0
-0max_wait =max (bps_wait, iops_wait);Looks pretty sane to me. Vivek?
Hi Jens,
This fix will introduce other side affects. And that is when an group has been idle for few seconds and a new IO gets queued in, we will not start a new slice and allow dispatch equivalent of those idle seconds before we throttle the group. So keeping group idle will become an incentive.
I have attached a patch which might work better. majianpeng, can you please give it a try.
Having said that it is strange that workqueue thread is triggering so late. Looking at timestamp of traces attached.
We scheduled a delayed work. Following is trace.
8,48 1 0 51.304705783 0 m N throtl schedule work. delay=51200 jiffies=4294905984
Now a worker should execute after 51.2 seconds (delay=51200).
8,48 1 0 102.632697082 0 m N throtl dispatch nr_queued=1 read=0 write=1
But worker executed at delay of 51.328 seconds (102.632 - 51.304). That is a delay of around 128 jiffies (51.328 - 51.2). That kind of seems odd. AFAIK, workers are fairly quick to execute after expiry. And it is because of this excessive delay that we think slice has expired.
May be it is something new. CCing Tejun, if he has seen anything like this and may be it is a known issue.
Even if it is a worker thread issue, I think applying below patch makes sense.
Thanks
Vivekblk-throttle: Do not start a new slice if IO is pending
It might happen that we queue an IO (because it overshot the rate) and then wait for certain jiffies to pass. We will set
slice_endaccordingly and wait for that time to elapse and worker thread will kick in, again evaluate whether group can dispatch or not. Now it might happen that current time is afterslice_endandtg_may_dispatch()will start a new slice.This will result in IO not being dispatched and be put back on wait again. And this will repeat, resulting in hang.
Do not start a new slice if an IO is pending in that group in the direction being queired. Instead extend the slice. New slice is supposed to start when a group has not been doing IO for some time and a new IO shows up. In that case we want do discard history and start a new slice.
Signed-off-by Vivek Goyal <[email protected]>---
block/blk-throttle.c| 8 +++++++-1 file changed, 7 insertions(+), 1 deletion(-)Index:
linux-2.6/block/blk-throttle.c
===================================================================block/blk-throttle.c +2
-0@@ -648,8 +648,14 @@ static bool tg_may_dispatch(struct throt* If previous slice expired, start a new one otherwise renew/extend * existing slice to make sure it is at least throtl_slice interval * long since now. +* +* Start a new slice only if there is no bio queued in that direction.
block/blk-throttle.c static bool tg_may_dispatch(struct throt · line 661
Quoted 10 lines
block/blk-throttle.c +5
-1+* That bio is waiting to be dispatched and slice needs to be +* extended. It might happen that bio waited to be dispatched but +* workqueue execution got little late it might restart a new slice +* instead of taking all the waited time into account. */ -if (throtl_slice_used (td, tg, rw)) +if (throtl_slice_used (td, tg, rw) && !tg->nr_queued[rw])throtl_start_new_slice (td, tg, rw);else {if (time_before (tg->slice_end[rw], jiffies + throtl_slice))
Hi vivek,
Your patch is ok.But i had a question:
What's condition tg->nr_queued[rw] = 0, but bio is not null?
Thanks! Jianpeng�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay� 蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��� �鹅�⒏?朓�