Re: [PATCH] blk-throttle: fix repeat limit on bio with BIO_BPS_THROTTLED

[周泰宇 <zhoutaiyu@kuaishou.com>]

Hi, kuai

Thank you so much for your feedback.

> The ponit here is that you break the rules about FIFO, blk-throttle
> only judge the bio from head if it's within limit. Current code to judge
> if tg iops reaches limit on the condition that no bio is throttled. And
> throtl time is always caculated by first throttled bio. But this patch
> will ignore throttled bio case, and that's why I said IO will not be
> throttled by iops limist anymore. You can test this with bps limit
> disabled.

Sorry, I don't get it.  Yes, I know this patch will break the FIFO rule.
But I didn't find any problems after letting split bio break this rule. 
I think the split bio should be ignored if it does not
make the tg out of iops limit and be judged in tg_within_iops_limit().

In the patch, split bio within iops limit  will be dispatched directly.
However, if the split bio makes a tg out of IOPS limit, the split bio will be throttled 
and be added to the tg's sq->queue[rw] by calling throtl_add_bio_tg().

There exists two cases after the split bio is judged to be throttled.

1) The tg->sq->queue is empty. In this case, the split bio will be the first queued bio.  The tg's 
latest dispatch time is calculated according to the split bio by calling tg_update_disptime(), and 
the dispatch timer is waked up by calling throtl_schedule_next_dispatch().

2) The tg->sq->queue is not empty. In this case, the tg's dispatch time has been updated
according to the first queued bio and the dispatch timer is also waked up.
So, blk-throttle just adds the split bio to the tail of the sq->queue[rw] which acts like
the current code (split bio is throttled because sq->nr_queue[rw] is not zero).


I have tested this patch by only setting iops limit as you advised, with following scripts

#!/bin/bash

CNUM=30
mkdir -p /sys/fs/cgroup/blkio/test0
#echo "8:16 10485760" > /sys/fs/cgroup/blkio/test0/blkio.throttle.write_bps_device
echo "8:16 50" > /sys/fs/cgroup/blkio/test0/blkio.throttle.write_iops_device

for ((i=0;i<$CNUM;i++));do
        fio -rw=write -direct=1 -bs=4M -iodepth=8 -size=200M -numjobs=1 -time_based=1 -runtime=110  -name=testt_$i -filename=testf_$i  > /dev/null &
        echo $! > /sys/fs/cgroup/blkio/test0/tasks
done

I monitor the w/s on iostat and the change of /sys/fs/cgroup/blkio/test0/blkio.throttle.io_serviced every second.
It seems work perfectly (around 50).  

I also see the split bio be throttled through the trace_pipe

    kworker/19:2-1603    [019] d..1.   583.556012:   8,16   X  WS 977153216 / 977155776 [kworker/19:2]
    kworker/19:2-1603    [019] d..1.   583.556012:   8,16  1,0  m   N throtl [W] bio. bdisp=0 sz=1572864 bps=18446744073709551615 iodisp=5 iops=50 queued=0/28
    ......
    kworker/19:2-1603    [019] d..1.   583.556017:   8,16   X  WS 1870472 / 1873032 [kworker/19:2]
    kworker/19:2-1603    [019] d..1.   583.556018:   8,16  1,0  m   N throtl [W] bio. bdisp=0 sz=1572864 bps=18446744073709551615 iodisp=5 iops=50 queued=0/29

Would you mind give me more hints to find out the problems of my patch or try my patch? 

Sincerely,
Taiyu Zhou