Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 2 Dec 2019 11:11:00 -0800
From:   Tejun Heo <tj@kernel.org>
To:     Michal =?iso-8859-1?Q?Koutn=FD?= <mkoutny@suse.com>
Cc:     cgroups@vger.kernel.org, Li Zefan <lizefan@huawei.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] cgroup/pids: Make pids.events notifications affine
 to pids.max
Message-ID: <20191202191100.GF16681@devbig004.ftw2.facebook.com>
References: <20191128172612.10259-1-mkoutny@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20191128172612.10259-1-mkoutny@suse.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello,

On Thu, Nov 28, 2019 at 06:26:12PM +0100, Michal Koutn? wrote:
> Currently, when pids.max limit is breached in the hierarchy, the event
> is counted and reported in the cgroup where the forking task resides.
> 
> The proper hierarchical behavior is to count and report the event in the
> cgroup whose limit is being exceeded. Apply this behavior in the default
> hierarchy.
> 
> Reasons for RFC:
> 
> 1) If anyone has adjusted their readings to this behavior, this is a BC
>    break.
> 
> 2) This solves no reported bug, just a spotted inconsistency.
> 
> 3) One step further would be to distinguish pids.events and
>    pids.events.local for proper hierarchical counting. (The current
>    behavior wouldn't match neither though.)

Yeah this is incosistent with memcg but there max / high events are
essentially useless because that doesn't indicate actual limit breach.
Both events are interesting - which cgroup's limit was reached and who
suffered because of that.

So, maybe sth like the following?

1. Make max event propagate hierarchically.  This is a behavior change
   but also an obvious bug fix.  Given that internal cgroups don't
   have processes in cgroup2, maybe it's safe enough?

2. Add another (hierarchical, of course) event which counts the number
   of fork rejects.  I can't think of a good name.  Any ideas?

Thanks.

-- 
tejun