Messages in this thread | | | Subject | Re: [PATCH v7 1/9] blk-throttle: fix that io throttle can only work for single bio | From | Yu Kuai <> | Date | Thu, 18 Aug 2022 09:23:39 +0800 |
| |
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. >
| |