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 526A4C6FD1D for ; Fri, 7 Apr 2023 06:03:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A47646B0072; Fri, 7 Apr 2023 02:03:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9F9346B0074; Fri, 7 Apr 2023 02:03:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8BEF76B0075; Fri, 7 Apr 2023 02:03:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 799B46B0072 for ; Fri, 7 Apr 2023 02:03:41 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 46A931C5AEE for ; Fri, 7 Apr 2023 06:03:41 +0000 (UTC) X-FDA: 80653553442.06.69B6C19 Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by imf23.hostedemail.com (Postfix) with ESMTP id 6C0D3140012 for ; Fri, 7 Apr 2023 06:03:38 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=C0OKCAkN; spf=pass (imf23.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=yosryahmed@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=1680847418; 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=4bG/ZnLi19D5O/SrE3f26gQDppjt4TYCXq5qDEfu3yk=; b=WKFca1ctmkAgXkqCnqXK/c8j7f41tX15KVoGfVlVmpAbIlDdPAtCnupwHIJZiFxN8ap6i0 ViezEP5ktlwHsUXa4RD90GDYO2qXSr3IJySKLDE1xogox8fU4ErP1Yz/iaLXTuC5O8EBmg 9pdLFggq3+a88UNFTTWll4O8dam/zT8= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=C0OKCAkN; spf=pass (imf23.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680847418; a=rsa-sha256; cv=none; b=x2Kk7w5nR5rDJNcbDph2NP8X3cwkTNgdXZT8bMkV+Nsmt/CSA3SN/RErJkwVZwhjLaGsvW o5sFIEHdD3awFthWZS14HQcTU8STKN7naKoi/KJgm9mmkKfOi4q5cmQycxqBow3Yuij+GP EEvHDC8LaAiGmoaMgRDKlBVo817Oj9A= Received: by mail-ej1-f47.google.com with SMTP id sg7so6262808ejc.9 for ; Thu, 06 Apr 2023 23:03:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680847417; x=1683439417; 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=4bG/ZnLi19D5O/SrE3f26gQDppjt4TYCXq5qDEfu3yk=; b=C0OKCAkNpCuqMVoglQszSXSfrLXbKCG88wzIvdCQWt2IKTq2EOQqXymr+cXj/mD0f5 GLf5/ZxghjmOVsI+B9Omic/DGS+gJTTeeyVne7MVe+2gTSvpfFkhLXjtNJdGcTtdeh7k 3q85559UjToVuMHX706df5nD8mngIV/snDziDP9A+F6AsxUK9/IB2m59k2k87FDz7BLn m96JJcWSapLbiaW8Sl71JnMxlpjUMQAmTOWO1I+ER6rAWA4dMoYZsjj74/zIWY5r8ASP MsJnkpHhzg8M2ZSLMTGb+5LVgB5MW6Vx4U4hKW2FiM0TZPE/3n+uMmxJwdeNMqxh8eLO EXQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680847417; x=1683439417; 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=4bG/ZnLi19D5O/SrE3f26gQDppjt4TYCXq5qDEfu3yk=; b=GCGQnq7tKmC3mlaeOZBT2yOkTH0vzU17Xx7jXs38X2xWW8psXY4BiHw/P85ecCNqMX 0IEce2RQlXGAOS3eeJkJnQ6MZR+m8FMOJseO5dpUkIRNnZqPTRZxx1iRBugt0okVYvO+ LEnWR+1R2vqAceypD+/ka8+NN/RbLw0wfCfZG1Qacv4jRVYOBr69Uav5zvv6wydnf0f7 IP576UsyXnnVghGxIOh43O6AbhGznDkfvZqXPBH458S68tLFj+GSAaEY/IBB/qAsvkyG dcAHB/kuQPLcPPCiwtRpIzoCbnkeP3lxgDrvy08UA/Ailgw+JkA3W/Ju1jXyML8Nmz+z NASA== X-Gm-Message-State: AAQBX9ewlCfsdjfPWIkUt9m1NjbhVSKLt2Ff/it1ob9ARhqA8hTNKZd7 1hcNrMzoqSNHEnFCHUCz6r1BBnPI36zw5XRuJluw0A== X-Google-Smtp-Source: AKy350Zs79NcKShlkUJkXCvv+lqDNYKXRHT7Sgzmh3o6Jvl7qDy8I9/biYjb2riqTPkATS+AGUjUtlLtPeu2PIsuWXo= X-Received: by 2002:a17:906:7ac5:b0:933:7658:8b44 with SMTP id k5-20020a1709067ac500b0093376588b44mr522670ejo.15.1680847416736; Thu, 06 Apr 2023 23:03:36 -0700 (PDT) MIME-Version: 1.0 References: <20230405200150.GA35884@cmpxchg.org> <20230406200213.GA50192@cmpxchg.org> In-Reply-To: <20230406200213.GA50192@cmpxchg.org> From: Yosry Ahmed Date: Thu, 6 Apr 2023 23:03:00 -0700 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: Johannes Weiner Cc: Yu Zhao , 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-Stat-Signature: iid6tm7ao79rbkbfbfw5qrdor9xhd6iw X-Rspam-User: X-Rspamd-Queue-Id: 6C0D3140012 X-Rspamd-Server: rspam06 X-HE-Tag: 1680847418-249761 X-HE-Meta: U2FsdGVkX1+JhNzeyxi06GpQ9c1QafW3K/o/GD8WEW8Pca84q40qM5RFdgGAh5/ipTyTJPBzYOVAKj8fj1rclD+xtj5Nxa+L0GkyiQE7jVkD5Mqnt4n5pboYuOfsAI8nNGh0dMWOoanYYA2ZbnZI3vj2ZcjFWHfQhoVbam8A5hmPWVibB+X50ICy9gna2AezxiOCII1ytlaN7XowQXR3Mrpr4RNQyP2ZOykoM07bG/1WdpiQXlJ1XP7c9gsyuX9jDjXej84PgHz2GAfh1aSQLfWinnPdaXXP1hrpBqLTp2cEQtntdhcW3jF8H4zAeas4TzUDR6/LxaXoG6wMxO1B/IU4tQWJTxpvSqWGad2Le9N1Uzn/neqkL0Cjpwl+Gjx7p/fUVwhdD+H3+hgopZAmiVbTovjv33FAg5L/mzBTlODeWGPoDqwbE0sjOQDKYTk7ABVpwpp5HDuy2S0cOP1atjwXTq6XzlY+xj5mf9hVtRTSZTXnL4SSk1uWW+FRrZd1jtuy1igVWzj1gXCRzes8V+Z+Z+6By8SQWK8+dFEMA8ixPR72IqVQSjzyqteuskRdlJE4yhlayWatlzq0ymnptwJSwAI94pNUtotD9HRtd4QoBNS1D5owbu3ebjH0J7JK1RbrBwLbvieYm/gMmFBERx6nesMh2M4DBeu2KyDUcWcRG+Ut50jsBiw5le7k3d56+0t8chT/KMmwWsIq77foxQu5qNwoo7kMXjIPnxLYCpTDdcVoNWCRR1Q7GG6cdu6Aws1IEwwfxLDog0oX/UUQxSWifwTchmw8L0ChWLZB35D40a6E7reHWNRw/ssXKWOBgfESMThzHcH1vdfWrEmAS5abwqRW/hRgsSiVCy2pfpZX2gC7H/lo3h5b8+hpicfyJDmOKfnPodLQ1DYhGtKjXc7vyl3rwb4UNmTMYvfYcMtBhPjUMut6467A4j7ZYGeXqYmM0VMyeoq7cuaLb55 6JGNgaB2 /qO/vPT3WIwVtdNnGiYjr8UMxt49amhAYk7lrtr2ptWpXZ2IelacUrTF4QQ4IiH33wH+CKnLlRFqqW7C5S/c+Go94QC8uq3AWPhK5NmFWbeuv5wH4YpXRVWQ5g13xMrdS4hexgpIjCbNe3FogJB3yVYlld7dXoHFWQgTmkpTloy4nLOfv+2zO5dp6KhNf9qVX4oijPYyJ5PzWnL4bf5CNbITDsnQmHqrkP6on6FEUHoUAHtdVJ1mlutz3aGPqqVJpYilJ2m8uuMxcMGcMcQ4FmkPvAHi1V/9HMCy6I3RXHxYsE7Vx2Q+F/DLcG6xNHNGYmd3wRMTPCeHXicBaWvCyDH5bYy9LhVf1Pl7x X-Bogosity: Ham, tests=bogofilter, spamicity=0.000003, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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->targe= t_mem_cgroup); > > > } > > > > > > The name suggests it's the same thing twice, with opposite > > > polarity. But of course they're subtly different, and not documented. > > > > > > When do you use which? > > > > The problem I saw is that target_mem_cgroup is set when writing to the > > 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() to > > 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).