linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: brookxu <brookxu.cn@gmail.com>
Cc: paolo.valente@linaro.org, axboe@kernel.dk,
	linux-block@vger.kernel.org, cgroups@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 00/14] bfq: introduce bfq.ioprio for cgroup
Date: Sun, 4 Apr 2021 12:09:29 -0400	[thread overview]
Message-ID: <YGnkuWYKeK7C8/Za@mtj.duckdns.org> (raw)
In-Reply-To: <cover.1616649216.git.brookxu@tencent.com>

Hello,

On Thu, Mar 25, 2021 at 02:57:44PM +0800, brookxu wrote:
> INTERFACE:
> 
> The bfq.ioprio interface now is available for cgroup v1 and cgroup
> v2. Users can configure the ioprio for cgroup through this
> interface, as shown below:
> 
> echo "1 2"> blkio.bfq.ioprio
> 
> The above two values respectively represent the values of ioprio
> class and ioprio for cgroup.
> 
> EXPERIMENT:
> 
> The test process is as follows:
> # prepare data disk
> mount /dev/sdb /data1
> 
> # prepare IO scheduler
> echo bfq > /sys/block/sdb/queue/scheduler
> echo 0 > /sys/block/sdb/queue/iosched/low_latency
> echo 1 > /sys/block/sdb/queue/iosched/better_fairness
> 
> It is worth noting here that nr_requests limits the number of
> requests, and it does not perceive priority. If nr_requests is
> too small, it may cause a serious priority inversion problem.
> Therefore, we can increase the size of nr_requests based on
> the actual situation.
> 
> # create cgroup v1 hierarchy
> cd /sys/fs/cgroup/blkio
> mkdir rt be0 be1 be2 idle
> 
> # prepare cgroup
> echo "1 0" > rt/blkio.bfq.ioprio
> echo "2 0" > be0/blkio.bfq.ioprio
> echo "2 4" > be1/blkio.bfq.ioprio
> echo "2 7" > be2/blkio.bfq.ioprio
> echo "3 0" > idle/blkio.bfq.ioprio

Here are some concerns:

* The main benefit of bfq compared to cfq at least was that the behavior
  model was defined in a clearer way. It was possible to describe what the
  control model was in a way which makes semantic sense. The main problem I
  see with this proposal is that it's an interface which grew out of the
  current implementation specifics and I'm having a hard time understanding
  what the end results should be with different configuration combinations.

* While this might work around some scheduling latency issues but I have a
  hard time imagining it being able to address actual QoS issues. e.g. on a
  lot of SSDs, without absolute throttling, device side latencies can spike
  by multiple orders of magnitude and no prioritization on the scheduler
  side is gonna help once such state is reached. Here, there's no robust
  mechanisms or measurement/control units defined to address that. In fact,
  the above direction to increase nr_requests limit will make priority
  inversions on the device and post-elevator side way more likely and
  severe.

So, maybe it helps with specific scenarios on some hardware, but given the
ad-hoc nature, I don't think it justifies all the extra interface additions.
My suggestion would be slimming it down to bare essentials and making the
user interface part as minimal as possible.

Thanks.

-- 
tejun

  parent reply	other threads:[~2021-04-04 16:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-25  6:57 [PATCH v3 00/14] bfq: introduce bfq.ioprio for cgroup brookxu
2021-03-25  6:57 ` [PATCH v3 01/14] bfq: introduce bfq_entity_to_bfqg helper method brookxu
2021-03-25  6:57 ` [PATCH v3 02/14] bfq: convert the type of bfq_group.bfqd to bfq_data* brookxu
2021-03-25  6:57 ` [PATCH v3 03/14] bfq: introduce bfq.ioprio for cgroup brookxu
2021-03-25  6:57 ` [PATCH v3 04/14] bfq: introduce bfq_ioprio_class to get ioprio class brookxu
2021-03-25  6:57 ` [PATCH v3 05/14] bfq: limit the IO depth of CLASS_IDLE to 1 brookxu
2021-03-25  6:57 ` [PATCH v3 06/14] bfq: keep the minimun bandwidth for CLASS_BE brookxu
2021-03-25  6:57 ` [PATCH v3 07/14] bfq: introduce better_fairness for container scene brookxu
2021-03-25  6:57 ` [PATCH v3 08/14] bfq: introduce prio_expire flag for bfq_queue brookxu
2021-03-25  6:57 ` [PATCH v3 09/14] bfq: expire in_serv_queue for prio_expire under better_fairness brookxu
2021-03-25  6:57 ` [PATCH v3 10/14] bfq: optimize IO injection " brookxu
2021-03-25  6:57 ` [PATCH v3 11/14] bfq: disable idle for prio_expire " brookxu
2021-03-25  6:57 ` [PATCH v3 12/14] bfq: disable merging between different groups " brookxu
2021-03-25  6:57 ` [PATCH v3 13/14] bfq: remove unnecessary initialization logic brookxu
2021-03-25  6:57 ` [PATCH v3 14/14] bfq: optimize the calculation of bfq_weight_to_ioprio() brookxu
2021-04-04 16:09 ` Tejun Heo [this message]
2021-04-06  7:31   ` [PATCH v3 00/14] bfq: introduce bfq.ioprio for cgroup brookxu

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=YGnkuWYKeK7C8/Za@mtj.duckdns.org \
    --to=tj@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=brookxu.cn@gmail.com \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paolo.valente@linaro.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).