Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
MIME-Version: 1.0
References: <20231119194740.94101-1-ryncsn@gmail.com>
In-Reply-To: <20231119194740.94101-1-ryncsn@gmail.com>
From:   Yosry Ahmed <yosryahmed@google.com>
Date:   Mon, 20 Nov 2023 11:09:58 -0800
Message-ID: <CAJD7tkYXcQkTZkUnAz89dR0O1YmEUr29UFeg3142t6Y09aaSpg@mail.gmail.com>
Subject: Re: [PATCH 00/24] Swapin path refactor for optimization and bugfix
To:     Kairui Song <kasong@tencent.com>
Cc:     linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
        "Huang, Ying" <ying.huang@intel.com>,
        David Hildenbrand <david@redhat.com>,
        Hugh Dickins <hughd@google.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Matthew Wilcox <willy@infradead.org>,
        Michal Hocko <mhocko@suse.com>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, Nov 19, 2023 at 11:48=E2=80=AFAM Kairui Song <ryncsn@gmail.com> wro=
te:
>
> From: Kairui Song <kasong@tencent.com>
>
> This series tries to unify and clean up the swapin path, fixing a few
> issues with optimizations:
>
> 1. Memcg leak issue: when a process that previously swapped out some
>    migrated to another cgroup, and the origianl cgroup is dead. If we
>    do a swapoff, swapped in pages will be accounted into the process
>    doing swapoff instead of the new cgroup. This will allow the process
>    to use more memory than expect easily.
>
>    This can be easily reproduced by:
>    - Setup a swap.
>    - Create memory cgroup A, B and C.
>    - Spawn process P1 in cgroup A and make it swap out some pages.
>    - Move process P1 to memory cgroup B.
>    - Destroy cgroup A.
>    - Do a swapoff in cgroup C
>    - Swapped in pages is accounted into cgroup C.
>
>    This patch will fix it make the swapped in pages accounted in cgroup B=
.
>

I guess this only works for anonymous memory and not shmem, right?

I think tying memcg charges to a process is not something we usually
do. Charging the pages to the memcg of the faulting process if the
previous owner is dead makes sense, it's essentially recharging the
memory to the new owner. Swapoff is indeed a special case, since the
faulting process is not the new owner, but an admin process or so. I
am guessing charging to the new memcg of the previous owner might make
sense in this case, but it is a change of behavior.