From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6D8ECC25B7A for ; Thu, 23 May 2024 20:26:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AAE226B0092; Thu, 23 May 2024 16:26:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A5E216B0093; Thu, 23 May 2024 16:26:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9254B6B0095; Thu, 23 May 2024 16:26:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 74B6E6B0092 for ; Thu, 23 May 2024 16:26:29 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 1CB11805F8 for ; Thu, 23 May 2024 20:26:29 +0000 (UTC) X-FDA: 82150793298.02.AE2DA3E Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf05.hostedemail.com (Postfix) with ESMTP id 2AB7A100020 for ; Thu, 23 May 2024 20:26:26 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=sJxBMj1i; spf=pass (imf05.hostedemail.com: domain of chrisl@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=chrisl@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716495987; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=0Oz8uava/hvgo+Stz5RjG+dqebgmh3OUgJGPAQ0PxrI=; b=cX6MfbAoBQL/z4gHnDVWZR62GtUr3QhcE1esYuC1vaWT+FZVAMmxHLtC3+lpEYrG5qtNlI THWH3z2vIRiZz2LhIa+ReBdi42ROk3NilnI+ZmWOW8xypTHIldiTnf6q1X8k9m+zIgzsa2 qGh5FvGia0bmrepKTM8+NBOfCd2zE6c= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716495987; a=rsa-sha256; cv=none; b=R8ashXgnN1JZsQM8eHOwHM0hl+rr1vib58uuvetMo9wmcwqkuBn4TgydIwSUC7iEZNDoQS HRgUE4W6UX6lQxvQkU2++gu3bnBwYZRLAG5IP3xPUYtI9NOMHbNQPRlmGTeVJsCtM/i1ak gFtprAV+MPRbbrHnnwNSwOYt7dBHkgA= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=sJxBMj1i; spf=pass (imf05.hostedemail.com: domain of chrisl@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=chrisl@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 05D5E62EFE for ; Thu, 23 May 2024 20:26:26 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 739F3C32782 for ; Thu, 23 May 2024 20:26:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1716495985; bh=JnrUkGmHC75Gk19tpLIyKV+pZYcgqu/foMs9FSsCvpc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=sJxBMj1iBdiVHZAY07bHX/exqJs43SnQIwAcdFGS6uxB/qrzKkm6ybVpsbWRsfG+f ckQpacOOGp85cIBPhnIUdFaUQuOqjwLaIAhXHspvauZu2iQGQ0Om89JVSHnqlS5S37 UtmZRHVo9akZNni0M+WYCE5vvu5noHTgbGtMHvTM2OSTPleahAgT8z5doJyq8agCPB /+xdyR1bSZWIG4eRVBOBq6kI2hUWlhV7rka3FpGY+Up3WpvlLsMXQKEA6s+iYm0s52 4g0G3RPp0ak07rjj5efTX106/prRE+XW64CS4S7VjDj91r4AUMBc3USGxGDMjj9tzn X84nNhVEQlonQ== Received: by mail-lf1-f49.google.com with SMTP id 2adb3069b0e04-52962423ed8so98299e87.2 for ; Thu, 23 May 2024 13:26:25 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCXu7I8tFrFRa9T9oq/SO3D6nkNQXG4+SOiU8uPJb43Peanh5ktsFO920bZtq3Eg++Yk6ky1w2Rczx4E1peuwt51qV8= X-Gm-Message-State: AOJu0YwwimwULDpME8kZaFmERzSFIK+JRc0/xL783qH20kcbne3HcJwc ce2YSvRmUpK0/duBjPV/bY6kBz4O5HzIou1JbWUma6aZJ3+NpMceWQYPAFce6Nt8CY7hy3XC9PO 57tmRIjRFt2t1JVOMucn+F2gFiw== X-Google-Smtp-Source: AGHT+IEuR1DxhXdiaoN5V+oe7jhw9uz8fupqmqOjSxM977L1TpJN8Xe6r6d7xTSuZXtPQgH0a8lnRyt7hiqhEfGsfgI= X-Received: by 2002:a05:6512:b1a:b0:51d:68fb:5d73 with SMTP id 2adb3069b0e04-529645e2436mr126990e87.8.1716495984097; Thu, 23 May 2024 13:26:24 -0700 (PDT) MIME-Version: 1.0 References: <20240509034138.2207186-1-roman.gushchin@linux.dev> In-Reply-To: From: Chris Li Date: Thu, 23 May 2024 13:26:10 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH rfc 0/9] mm: memcg: separate legacy cgroup v1 code and put under config option To: Roman Gushchin Cc: Kairui Song , Shakeel Butt , Andrew Morton , Muchun Song , Johannes Weiner , Michal Hocko , Matthew Wilcox , linux-mm@kvack.org, linux-kernel@vger.kernel.org, gthelen@google.coma, rientjes@google.com, Michal Hocko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: hugwnzweb9ra1xx55jb7t4i55rtmc8nx X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 2AB7A100020 X-HE-Tag: 1716495986-517780 X-HE-Meta: U2FsdGVkX18tYBBHGdAa0ug02+hebKmXH8vJMoAY6Gf6xwx0iziqNELGQO0rX3x3e+P2rghuKQju1M9lTjwcf+dnImNReaNm9Rox3cxgA6C78blMEvNAnpKja5ceA2wlGnY4BHZ0D9KVNx8FpqFsXmV3j0bkhdPEnQK2dYXhhI7wx6eJwFahmuwKDyadqDCVOzgw3TZA72+GJwfXqDe+t/lc2biQ9jkmrQza/CyabVkE6SHxmQKeLVVWKDr3G3Cg040O2AmL0GDaQFn11VezGcu/x8InRX1Hwz7q2IQfVAUqIRMujsJY6trhEESl1vbkrYc7Nw36CN+NGjCDcLZxcp51iQ137RIp69UeGhHvEPhnVTulk9QWIMbPPrtNMqqKFnGL7mzwlrI0ADhhO37JhKRZGUiaYsLggj6FtEKBZMZHCEvt/E69X3sTkRoeDlzMmO68wHaP1gVagrBsjNvjZJyxYQ8yaTrzgkieAngqbeu79rNu0cW/UYatMe0D1myYlNDcAwH9CDftKIjneC5mBejYRN3G52KOEAnudt9n0JuIjHf+FuKAlEVgFQFVR/BE4/UolwNYAhjFjsbBXB5uEC0lMuMgcou2LGVJLz1NTf+mnxVwhHCt7lriEgzDbhB8GGNQnqxadHkGk2czEhmxB1rHl+sjHyxkylveUN01Ln6CNbCyS4b90Bdx7Pl6+372d8Aty3Pw181U5J80GFjHuDHX1yJDEB6w+hfEn30Cu2r1yi3c/BUbobzGouUswu66zCkt/A0efw7LFEpt+zRDc2vy+Ec9XOFc8JGjaIL+bgPe6ze/nv09VrLeX7oX76dnh7NZyGNVbgGeiFV4G5NdpRv519mTJgHvL3NaoJyl05bnBbZWHmj1kuts6EpUUnlazOsraZo8LmkkBy56oUr0dlXnD4HE4DhCb2TtPO/EW4QgBTPSFGIC7Zo+R4+NmGx6rj1lvVue8AH2BrVuYwZ uw27Ttoz rR3HSbSADU+xWPRV5xBlTzr2jQvpVrQq4eDCr7rjWduewy/ToQGEfK/Fl2FtVXjSymY2eDc929AqwCEEz3t87vSIkHAAyQYg4xysYv8yhWDlfHDWly4gxog4T37wUChak1TOdi2HhDXei981KR6YBx2LS4b37Gur2wslVsqTITtV1VQXdRBiUuL4k5mlEKaaRDV7VyxHx2AFV/b/WlVbB3ns7QXfor2BMUDOp X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, May 23, 2024 at 12:56=E2=80=AFPM Roman Gushchin wrote: > > On Thu, May 23, 2024 at 01:58:49AM +0800, Kairui Song wrote: > > On Thu, May 9, 2024 at 2:33=E2=80=AFPM Shakeel Butt wrote: > > > > > > On Wed, May 08, 2024 at 08:41:29PM -0700, Roman Gushchin wrote: > > > > Cgroups v2 have been around for a while and many users have fully a= dopted them, > > > > so they never use cgroups v1 features and functionality. Yet they h= ave to "pay" > > > > for the cgroup v1 support anyway: > > > > 1) the kernel binary contains useless cgroup v1 code, > > > > 2) some common structures like task_struct and mem_cgroup have neve= r used > > > > cgroup v1-specific members, > > > > 3) some code paths have additional checks which are not needed. > > > > > > > > Cgroup v1's memory controller has a number of features that are not= supported > > > > by cgroup v2 and their implementation is pretty much self contained= . > > > > Most notably, these features are: soft limit reclaim, oom handling = in userspace, > > > > complicated event notification system, charge migration. > > > > > > > > Cgroup v1-specific code in memcontrol.c is close to 4k lines in siz= e and it's > > > > intervened with generic and cgroup v2-specific code. It's a burden = on > > > > developers and maintainers. > > > > > > > > This patchset aims to solve these problems by: > > > > 1) moving cgroup v1-specific memcg code to the new mm/memcontrol-v1= .c file, > > > > 2) putting definitions shared by memcontrol.c and memcontrol-v1.c i= nto the > > > > mm/internal.h header > > > > 3) introducing the CONFIG_MEMCG_V1 config option, turned on by defa= ult > > > > 4) making memcontrol-v1.c to compile only if CONFIG_MEMCG_V1 is set > > > > 5) putting unused struct memory_cgroup and task_struct members unde= r > > > > CONFIG_MEMCG_V1 as well. > > > > > > > > This is an RFC version, which is not 100% polished yet, so but it w= ould be great > > > > to discuss and agree on the overall approach. > > > > > > > > Some open questions, opinions are appreciated: > > > > 1) I consider renaming non-static functions in memcontrol-v1.c to h= ave > > > > mem_cgroup_v1_ prefix. Is this a good idea? > > > > 2) Do we want to extend it beyond the memory controller? Should > > > > 3) Is it better to use a new include/linux/memcontrol-v1.h instead = of > > > > mm/internal.h? Or mm/memcontrol-v1.h. > > > > > > > > > > Hi Roman, > > > > > > A very timely and important topic and we should definitely talk about= it > > > during LSFMM as well. I have been thinking about this problem for qui= te > > > sometime and I am getting more and more convinced that we should aim = to > > > completely deprecate memcg-v1. > > > > > > More specifically: > > > > > > 1. What are the memcg-v1 features which have no alternative in memcg-= v2 > > > and are blocker for memcg-v1 users? (setting aside the cgroup v2 > > > structual restrictions) > > > > > > 2. What are unused memcg-v1 features which we should start deprecatin= g? > > > > > > IMO we should systematically start deprecating memcg-v1 features and > > > start unblocking the users stuck on memcg-v1. > > > > > > Now regarding the proposal in this series, I think it can be a first > > > step but should not give an impression that we are done. The only > > > concern I have is the potential of "out of sight, out of mind" situat= ion > > > with this change but if we keep the momentum of deprecation of memcg-= v1 > > > it should be fine. > > > > > > I have CCed Greg and David from Google to get their opinion on what > > > memcg-v1 features are blocker for their memcg-v2 migration and if the= y > > > have concern in deprecation of memcg-v1 features. > > > > > > Anyone else still on memcg-v1, please do provide your input. > > > > > > > Hi, > > > > Sorry for joining the discussion late, but I'd like to add some info > > here: We are using the "memsw" feature a lot. It's a very useful knob > > for container memory overcommitting: It's a great abstraction of the > > "expected total memory usage" of a container, so containers can't > > allocate too much memory using SWAP, but still be able to SWAP out. +Michal, Just FYI, we do have companies like Tensent using the V1 combine memsw limitation as well. Google is not the only company using this API. > > > > For a simple example, with memsw.limit =3D=3D memory.limit, containers > > can't exceed their original memory limit, even with SWAP enabled, they > > get OOM killed as how they used to, but the host is now able to > > offload cold pages. > > > > Similar ability seems absent with V2: With memory.swap.max =3D=3D 0, th= e > > host can't use SWAP to reclaim container memory at all. But with a > > value larger than that, containers are able to overuse memory, causing > > delayed OOM kill, thrashing, CPU/Memory usage ratio could be heavily > > out of balance, especially with compress SWAP backends. > > > > Cgroup accounting of ZSWAP/ZRAM doesn't really help, we want to > > account for the total raw usage, not the compressed usage. One example > > is that if a container uses tons of duplicated pages, then it can > > allocate much more memory than it is limited, that could cause > > trouble. > > So you don't need separate swap knobs, only combined, right? > > > I saw Chris also mentioned Google has a workaround internally for it > > for Cgroup V2. This will be a blocker for us and a similar workaround > > might be needed. It will be great so see an upstream support for this. > > I think that _at least_ we should refactor the code so that it would > be a minimal patch (e.g. one #define) to switch to the old mode. That would be great to have a path forward to allow cgroup V2 to provide the combined memsw limitations. > > I don't think it's reasonable to add a new interface, but having a > patch/config option or even a mount option which changes the semantics > of memory.swap.max to the v1-like behavior should be ok. Using sysctl or a slightly different cgroup API is fine. The feature needs to be there. > I'll try to do the first part (refactoring this code), and we can have > a discussion from there. Looking forward to it. Thanks Chris