From: Jens Axboe <axboe@kernel.dk>
To: Josef Bacik <josef@toxicpanda.com>
Cc: linux-block@vger.kernel.org, kernel-team@fb.com,
akpm@linux-foundation.org, hannes@cmpxchg.org,
linux-kernel@vger.kernel.org, tj@kernel.org,
linux-fsdevel@vger.kernel.org, Josef Bacik <jbacik@fb.com>
Subject: Re: [PATCH 12/15] block: introduce blk-iolatency io controller
Date: Wed, 27 Jun 2018 13:24:55 -0600 [thread overview]
Message-ID: <784f0862-0441-5ed2-1beb-3effa82b3438@kernel.dk> (raw)
In-Reply-To: <20180627192046.ieqncfl6ioy37mof@destiny>
On 6/27/18 1:20 PM, Josef Bacik wrote:
> On Wed, Jun 27, 2018 at 01:06:31PM -0600, Jens Axboe wrote:
>> On 6/25/18 9:12 AM, Josef Bacik wrote:
>>> +static void __blkcg_iolatency_throttle(struct rq_qos *rqos,
>>> + struct iolatency_grp *iolat,
>>> + spinlock_t *lock, bool issue_as_root,
>>> + bool use_memdelay)
>>> + __releases(lock)
>>> + __acquires(lock)
>>> +{
>>> + struct rq_wait *rqw = &iolat->rq_wait;
>>> + unsigned use_delay = atomic_read(&lat_to_blkg(iolat)->use_delay);
>>> + DEFINE_WAIT(wait);
>>> + bool first_block = true;
>>> +
>>> + if (use_delay)
>>> + blkcg_schedule_throttle(rqos->q, use_memdelay);
>>> +
>>> + /*
>>> + * To avoid priority inversions we want to just take a slot if we are
>>> + * issuing as root. If we're being killed off there's no point in
>>> + * delaying things, we may have been killed by OOM so throttling may
>>> + * make recovery take even longer, so just let the IO's through so the
>>> + * task can go away.
>>> + */
>>> + if (issue_as_root || fatal_signal_pending(current)) {
>>> + atomic_inc(&rqw->inflight);
>>> + return;
>>> + }
>>> +
>>> + if (iolatency_may_queue(iolat, &wait, first_block))
>>> + return;
>>> +
>>> + do {
>>> + prepare_to_wait_exclusive(&rqw->wait, &wait,
>>> + TASK_UNINTERRUPTIBLE);
>>> +
>>> + iolatency_may_queue(iolat, &wait, first_block);
>>> + first_block = false;
>>> +
>>> + if (lock) {
>>> + spin_unlock_irq(lock);
>>> + io_schedule();
>>> + spin_lock_irq(lock);
>>> + } else {
>>> + io_schedule();
>>> + }
>>> + } while (1);
>>
>> So how does this wait loop ever exit?
>>
>
> Sigh, I cleaned this up from what we're using in production and did it poorly,
> I'll fix it up. Thanks,
Also may want to consider NOT using exclusive add if first_block == false, as
you'll end up at the tail of the waitqueue after sleeping and being denied.
This is similar to the wbt change I posted last week.
For may_queue(), your wq_has_sleeper() is also going to be always true
inside your loop, since you call it after doing the prepare_to_wait()
which adds you to the queue. That's why wbt does the list checks, but
it'd be nicer to have a wq_has_other_sleepers() for that. So your
first iolatency_may_queue() inside the loop will always be false.
--
Jens Axboe
next prev parent reply other threads:[~2018-06-27 19:24 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-25 15:12 [PATCH 0/15][V3] Introduce io.latency io controller for cgroups Josef Bacik
2018-06-25 15:12 ` [PATCH 01/15] block: add bi_blkg to the bio " Josef Bacik
2018-06-25 15:12 ` [PATCH 02/15] block: introduce bio_issue_as_root_blkg Josef Bacik
2018-06-25 15:12 ` [PATCH 03/15] blk-cgroup: allow controllers to output their own stats Josef Bacik
2018-06-25 15:12 ` [PATCH 04/15] blk: introduce REQ_SWAP Josef Bacik
2018-06-25 15:12 ` [PATCH 05/15] swap,blkcg: issue swap io with the appropriate context Josef Bacik
2018-06-25 15:12 ` [PATCH 06/15] blkcg: add generic throttling mechanism Josef Bacik
2018-06-25 15:12 ` [PATCH 07/15] memcontrol: schedule throttling if we are congested Josef Bacik
2018-06-25 15:12 ` [PATCH 08/15] blk-stat: export helpers for modifying blk_rq_stat Josef Bacik
2018-06-25 15:12 ` [PATCH 09/15] blk-rq-qos: refactor out common elements of blk-wbt Josef Bacik
2018-06-25 15:12 ` [PATCH 10/15] block: remove external dependency on wbt_flags Josef Bacik
2018-06-25 15:12 ` [PATCH 11/15] rq-qos: introduce dio_bio callback Josef Bacik
2018-06-25 15:12 ` [PATCH 12/15] block: introduce blk-iolatency io controller Josef Bacik
2018-06-27 19:06 ` Jens Axboe
2018-06-27 19:20 ` Josef Bacik
2018-06-27 19:24 ` Jens Axboe [this message]
2018-06-28 13:26 ` Josef Bacik
2018-06-28 15:35 ` Jens Axboe
2018-06-25 15:12 ` [PATCH 13/15] Documentation: add a doc for blk-iolatency Josef Bacik
2018-06-25 22:36 ` Randy Dunlap
2018-06-25 15:12 ` [PATCH 14/15] skip readahead if the cgroup is congested Josef Bacik
2018-06-25 15:12 ` [PATCH 15/15] block: use irq variant for blkcg->lock Josef Bacik
2018-06-27 19:06 ` [PATCH 12/15] block: introduce blk-iolatency io controller Jens Axboe
2018-06-27 23:53 ` Randy Dunlap
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=784f0862-0441-5ed2-1beb-3effa82b3438@kernel.dk \
--to=axboe@kernel.dk \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=jbacik@fb.com \
--cc=josef@toxicpanda.com \
--cc=kernel-team@fb.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@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: 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).