From: Tejun Heo <tj@kernel.org>
To: Tao Cui <cui.tao@linux.dev>
Cc: Josef Bacik <josef@toxicpanda.com>, Jens Axboe <axboe@kernel.dk>,
Christoph Hellwig <hch@infradead.org>,
cgroups@vger.kernel.org, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, cuitao@kylinos.cn, tj@kernel.org
Subject: Re: [PATCH v3 0/4] blk-iocost: charge flushes and zone appends
Date: Mon, 21 Sep 2026 08:41:16 -1000 [thread overview]
Message-ID: <8d777202a64f3a02f7d13d90ee97c4e7@kernel.org> (raw)
In-Reply-To: <20260921033453.1912971-1-cui.tao@linux.dev>
Hello, Tao.
On Mon, Sep 21, 2026 at 11:34:49AM +0800, Tao Cui wrote:
> While testing iocost's weight-based throttling under concurrent IO, we
> observed that a cgroup limited to 1% weight could issue an unbounded
> number of flushes without being throttled: an fsync loop produced ~510k
> flushes in 12s with cost.usage staying at zero the entire time. The
> device was monopolized while iocost reported no activity. Zone appends
> were in the same position: the builtin linear cost model defines
> coefficients only for READ and WRITE, so REQ_OP_ZONE_APPEND is priced
> at zero and excluded from the latency statistics as well.
Looks good to me. A few nits:
- 1/4: REQ_PREFLUSH and REQ_FUA are in REQ_NOMERGE_FLAGS, so !is_merge is
always true there. Can you drop the wrapper and the separate flush_cost
accumulator? Adding to cost before the empty bio check gives the same
result, and the FUA condition fits on one line.
- 1/4: The comment mostly restates the two ifs and the coefficient
formula. The FUA condition is the only non-obvious part. Can it be
trimmed to that?
- 1/4: Pending pre-flushes get coalesced into one device flush, so the
charge doesn't quite mirror what the block layer issues. It's a
per-request policy charge, which the description already says. The
other iops coefficients also have the page cost subtracted, so they
aren't translated the same way. There's also a short broken line in
that paragraph.
- 1/4: cgroup-v2.rst doesn't use REQ_* names anywhere else. Maybe
describe them as a write with a preceding cache flush and a FUA write?
The table row could follow its siblings too ("The maximum ... per
second").
For the series,
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
prev parent reply other threads:[~2026-09-21 18:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-21 3:34 [PATCH v3 0/4] blk-iocost: charge flushes and zone appends Tao Cui
2026-09-21 3:34 ` [PATCH v3 1/4] blk-iocost: add flush cost support with the flushiops model parameter Tao Cui
2026-09-21 3:34 ` [PATCH v3 2/4] blk-iocost: charge zone appends as writes Tao Cui
2026-09-21 3:34 ` [PATCH v3 3/4] blk-iocost: account zone append completions in latency stats Tao Cui
2026-09-21 3:34 ` [PATCH v3 4/4] blk-iocost: fix stale comment in ioc_rqos_throttle() Tao Cui
2026-09-21 18:41 ` Tejun Heo [this message]
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=8d777202a64f3a02f7d13d90ee97c4e7@kernel.org \
--to=tj@kernel.org \
--cc=axboe@kernel.dk \
--cc=cgroups@vger.kernel.org \
--cc=cui.tao@linux.dev \
--cc=cuitao@kylinos.cn \
--cc=hch@infradead.org \
--cc=josef@toxicpanda.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.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: link
Be 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.