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 7905FC6FD1D for ; Fri, 7 Apr 2023 18:14:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E0C746B0072; Fri, 7 Apr 2023 14:14:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DBCC26B0075; Fri, 7 Apr 2023 14:14:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C849C900002; Fri, 7 Apr 2023 14:14:26 -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 AAD546B0072 for ; Fri, 7 Apr 2023 14:14:26 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3522D16092C for ; Fri, 7 Apr 2023 18:08:21 +0000 (UTC) X-FDA: 80655379602.14.6072757 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) by imf27.hostedemail.com (Postfix) with ESMTP id 4614340024 for ; Fri, 7 Apr 2023 18:08:19 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="L7N/LS8y"; spf=pass (imf27.hostedemail.com: domain of yuzhao@google.com designates 209.85.128.41 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680890899; 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=m3im81jdsus/Yk7rDAQ6nsUJCO1TAGpuJlX+PUtR7Vo=; b=zYrL+qOUAO/9E+NXSaw04qE1ysbNLIp5npTqxcUd6ILZLkNi93yhSHxobKW9g/51IJmaSb GW2Z9wvzK4I7jUqJakoOZEWgpDQcfwldKh4GoyQUEQsFlxzvEbg0cA9KkA1FDsgZ8qCHRV T5XNlSVu0qAboAaWIhkA28UaKCzAnBw= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="L7N/LS8y"; spf=pass (imf27.hostedemail.com: domain of yuzhao@google.com designates 209.85.128.41 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680890899; a=rsa-sha256; cv=none; b=4aJnpezkaJmHqH9wTD0uEu8WaFMfowW9Kvhsmj/s9QTnL91zzavxW1rY6/LjTlM79ILCkv h66auhb3iuRIUd3wmVWUJo3Ih3Kc3D8UuY55MD2UX6J0gGH1jL0+UhdTk2KPaWhjspiMUv b4bA/fK3xxjAjlp8dbnE80UzqvxU1Ik= Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-3ee75104d2cso135345e9.0 for ; Fri, 07 Apr 2023 11:08:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680890898; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=m3im81jdsus/Yk7rDAQ6nsUJCO1TAGpuJlX+PUtR7Vo=; b=L7N/LS8yQNYCZceC2yHepsUo0LzYzuOSNAgV++z018/fbKXDkXQODPmXvNR4flj7KX Bw+ZZJHuHkqMau2tBNMVn8DcjIOgvRsUFP43QQiAAJZNRs+TowD8zx6APkWSc7dAuR0O ebA4Ae1vbiFiHl1XBMr7ZsLNvn87+7w70nMg71RzCBcfHiialclrn/iGQVEg4VtGFs5u //sTmi7yeJw1GsM6TaUfqn7aWv4BPiKBkFxit2kcyhRmLt/xL/oUVy1U0JZoiBZagGQJ lC0an42UPWyjIDDUWR//7G0n6Q7toOFYTc/51RTLHmsRd2wldTDev6jfEsABuX7uba2r Zi6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680890898; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=m3im81jdsus/Yk7rDAQ6nsUJCO1TAGpuJlX+PUtR7Vo=; b=ULXeo2zPyBCvtaS8cZ/uUqT5YB7/IuaHrkxn1ECeJzRu+NpAVgDAMmtwFmoEp39bpf dcZtoOFvhwqtgHDpyWHzP9gE/GsdCigptc1wvLhh98Z2WBnVBT6fsezWf5m70d67NxOL p9Q0bcxI4IgtwTn2SdAsZN79sI19AoHyseMPIbjRoPdJHIoJrNrcY1BmlATswZB31JkZ YmvVJzlGH7XAN1x5t/jt2uA3Mtcy29DWzG2g3rp5QJ4grtNX0NLvm9fbQx0H5PqUV6Hr uhLTnhz9+PJCsbHr+TxwM1DL6LQt7eCKXfFesSvILaXYmMJeFO8jbcuCIMNFMz21nLra Wa6w== X-Gm-Message-State: AAQBX9fRRqZh63rh5Y4wZYnV1TDmBfbitG5kVQATpLq/8qViUtuFGHux PrO2raxvCDU0xsr6zMbittmmHtwFpjjBNCxiE6KUtQ== X-Google-Smtp-Source: AKy350bj2LuHP6CSAaRSdFqFuxzZBOchnEQ15/j6t1PmhCJfRaf+gRVdZBcSdbFqsz2Oxz2EK9in4urxgaJb/hv+1Ho= X-Received: by 2002:a05:600c:82c9:b0:3df:f3cb:e8ce with SMTP id eo9-20020a05600c82c900b003dff3cbe8cemr7124wmb.7.1680890896734; Fri, 07 Apr 2023 11:08:16 -0700 (PDT) MIME-Version: 1.0 References: <20230405200150.GA35884@cmpxchg.org> <20230406200213.GA50192@cmpxchg.org> In-Reply-To: From: Yu Zhao Date: Fri, 7 Apr 2023 12:07:39 -0600 Message-ID: Subject: Re: global_reclaim() and code documentation (was: Re: [PATCH v3 3/3] mm: vmscan: ignore non-LRU-based reclaim in memcg reclaim To: Yosry Ahmed , Johannes Weiner Cc: Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Shakeel Butt , Michal Hocko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 4614340024 X-Stat-Signature: h1cwibu3e45weyg5rtbccea3xnwd3sim X-Rspam-User: X-HE-Tag: 1680890899-963741 X-HE-Meta: U2FsdGVkX19As5z/mLZ86Bvwbw/XzVOJh1ujxc2rkVz9xVa1LOkfZ/GyKlhYVFgDZwm+K1Qhq8OZP/Afxz+AVYaOg9+cKTxh6jwemEuxtg7CeXOk8W7FC9Gv6VunP4GiPw+tjAargirbcg5pjRAn8BWxB2LWaC6v2giqzrxX3v31zpl7lK++wy6rvzMlCnDqK7rzf4KgLPq1lzrqMQLR0ng0grRwfyLdh2zWwMRGWEOHgmI1bOi6+gCd4/a6YQGTHYpjCUSfuG1OQ6f9p/gf5CV6fK+uKqoezDqwi1WmLh9xVBXSkCIRc0CrvyUVGIV/V0m216VewWp8pSBMkGECYWuipjhxlfVlHjIudjh2NfJI6VvUwLJkmz4qWpphYyFtCJG/Sb5lAeuPEdbEuc+1ijmg9fMlj6awQ3chfc1KHP30QqSzX2m1b+kr8QzrtCp/4FZ8jvMXBAPWYmtqR83yvdSFd0HS3mskkpswlHZhbv8iOQtarJvb6p/ugMT8YNJVtSJS+13r05wXyv7HukArNXatmiWcLOCCYyXQBZcBIhwze9hBsaFpOuS7LhTuhOPN5T1NDR0lO7TgtLMIPZylvxN8JnObjQEpPnZMilNRXEUZ9kQ23a/8/GBYSv/I1jw8Pi3XylkaDzhQ6DcCdNcE2llcdVFt2rTjhXU+sCvzrs5JY7YZ955Up0HRhLHJJu+KS1LxuFNqOqhCsuLp9GPgtnsAPXnhj5bhJnVk8E6gYakbmQWDpyyjUMQksCaMB5L72GocSAhPb3gbSXPLWLHbli2gE9VdsEyKscV0ezeC2CzazbOugnP1OijJEQ2dJ9anIUyml9OEmHfnOU21USo+Z5I6eCQCBfLTQN9bXCM3AR36MFTbbjV20+EORLM5k88rxQfnd9rIuG3U91R1FoN1hgEuakthmDazg7M2p5/AFgXS7Opi15dx0QWXT/MaWs5AzWmWtf1iQvj1JoAVKCO BF0p9Yr/ I7qJes0IfhFVfIUbR36nZwDakzNw1ooZEvxvWlHo1GrtDkBEm/Zx+R9FLcmQLAUfRgF5+NOr1LjV3z8tlmkdqcIEfZj6lj8Xz8q8pLDMRkvspGvxWexgPTFW2/KR+boEr546kR2bDuR/FN5ETMAmzkchLRobma0RCdDyCE6TZ1GzesFyh5Kt5x7j6IRg5QUy0Hr0YBbdxGlc1HNWjjVPuCI0C5zvX/ZoPztR7k0HLeuhA9wERFHH4eA0PVfg6rAbA60GF08LiVZ09o6nkntn2JXCQmPPFZAgU8RGzVyW9bvPhIPmfysTByD+6TFfkEu4y0EjI2BlW316/szc/Sa4NxpvHMjEHgxVfo7+BLlToCTV2CsxEtGyfqLvi1Q== 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: On Fri, Apr 7, 2023 at 12:03=E2=80=AFAM Yosry Ahmed = wrote: > > On Thu, Apr 6, 2023 at 1:02=E2=80=AFPM Johannes Weiner wrote: > > > > On Wed, Apr 05, 2023 at 03:09:27PM -0600, Yu Zhao wrote: > > > On Wed, Apr 5, 2023 at 2:01=E2=80=AFPM Johannes Weiner wrote: > > > > static bool cgroup_reclaim(struct scan_control *sc) > > > > { > > > > return sc->target_mem_cgroup; > > > > } > > > > > > > > static bool global_reclaim(struct scan_control *sc) > > > > { > > > > return !sc->target_mem_cgroup || mem_cgroup_is_root(sc->tar= get_mem_cgroup); > > > > } > > > > > > > > The name suggests it's the same thing twice, with opposite > > > > polarity. But of course they're subtly different, and not documente= d. > > > > > > > > When do you use which? > > > > > > The problem I saw is that target_mem_cgroup is set when writing to th= e > > > root memory.reclaim. And for this case, a few places might prefer > > > global_reclaim(), e.g., in shrink_lruvec(), in addition to where it's > > > being used. > > > > > > If this makes sense, we could 1) document it (or rename it) and apply > > > it to those places, or 2) just unset target_mem_cgroup for root and > > > delete global_reclaim(). Option 2 might break ABI but still be > > > acceptable. > > > > Ah, cgroup_reclaim() tests whether it's limit/proactive reclaim or > > allocator reclaim. global_reclaim() tests whether it's root reclaim > > (which could be from either after memory.reclaim). > > > > I suppose we didn't clarify when introducing memory.reclaim what the > > semantics should be on the root cgroup: > > > > - We currently exclude PGSCAN and PGSTEAL stats from /proc/vmstat for > > limit reclaim to tell cgroup constraints from physical pressure. We > > currently exclude root memory.reclaim activity as well. Seems okay. > > > > - The file_is_tiny heuristic is for allocator reclaim near OOM. It's > > currently excluded for root memory.reclaim, which seems okay too. > > > > - Like limit reclaim, root memory.reclaim currently NEVER swaps when > > global swappiness is 0. The whole cgroup-specific swappiness=3D0 > > semantic is kind of quirky. But I suppose we can keep it as-is. > > > > - Proportional reclaim is disabled for everybody but direct reclaim > > from the allocator at initial priority. Effectively disabled for all > > situations where it might matter, including root memory.reclaim. We > > should also keep this as-is. > > > > - Writeback throttling is interesting. Historically, kswapd set the > > congestion state when running into lots of PG_reclaim pages, and > > clear it when the node is balanced. This throttles direct reclaim. > > > > Cgroup limit reclaim would set and clear congestion on non-root only > > to do local limit-reclaim throttling. But now root memory.reclaim > > might clear a bit set by kswapd before the node is balanced, and > > release direct reclaimers from throttling prematurely. This seems > > wrong. However, the alternative is throttling memory.reclaim on > > subgroup congestion but not root, which seems also wrong. > > > > - Root memory.reclaim is exempted from the compaction_ready() bail > > condition as well as soft limit reclaim. But they'd both be no-ops > > anyway if we changed cgroup_reclaim() semantics. > > > > IMO we should either figure out what we want to do in those cases > > above, at least for writeback throttling. > > > > Are you guys using root-level proactive reclaim? > > > > > If we don't want to decide right now, I can rename global_reclaim() t= o > > > root_reclaim() and move it within #ifdef CONFIG_LRU_GEN and probably > > > come back and revisit later. > > > > So conventional vmscan treats root-level memory.reclaim the same as > > any other cgroup reclaim. And the cgroup_reclaim() checks are mostly > > reserved for (questionable) allocator reclaim-specific heuristics. > > > > Is there a chance lrugen could do the same, and you'd be fine with > > using cgroup_reclaim()? Or is it a data structure problem? > > > > If so, I agree it could be better to move it to CONFIG_LRU_GEN and > > rename it for the time being. root_reclaim() SGTM. > > Oh and if we decide to keep the helper as root_reclaim I would prefer > it to be outside CONFIG_LRU_GEN so that I can still use it in the > patch series that this thread was originally a part of (ignoring > non-LRU freed pages in memcg reclaim). Thanks for the summaries. I think you both covered all the important detail= s. Just to recap -- it all comes down to how we want to define the semantics for the root memory.reclaim for *future* use case, since currently it has no known users (I personally use it for testing purposes, which is not important at all). So I'm fine with whatever you two see fit (delete, refactor or rename).