Hello, On Tue, Apr 16, 2024 at 10:17:29PM +0800, Yu Kuai wrote: > > > This is a half-way conversion, right? You're adding a dedicated hook to > > > rq_qos and none of the other hooks can be used by blk-throtl. Even > > > the name, > > Actually, rq_qos_exit() is used as well for destroy blk-throtl. I see. > > > rq_qos_throttle_bio(), becomes a misnomer. I'm not really sure this makes > > > things better or worse. It makes certain things a bit cleaner but other > > > things nastier. I don't know. > > > > Yes, the final goal is making all blk-cgroup policies modular, and this > > patch use rq-qos to prevent exposing blk-throtle to block layer, like > > other policies. > > After thinking this a bit more, I still think probably rq_qos is a > better choice, and there is something that I want to discuss. > > There are two different direction, first is swith blk-throttle to > rq_qos_throttle() as well, which is called for each rq: > > 1) For, rq-based device, why blk-throtl must throttle before > rq_qos_throttle()? And blk-throtl have to handle the bio split case > seperately. And it looks like blk-throttle can switch to use > rq_qos_throttle() with the benefit that io split does't need > special handling. > > 2) blk-throtl treats split IO as additional iops, while it ignores > merge IO, this looks wrong to me. If multiple bio merged into one > request, iostat will see just one IO. And after switching to rq_qos, > bio merge case can be handled easily as well. If we could properly convert blk-throtl to rq-qos, that'd be great. The only problem is that there probably are users who are using blk-throtl with bio-based drivers because blk-throtl has supported that for a very long time, so we're in a bit of bind for blk-throtl. > Another is still add a rq_qos_throttle_bio(perhaps another name?), and > meanwhile iocost can benefit from this new helper as well. Because > iocost really is based on bio, currently it must handle the io merge > case by debt. I really don't think adding a different operation mode to rq-qos or iocost is a good idea. I'd much rather keep blk-throtl where it is and come up with a better replacement (e.g. iocost based .max throttling) in the future. Thanks. -- tejun
