mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: John Stultz <john.stultz@linaro.org>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Dmitry Shmidt <dimitrysh@google.com>,
	Rom Lemarchand <romlem@google.com>,
	Colin Cross <ccross@google.com>, Todd Kjos <tkjos@google.com>,
	Oleg Nesterov <oleg@redhat.com>
Subject: Re: Severe performance regression w/ 4.4+ on Android due to cgroup locking changes
Date: Wed, 13 Jul 2016 14:21:02 -0400	[thread overview]
Message-ID: <20160713182102.GJ4065@mtj.duckdns.org> (raw)
In-Reply-To: <CALAqxLV2H06M2AxP2kK7pGHjLgQAaGdHB9HJEGZjH_Rouy4WhQ@mail.gmail.com>

(cc'ing Oleg)

Hello,

On Tue, Jul 12, 2016 at 05:00:04PM -0700, John Stultz wrote:
>   So Dmitry Shmidt recently noticed that with 4.4 based systems we're
> seeing quite a bit of performance overhead from
> __cgroup_procs_write().
> 
> With 4.4 tree as it stands, we're seeing __cgroup_procs_write() quite
> often take 10s of miliseconds to execute (with max times up in the
> 80ms range).

Yikes, that's pretty high.  Does this happen only while the system is
generally busy or regardless of overall load?

> While with 4.1 it was quite often in the single usec range, and max
> time values still in in sub-milisecond range.
> 
> The majority of these performance regressions seem to come from the
> locking changes in:
> 
> 3014dde762f6 ("cgroup: simplify threadgroup locking")
> and
> 1ed1328792ff  ("sched, cgroup: replace signal_struct->group_rwsem with
> a global percpu_rwsem")
> 
> Dmitry has found that by reverting these two changes (which don't
> revert easiliy), we can get back down to tens 10-100 usec range for
> most calls, with max values occasionally spiking to ~18ms.
> 
> Those two commits do talk about performance regressions, that were
> supposedly alleviated by percpu_rwsem changes, but I'm not sure we are
> seeing this.
> 
> In 1ed1328792ff, the commit talks about the write path being a fairly
> cold path, but with Android I worry this may not actually be the case,
> as Android uses cpuset cgroups to group tasks into foreground and
> background tasks, but this means when switching applications, tasks
> are migrated between cgroups. Putting an additional 80 milisecond
> delay on this adds potentially visible latencies on task switching.

Switching between foreground and background isn't a hot path.  It's
human initiated operations after all.  It taking 80 msecs sure is
problematic but I'm skeptical that this is from actual contention
given that the only reader side holders are fork and exit paths.

> Reverting those two changes in the Android common.git tree doesn't
> feel like a good long term solution here, so I was wondering if you
> had any thoughts on how to further reduce the performance regression
> here?

One interesting thing to try would be replacing it with a regular
non-percpu rwsem and see how it behaves.  That should easily tell us
whether this is from actual contention or artifacts from percpu_rwsem
implementation.

Thanks.

-- 
tejun

  parent reply	other threads:[~2016-07-13 18:21 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-13  0:00 John Stultz
2016-07-13  8:21 ` Peter Zijlstra
2016-07-13 14:42   ` Paul E. McKenney
2016-07-13 18:13     ` Dmitry Shmidt
2016-07-13 18:32       ` Paul E. McKenney
2016-07-13 18:21 ` Tejun Heo [this message]
2016-07-13 18:33   ` Tejun Heo
2016-07-13 20:13     ` John Stultz
2016-07-13 20:18       ` Tejun Heo
2016-07-13 20:26         ` Peter Zijlstra
2016-07-13 20:39           ` Tejun Heo
2016-07-13 20:51             ` Peter Zijlstra
2016-07-13 21:01               ` Tejun Heo
2016-07-13 21:03               ` Paul E. McKenney
2016-07-13 21:05                 ` Tejun Heo
2016-07-13 21:18                   ` Paul E. McKenney
2016-07-13 21:42                     ` Paul E. McKenney
2016-07-13 21:46                       ` John Stultz
2016-07-13 22:17                         ` Paul E. McKenney
2016-07-13 22:39                           ` John Stultz
2016-07-13 23:02                             ` Paul E. McKenney
2016-07-13 23:04                               ` Paul E. McKenney
2016-07-14 11:35                                 ` Tejun Heo
2016-07-14 12:04                                   ` Peter Zijlstra
2016-07-14 12:08                                     ` Tejun Heo
2016-07-14 12:20                                       ` Peter Zijlstra
2016-07-14 15:07                                         ` Tejun Heo
2016-07-14 15:24                                           ` Tejun Heo
2016-07-14 16:32                                           ` Peter Zijlstra
2016-07-14 17:34                                             ` Oleg Nesterov
2016-07-14 16:54                               ` John Stultz
2016-07-13 22:25                       ` John Stultz
2016-07-13 22:01                     ` Tejun Heo
2016-07-13 22:33                       ` Paul E. McKenney
2016-07-14  6:49                       ` Peter Zijlstra
2016-07-14 11:20                         ` Tejun Heo
2016-07-14 12:11                           ` Peter Zijlstra
2016-07-14 15:14                             ` Tejun Heo
2016-07-14 13:18               ` Peter Zijlstra
2016-07-14 14:14                 ` Peter Zijlstra
2016-07-14 14:58                 ` Oleg Nesterov
2016-07-14 16:14                   ` Peter Zijlstra
2016-07-14 16:37                   ` Peter Zijlstra
2016-07-14 17:05                     ` Oleg Nesterov
2016-07-14 16:23                 ` Paul E. McKenney
2016-07-14 16:45                   ` Peter Zijlstra
2016-07-14 17:15                     ` Paul E. McKenney
2016-07-14 16:43                 ` John Stultz
2016-07-14 16:49                   ` Peter Zijlstra
2016-07-14 17:02                     ` John Stultz
2016-07-14 17:13                       ` Oleg Nesterov
2016-07-14 17:30                         ` John Stultz
2016-07-14 17:41                           ` Oleg Nesterov
2016-07-14 17:51                             ` John Stultz
2016-07-14 18:09                 ` Oleg Nesterov
2016-07-14 18:36                   ` Peter Zijlstra
2016-07-14 19:35                     ` Peter Zijlstra
2016-07-13 20:57             ` John Stultz
2016-07-13 20:52           ` Paul E. McKenney
2016-07-13 20:57             ` Peter Zijlstra
2016-07-13 21:08               ` Paul E. McKenney
2016-07-13 21:01             ` Dmitry Shmidt
2016-07-13 21:03               ` John Stultz
2016-07-13 21:05               ` Paul E. McKenney
2016-07-13 20:31     ` Dmitry Shmidt
2016-07-13 20:44   ` Colin Cross
2016-07-13 20:54     ` Tejun Heo

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=20160713182102.GJ4065@mtj.duckdns.org \
    --to=tj@kernel.org \
    --cc=ccross@google.com \
    --cc=dimitrysh@google.com \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=romlem@google.com \
    --cc=tkjos@google.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

all inboxes | Powered by JetHome®