From: brookxu <brookxu.cn@gmail.com> To: Tejun Heo <tj@kernel.org> Cc: axboe@kernel.dk, cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] blk-throtl: optimize IOPS throttle for large IO scenarios Date: Tue, 27 Jul 2021 11:06:18 +0800 [thread overview] Message-ID: <34a6f4b5-9055-e519-5693-068f8dcb169c@gmail.com> (raw) In-Reply-To: <YP8tPwkJNMAcjDqk@mtj.duckdns.org> Tejun Heo wrote on 2021/7/27 5:46: > Hello, > > On Tue, Jul 20, 2021 at 12:35:54AM +0800, brookxu wrote: >> In order to avoid code duplication and IOPS stability problems caused by estimating >> the equivalent number of IOs, and to avoid potential deadlock problems caused by >> synchronization through queue_lock. I tried to count the number of splited IOs in >> the current window through two atomic counters. Add the value of the atomic variable >> when calculating io_disp[rw], which can also avoid the problem of inaccurate IOPS in >> large IO scenarios. How do you think of this approach? Thanks for your time. > > I guess it's okay but am still not a big fan of adding another hook. This is > primarily because blk-throtl is sitting too early in the stack - e.g. rq_qos > is doing the same thing but sits after the split path - and it's a bit nasty > to add an additional hook for it. > > Do you think it can be an option to relocate the blk-throtl hooks to the > same spots as rq-qos or, even better, make it use rq-qos? Make blk-throttle use rq-qos may be more elegant. But I found that there may be at least one problem that is difficult to solve. blk-throttle supports separate throttle for read and write IOs, which means that we cannot suspend tasks during throttle, but rq-qos throttle IOs by suspending tasks. We may be able to relocate the blk-throttle hooks to the rq-qos hooks. Since we may not be able to replace the throttle hook, in this case, if we register a rq-qos to the system, part of the blk-throttle hooks is in rq-qos and part hooks not, which feels a bit confusing. In addition, we may need to implement more hooks, such as IO merge hook. Thanks for you time. > Thanks. >
WARNING: multiple messages have this Message-ID (diff)
From: brookxu <brookxu.cn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Cc: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH] blk-throtl: optimize IOPS throttle for large IO scenarios Date: Tue, 27 Jul 2021 11:06:18 +0800 [thread overview] Message-ID: <34a6f4b5-9055-e519-5693-068f8dcb169c@gmail.com> (raw) In-Reply-To: <YP8tPwkJNMAcjDqk-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org> Tejun Heo wrote on 2021/7/27 5:46: > Hello, > > On Tue, Jul 20, 2021 at 12:35:54AM +0800, brookxu wrote: >> In order to avoid code duplication and IOPS stability problems caused by estimating >> the equivalent number of IOs, and to avoid potential deadlock problems caused by >> synchronization through queue_lock. I tried to count the number of splited IOs in >> the current window through two atomic counters. Add the value of the atomic variable >> when calculating io_disp[rw], which can also avoid the problem of inaccurate IOPS in >> large IO scenarios. How do you think of this approach? Thanks for your time. > > I guess it's okay but am still not a big fan of adding another hook. This is > primarily because blk-throtl is sitting too early in the stack - e.g. rq_qos > is doing the same thing but sits after the split path - and it's a bit nasty > to add an additional hook for it. > > Do you think it can be an option to relocate the blk-throtl hooks to the > same spots as rq-qos or, even better, make it use rq-qos? Make blk-throttle use rq-qos may be more elegant. But I found that there may be at least one problem that is difficult to solve. blk-throttle supports separate throttle for read and write IOs, which means that we cannot suspend tasks during throttle, but rq-qos throttle IOs by suspending tasks. We may be able to relocate the blk-throttle hooks to the rq-qos hooks. Since we may not be able to replace the throttle hook, in this case, if we register a rq-qos to the system, part of the blk-throttle hooks is in rq-qos and part hooks not, which feels a bit confusing. In addition, we may need to implement more hooks, such as IO merge hook. Thanks for you time. > Thanks. >
next prev parent reply other threads:[~2021-07-27 3:06 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-16 6:22 [PATCH] blk-throtl: optimize IOPS throttle for large IO scenarios brookxu 2021-07-16 16:09 ` Tejun Heo 2021-07-16 16:09 ` Tejun Heo 2021-07-16 23:07 ` brookxu 2021-07-19 16:35 ` brookxu 2021-07-19 16:35 ` brookxu 2021-07-26 21:46 ` Tejun Heo 2021-07-26 21:46 ` Tejun Heo 2021-07-27 3:06 ` brookxu [this message] 2021-07-27 3:06 ` brookxu 2021-07-27 16:21 ` Tejun Heo 2021-07-27 16:21 ` Tejun Heo 2021-07-28 2:33 ` brookxu 2021-07-28 2:33 ` brookxu 2021-07-28 7:48 ` Tejun Heo 2021-07-28 7:48 ` Tejun Heo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=34a6f4b5-9055-e519-5693-068f8dcb169c@gmail.com \ --to=brookxu.cn@gmail.com \ --cc=axboe@kernel.dk \ --cc=cgroups@vger.kernel.org \ --cc=linux-block@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=tj@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.