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;
MIME-Version: 1.0
References: <20221122203850.2765015-1-almasrymina@google.com> <Y35fw2JSAeAddONg@cmpxchg.org>
In-Reply-To: <Y35fw2JSAeAddONg@cmpxchg.org>
From:   Mina Almasry <almasrymina@google.com>
Date:   Wed, 23 Nov 2022 13:20:57 -0800
Message-ID: <CAHS8izN+xqM67XLT4y5qyYnGQMUWRQCJrdvf2gjTHd8nZ_=0sw@mail.gmail.com>
Subject: Re: [RFC PATCH V1] mm: Disable demotion from proactive reclaim
To:     Johannes Weiner <hannes@cmpxchg.org>
Cc:     Huang Ying <ying.huang@intel.com>,
        Yang Shi <yang.shi@linux.alibaba.com>,
        Yosry Ahmed <yosryahmed@google.com>,
        Tim Chen <tim.c.chen@linux.intel.com>, weixugc@google.com,
        shakeelb@google.com, gthelen@google.com, fvdl@google.com,
        Michal Hocko <mhocko@kernel.org>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Muchun Song <songmuchun@bytedance.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
        linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Nov 23, 2022 at 10:00 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> Hello Mina,
>
> On Tue, Nov 22, 2022 at 12:38:45PM -0800, Mina Almasry wrote:
> > Since commit 3f1509c57b1b ("Revert "mm/vmscan: never demote for memcg
> > reclaim""), the proactive reclaim interface memory.reclaim does both
> > reclaim and demotion. This is likely fine for us for latency critical
> > jobs where we would want to disable proactive reclaim entirely, and is
> > also fine for latency tolerant jobs where we would like to both
> > proactively reclaim and demote.
> >
> > However, for some latency tiers in the middle we would like to demote but
> > not reclaim. This is because reclaim and demotion incur different latency
> > costs to the jobs in the cgroup. Demoted memory would still be addressable
> > by the userspace at a higher latency, but reclaimed memory would need to
> > incur a pagefault.
> >
> > To address this, I propose having reclaim-only and demotion-only
> > mechanisms in the kernel. There are a couple possible
> > interfaces to carry this out I considered:
> >
> > 1. Disable demotion in the memory.reclaim interface and add a new
> >    demotion interface (memory.demote).
> > 2. Extend memory.reclaim with a "demote=<int>" flag to configure the demotion
> >    behavior in the kernel like so:
> >       - demote=0 would disable demotion from this call.
> >       - demote=1 would allow the kernel to demote if it desires.
> >       - demote=2 would only demote if possible but not attempt any
> >         other form of reclaim.
>
> Unfortunately, our proactive reclaim stack currently relies on
> memory.reclaim doing both. It may not stay like that, but I'm a bit
> wary of changing user-visible semantics post-facto.
>
> In patch 2, you're adding a node interface to memory.demote. Can you
> add this to memory.reclaim instead? This would allow you to control
> demotion and reclaim independently as you please: if you call it on a
> node with demotion targets, it will demote; if you call it on a node
> without one, it'll reclaim. And current users will remain unaffected.

Hello Johannes, thanks for taking a look!

I can certainly add the "nodes=" arg to memory.reclaim and you're
right, that would help in bridging the gap. However, if I understand
the underlying code correctly, with only the nodes= arg the kernel
will indeed attempt demotion first, but the kernel will also merrily
fall back to reclaiming if it can't demote the full amount. I had
hoped to have the flexibility to protect latency sensitive jobs from
reclaim entirely while attempting to do demotion.

There are probably ways to get around that in the userspace. I presume
the userspace can check if there is available memory on the node's
demotion targets, and if so, the kernel should demote-only. But I feel
that wouldn't be reliable as the demotion logic may change across
kernel versions. The userspace may think the kernel would demote but
instead demotion failed due to whatever heuristic introduced into the
new kernel version.

The above is just one angle of the issue. Another angle (which Yosry
would care most about I think) is that at Google we call
memory.reclaim mainly when memory.current is too close to memory.max
and we expect the memory usage of the cgroup to drop as a result of a
success memory.reclaim call. I suspect once we take in commit
3f1509c57b1b ("Revert "mm/vmscan: never demote for memcg reclaim""),
we would run into that regression, but I defer to Yosry here, he may
have a solution for that in mind already.

For these reasons, what do you think about adding both the "nodes="
and "demote=" args from my patch series to memory.reclaim? With these
args the default memory.reclaim behavior would suit meta as-is (no
change) but we would be able to configure it to address our use cases
as well.

As always, thanks for taking the time to review!