Re: Severe performance regression w/ 4.4+ on Android due to cgroup locking changes

[Tejun Heo]

(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