Hi,
在 2022/08/18 9:23, Yu Kuai 写道:
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);
While implementing the new method, I found that there seems to be a
misunderstanding here, the code seems to try to add flag to split bio
so that it won't be throttled again for bps limit, however:
1) for blk throttle, split bio is issued directly and will never be
throttled again, while orignal bio will go through throttle path again.
2) if cloned bio is directed to a new disk, the flag is cleared anyway.
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.
.
