lkml.org 
[lkml]   [2022]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v7 1/9] blk-throttle: fix that io throttle can only work for single bio
    Date
    From: Yu Kuai <yukuai3@huawei.com>

    Test scripts:
    cd /sys/fs/cgroup/blkio/
    echo "8:0 1024" > blkio.throttle.write_bps_device
    echo $$ > cgroup.procs
    dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct &
    dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct &

    Test result:
    10240 bytes (10 kB, 10 KiB) copied, 10.0134 s, 1.0 kB/s
    10240 bytes (10 kB, 10 KiB) copied, 10.0135 s, 1.0 kB/s

    The problem is that the second bio is finished after 10s instead of 20s.

    Root cause:
    1) second bio will be flaged:

    __blk_throtl_bio
    while (true) {
    ...
    if (sq->nr_queued[rw]) -> some bio is throttled already
    break
    };
    bio_set_flag(bio, BIO_THROTTLED); -> flag the bio

    2) flaged bio will be dispatched without waiting:

    throtl_dispatch_tg
    tg_may_dispatch
    tg_with_in_bps_limit
    if (bps_limit == U64_MAX || bio_flagged(bio, BIO_THROTTLED))
    *wait = 0; -> wait time is zero
    return true;

    commit 9f5ede3c01f9 ("block: throttle split bio in case of iops limit")
    support to count splited bios for iops limit, thus it adds flaged bio
    checking in tg_with_in_bps_limit() so that splited bios will only count
    once for bps limit, however, it introduce a new problem that io throttle
    won't work if multiple bios are throttled.

    In order to fix the problem, at first, don't skip flaged bio in
    tg_with_in_bps_limit(), however, this will break that splited bios should
    only count once for bps limit. And this patch tries to avoid
    over-accounting by decrementing it first in __blk_throtl_bio(), and
    then counting it again while dispatching it.

    Fixes: 9f5ede3c01f9 ("block: throttle split bio in case of iops limit")
    Signed-off-by: Yu Kuai <yukuai3@huawei.com>
    Reviewed-by: Ming Lei <ming.lei@redhat.com>
    ---
    block/blk-throttle.c | 26 ++++++++++++++++++++------
    1 file changed, 20 insertions(+), 6 deletions(-)

    diff --git a/block/blk-throttle.c b/block/blk-throttle.c
    index 9f5fe62afff9..2957e2c643f4 100644
    --- a/block/blk-throttle.c
    +++ b/block/blk-throttle.c
    @@ -811,7 +811,7 @@ static bool tg_with_in_bps_limit(struct throtl_grp *tg, struct bio *bio,
    unsigned int bio_size = throtl_bio_data_size(bio);

    /* no need to throttle if this bio's bytes have been accounted */
    - if (bps_limit == U64_MAX || bio_flagged(bio, BIO_THROTTLED)) {
    + if (bps_limit == U64_MAX) {
    if (wait)
    *wait = 0;
    return true;
    @@ -921,11 +921,8 @@ static void throtl_charge_bio(struct throtl_grp *tg, struct bio *bio)
    unsigned int bio_size = throtl_bio_data_size(bio);

    /* Charge the bio to the group */
    - if (!bio_flagged(bio, BIO_THROTTLED)) {
    - tg->bytes_disp[rw] += bio_size;
    - tg->last_bytes_disp[rw] += bio_size;
    - }
    -
    + tg->bytes_disp[rw] += bio_size;
    + tg->last_bytes_disp[rw] += bio_size;
    tg->io_disp[rw]++;
    tg->last_io_disp[rw]++;

    @@ -2121,6 +2118,23 @@ bool __blk_throtl_bio(struct bio *bio)
    tg->last_low_overflow_time[rw] = jiffies;
    throtl_downgrade_check(tg);
    throtl_upgrade_check(tg);
    +
    + /*
    + * Splited bios can be re-entered because iops limit should be
    + * counted again, however, bps limit should not. Since bps limit
    + * will be counted again while dispatching it, compensate the
    + * over-accounting here. Noted that compensation can fail if
    + * new slice is started.
    + */
    + if (bio_flagged(bio, BIO_THROTTLED)) {
    + unsigned int bio_size = throtl_bio_data_size(bio);
    +
    + if (tg->bytes_disp[rw] >= bio_size)
    + tg->bytes_disp[rw] -= bio_size;
    + if (tg->last_bytes_disp[rw] >= bio_size)
    + tg->last_bytes_disp[rw] -= bio_size;
    + }
    +
    /* throtl is FIFO - if bios are already queued, should queue */
    if (sq->nr_queued[rw])
    break;
    --
    2.31.1
    \
     
     \ /
      Last update: 2022-08-02 15:54    [W:4.637 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site