lkml.org 
[lkml]   [2022]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v7 1/9] blk-throttle: fix that io throttle can only work for single bio
From
Date
Hi, Tejun!

在 2022/08/18 1:50, Tejun Heo 写道:
> Hello,
>
> On Wed, Aug 17, 2022 at 09:13:38AM +0800, Yu Kuai wrote:
>>> So, as a fix for the immediate problem, I guess this might do but this feels
>>> really fragile. How can we be certain that re-entering only happens because
>>> of splitting? What if future core development changes that? It seems to be
>>> solving the problem in the wrong place. Shouldn't we flag the bio indicating
>>> that it's split when we're splitting the bio so that we only limit them for
>>> iops in the first place?
>>
>> Splited bio is tracked in __bio_clone:
>
> As the word is used in commit messages and comments, the past perfect form
> of the verb "split" is "split". It looks like "splitted" is used in rare
> cases but dictionary says it's an archaic form.

Ok, thanks for pointing it out, I'll change that in next iteration.
>
>> if (bio_flagged(bio_src, BIO_THROTTLED))
>> bio_set_flag(bio, BIO_THROTTLED);
>>
>> And currenty, the iops limit and bps limit are treated differently,
>> however there are only one flag 'BIO_THROTTLED' and they can't be
>> distinguished.
>>
>> Perhaps I can use two flags, for example BIO_IOPS_THROTTLED and
>> BIO_BPS_THROTTLED, this way only iops limit can be handled and bps
>> limit can be skipped for splited bio.
>>
>> What do you think?
>
> I think the code would be a lot more intuitive and less fragile if we used
> two flags but the bits in the bi_flags field are a scarce resource
> unfortunately. Even then, I think the right thing to do here is using two
> flags.

Yes, the field 'bio->bi_flags' is unsigned short, and there are only two
bits left. I'll use the new sulution which will acquire a new bit.

Thanks,
Kuai
>
> Thanks.
>

\
 
 \ /
  Last update: 2022-08-18 03:24    [W:0.063 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site