Re:Re: cgroup: Fix split bio been throttled more than once

[aaronlee0817 <aaronlee0817@163.com>]

[-- Attachment #1: Type: text/plain, Size: 2314 bytes --]

>At 2016-07-06 22:09:19, "Tejun Heo" <tj@kernel.org> wrote:
>Hello, Ming.
>
>On Wed, Jul 06, 2016 at 09:10:00AM +0800, Ming Lei wrote:
>> > Then we did some research and find that in kernel version 4.3 brought in
>> > blk_queue_split() function to split the big size bio into several parts,
>> > and some of them are calling the generic_make_request() again, this result
>> > the bio been throttled more than once. so the actual bio sent to device is
>> > less than we expected.
>> 
>> Except for blk_queue_split(), there are other(stacked) drivers which call
>> generic_make_request() too, such as drbd, dm, md and bcache.
>
>So, blk-throtl already uses REQ_THROTTLED to avoid throttling the same
>bio multiple times.  The problem seems that the flag isn't maintained
>through clone.

Yea， REQ_THROTTLED is used to avoid throttling the same bio multiple times, 
but cannot avoid counting the same bio multiple times, this makes the counts of
dispatched bytes no real.
Before bringing in splitting, it works well.But now, this cannot cover all  scenarios.
For example:


generic_make_request()
blk_throtl_bio
    bool throttled = false
    throtl_charge_bio()      count the bytes, set REQ_THROTTLED
    goto out
    if(!throttled)
        clean REQ_THROTTLED       
    return 


Before return, REQ_THROTTLED will be cleaned.
While splitting will copy bio->rw, but REQ_THROTTLED has been cleaned.
So throttling function will count the bytes again. In this scenario  cannot 
avoid counting the same bio multiple times.  


>
>> >
>> > We have checked the newest kernel of 4.7-rc5, this problem is still exist.
>> >
>> > Based on this kind of situation, we propose a fix solution to add a flag bit
>> > in bio to let the splited bio bypass the blk_queue_split(). Below is the patch
>> > we used to fix this problem.
>> 
>> The splitted bio is just a fast-cloned bio(except for discard bio) and not very
>> special compared with other fast-cloned bio, which is quite common used.
>> 
>> So I guess what you need is to bypass BIO_CLONED bio for this purpose
>> since all fast-cloned bio shares the same bvec table of the source bio.
>
>Depending on how a device handles a bio, that could allow bios to
>bypass throttling entirely, no?  Wouldn't adding REQ_THROTTLED to
>REQ_CLONE_MASK work?
>
>Thanks.
>
>-- 
>tejun

[-- Attachment #2: Type: text/html, Size: 2972 bytes --]