All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Alakesh Haloi <alakeshh@amazon.com>
Cc: stable@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org,
	Xunlei Pang <xlpang@linux.alibaba.com>
Subject: Re: [PATCH] sched/fair: Fix bandwidth timer clock drift condition
Date: Fri, 18 Jan 2019 17:05:20 +0100	[thread overview]
Message-ID: <20190118160520.GD11503@kroah.com> (raw)
In-Reply-To: <20190116195202.GA89178@dev-dsk-alakeshh-2c-f8a3e6e0.us-west-2.amazon.com>

On Wed, Jan 16, 2019 at 07:52:08PM +0000, Alakesh Haloi wrote:
> [ Upstream commit 512ac999d2755d2b7109e996a76b6fb8b888631d ]
> 
> I noticed that cgroup task groups constantly get throttled even
> if they have low CPU usage, this causes some jitters on the response
> time to some of our business containers when enabling CPU quotas.
> 
> It's very simple to reproduce:
> 
>   mkdir /sys/fs/cgroup/cpu/test
>   cd /sys/fs/cgroup/cpu/test
>   echo 100000 > cpu.cfs_quota_us
>   echo $$ > tasks
> 
> then repeat:
> 
>   cat cpu.stat | grep nr_throttled  # nr_throttled will increase steadily
> 
> After some analysis, we found that cfs_rq::runtime_remaining will
> be cleared by expire_cfs_rq_runtime() due to two equal but stale
> "cfs_{b|q}->runtime_expires" after period timer is re-armed.
> 
> The current condition to judge clock drift in expire_cfs_rq_runtime()
> is wrong, the two runtime_expires are actually the same when clock
> drift happens, so this condtion can never hit. The orginal design was
> correctly done by this commit:
> 
>   a9cf55b28610 ("sched: Expire invalid runtime")
> 
> ... but was changed to be the current implementation due to its locking bug.
> 
> This patch introduces another way, it adds a new field in both structures
> cfs_rq and cfs_bandwidth to record the expiration update sequence, and
> uses them to figure out if clock drift happens (true if they are equal).
> 
> Signed-off-by: Xunlei Pang <xlpang@linux.alibaba.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> [alakeshh: backport: Fixed merge conflicts:
>  - sched.h: Fix the indentation and order in which the variables are
>    declared to match with coding style of the existing code in 4.14
>    Struct members of same type were declared in separate lines in
>    upstream patch which has been changed back to having multiple
>    members of same type in the same line.
>    e.g. int a; int b; ->  int a, b; ]

Now queued up, thanks!

greg k-h

      reply	other threads:[~2019-01-18 16:05 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-16 19:52 [PATCH] sched/fair: Fix bandwidth timer clock drift condition Alakesh Haloi
2019-01-18 16:05 ` Greg KH [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=20190118160520.GD11503@kroah.com \
    --to=greg@kroah.com \
    --cc=alakeshh@amazon.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=xlpang@linux.alibaba.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 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.