public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Tejun Heo <tj@kernel.org>
Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com, cgroups@vger.kernel.org,
	Vlad Dmitriev <vvd@fb.com>
Subject: Re: [block/for-5.7] iocost: protect iocg->abs_vdebt with iocg->waitq.lock
Date: Tue, 5 May 2020 09:23:35 -0600	[thread overview]
Message-ID: <e4377265-1605-077b-9415-183c8efe3da5@kernel.dk> (raw)
In-Reply-To: <20200504232754.GB3209250@mtj.thefacebook.com>

On 5/4/20 5:27 PM, Tejun Heo wrote:
> Subject: iocost: protect iocg->abs_vdebt with iocg->waitq.lock
> 
> abs_vdebt is an atomic_64 which tracks how much over budget a given cgroup
> is and controls the activation of use_delay mechanism. Once a cgroup goes
> over budget from forced IOs, it has to pay it back with its future budget.
> The progress guarantee on debt paying comes from the iocg being active -
> active iocgs are processed by the periodic timer, which ensures that as time
> passes the debts dissipate and the iocg returns to normal operation.
> 
> However, both iocg activation and vdebt handling are asynchronous and a
> sequence like the following may happen.
> 
> 1. The iocg is in the process of being deactivated by the periodic timer.
> 
> 2. A bio enters ioc_rqos_throttle(), calls iocg_activate() which returns
>    without anything because it still sees that the iocg is already active.
> 
> 3. The iocg is deactivated.
> 
> 4. The bio from #2 is over budget but needs to be forced. It increases
>    abs_vdebt and goes over the threshold and enables use_delay.
> 
> 5. IO control is enabled for the iocg's subtree and now IOs are attributed
>    to the descendant cgroups and the iocg itself no longer issues IOs.
> 
> This leaves the iocg with stuck abs_vdebt - it has debt but inactive and no
> further IOs which can activate it. This can end up unduly punishing all the
> descendants cgroups.
> 
> The usual throttling path has the same issue - the iocg must be active while
> throttled to ensure that future event will wake it up - and solves the
> problem by synchronizing the throttling path with a spinlock. abs_vdebt
> handling is another form of overage handling and shares a lot of
> characteristics including the fact that it isn't in the hottest path.
> 
> This patch fixes the above and other possible races by strictly
> synchronizing abs_vdebt and use_delay handling with iocg->waitq.lock.

Applied, thanks.

-- 
Jens Axboe


      reply	other threads:[~2020-05-05 15:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-04 23:27 [block/for-5.7] iocost: protect iocg->abs_vdebt with iocg->waitq.lock Tejun Heo
2020-05-05 15:23 ` Jens Axboe [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=e4377265-1605-077b-9415-183c8efe3da5@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=cgroups@vger.kernel.org \
    --cc=kernel-team@fb.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=vvd@fb.com \
    /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