Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 9 May 2022 17:44:24 -0700
From:   Andrew Morton <akpm@linux-foundation.org>
To:     Johannes Weiner <hannes@cmpxchg.org>
Cc:     David Vernet <void@manifault.com>,
        Michal =?ISO-8859-1?Q?Koutn=FD?= <mkoutny@suse.com>,
        tj@kernel.org, roman.gushchin@linux.dev,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        cgroups@vger.kernel.org, mhocko@kernel.org, shakeelb@google.com,
        kernel-team@fb.com, Richard Palethorpe <rpalethorpe@suse.com>
Subject: Re: [PATCH v2 2/5] cgroup: Account for memory_recursiveprot in
 test_memcg_low()
Message-Id: <20220509174424.e43e695ffe0f7333c187fba8@linux-foundation.org>
In-Reply-To: <Ynkum8DeJIAtGi9y@cmpxchg.org>
References: <20220423155619.3669555-1-void@manifault.com>
        <20220423155619.3669555-3-void@manifault.com>
        <20220427140928.GD9823@blackbody.suse.cz>
        <20220429010333.5rt2jwpiumnbuapf@dev0025.ash9.facebook.com>
        <20220429092620.GA23621@blackbody.suse.cz>
        <20220506164015.fsdsuv226nhllos5@dev0025.ash9.facebook.com>
        <Ynkum8DeJIAtGi9y@cmpxchg.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, 9 May 2022 11:09:15 -0400 Johannes Weiner <hannes@cmpxchg.org> wrot=
e:

> On Fri, May 06, 2022 at 09:40:15AM -0700, David Vernet wrote:
> > Sorry for the delayed reply, Michal. I've been at LSFMM this week.
> >=20
> > On Fri, Apr 29, 2022 at 11:26:20AM +0200, Michal Koutn=FD wrote:
> > > I still think that the behavior when there's no protection left for t=
he
> > > memory.low =3D=3D 0 child, there should be no memory.low events (not =
just
> > > uncounted but not happening) and test should not accept this (even
> > > though it's the current behavior).
> >
> > That's fair. I think part of the problem here is that in general, the
> > memcontroller itself is quite heuristic, so it's tough to write tests t=
hat
> > provide useful coverage while also being sufficiently flexible to avoid
> > flakiness and over-prescribing expected behavior. In this case I think =
it's
> > probably correct that the memory.low =3D=3D 0 child shouldn't inherit
> > protection from its parent under any circumstances due to its siblings
> > overcommitting the parent's protection, but I also wonder if it's really
> > necessary to enforce that. If you look at how much memory A/B/E gets at=
 the
> > end of the reclaim, it's still far less than 1MB (though should it be 0=
?).
> > I'd be curious to hear what Johannes thinks.
>=20
> We need to distinguish between what the siblings declare and what they
> consume.
>=20
> My understanding of the issue you're raising, Michal, is that
> protected siblings start with current > low, then get reclaimed
> slightly too much and end up with current < low. This results in a
> tiny bit of float that then gets assigned to the low=3D0 sibling; when
> that sibling gets reclaimed regardless, it sees a low event. Correct
> me if I missed a detail or nuance here.
>=20
> But unused float going to siblings is intentional. This is documented
> in point 3 in the comment above effective_protection(): if you use
> less than you're legitimately claiming, the float goes to your
> siblings. So the problem doesn't seem to be with low accounting and
> event generation, but rather it's simply overreclaim.
>=20
> It's conceivable to make reclaim more precise and then tighten up the
> test. But right now, David's patch looks correct to me.

So I think we're OK with [2/5] now.  Unless there be objections, I'll
be looking to get this series into mm-stable later this week.