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 9C029C369A2 for ; Mon, 14 Apr 2025 18:57:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 644C1280079; Mon, 14 Apr 2025 14:57:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5F2DF280077; Mon, 14 Apr 2025 14:57:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4B894280079; Mon, 14 Apr 2025 14:57:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 2D82E280077 for ; Mon, 14 Apr 2025 14:57:24 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 822A2805FE for ; Mon, 14 Apr 2025 18:57:24 +0000 (UTC) X-FDA: 83333557608.10.8B11B2A Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf25.hostedemail.com (Postfix) with ESMTP id F0B37A0006 for ; Mon, 14 Apr 2025 18:57:21 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=htpWOO7o; dmarc=pass (policy=quarantine) header.from=redhat.com; spf=pass (imf25.hostedemail.com: domain of llong@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=llong@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744657042; a=rsa-sha256; cv=none; b=K4jDYHz8lr/4kGBMd8OAv0PjRllVdlK7+/yK96dP2zamIgg6MKsPLJMbGyEqCq4r9q/E+Y HyDBXHLznNeOvlkj3MKcWgpL3CctnBdPWiYTQceH4P+EXjYAdgK1MAG333qgyb7kfisL6p lWuo2sS+SwLCma0MP/rkBOwhny1dFsY= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=htpWOO7o; dmarc=pass (policy=quarantine) header.from=redhat.com; spf=pass (imf25.hostedemail.com: domain of llong@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=llong@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744657042; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=tub2ZcZaeHdUuzOD8j5+pXprxE40BWeJzXRhp/WOp+E=; b=OOjdeWKbSOSOAhxNXvO6/Pabsg82rYHMCUGbU7muJLvNcuQESm6dzxh8IbwKDEHsWPBtO5 +vyTRvDeG37vhp8Ao5limfGA0Yy3VNwKaiMvZk+s20LDK+ysRyxYLrFfiy5iAvC2FggUmR lyYP54l6Qv1iiIDkXU9AwO/LmJmLnwY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1744657041; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=tub2ZcZaeHdUuzOD8j5+pXprxE40BWeJzXRhp/WOp+E=; b=htpWOO7ofpISmhAENmkWVop7YE8DgMT5HhihOKEgLS17rIw+Dn3v9n/PV9jHWFLHjdKebF sdWgKhNvv65L2wGNuJY0UIDQZ9G2VrX+ypgNO9nosMYAuHyeyCLeDEh4H26flq94gAvTD9 YbB7P7fhzSdB9Pr4APlIn9RzEjN42ms= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-623-pgu7mHK_OQCMnDUYj07t0A-1; Mon, 14 Apr 2025 14:57:17 -0400 X-MC-Unique: pgu7mHK_OQCMnDUYj07t0A-1 X-Mimecast-MFC-AGG-ID: pgu7mHK_OQCMnDUYj07t0A_1744657037 Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-6e8feffbe08so114047476d6.0 for ; Mon, 14 Apr 2025 11:57:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744657037; x=1745261837; h=in-reply-to:content-language:references:cc:to:subject:user-agent :mime-version:date:message-id:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=tub2ZcZaeHdUuzOD8j5+pXprxE40BWeJzXRhp/WOp+E=; b=Qv0/F2VzbwtoKRiNGDyC0nsBoqdlxfP8dtx6MEimHKcDvjYMzGf0rtDisF53388DpG HTUA8cTJkqwxlC10wecgMFpAklP6oI5MwiFjBJ80Dk+0aRFbk/Tf/aVdzvnOawLgp0iS 7QYYGsxtF+EfP3fZAFzNiUx/x0g7N1pHi8QksdgTQANWNgD1iqZ8gmrxjuY7z/nzMRtV DlYsguDTBnMZ7NDPFrMpxjBMcj/as6tKyxjhWRhslZCcBHFw4wZH8ibeY5IEhjeBXwi9 n345erwGk6/luJEDTL5GNAP2c4MiMv0YtjXs1XsSkYDCAcIw+gg4FfsJnNVeHwin5m4H B8bQ== X-Forwarded-Encrypted: i=1; AJvYcCVdKDwtVenUfklI+ybcKczmJx9XkjR+kyAbpr+3HDnPuN6LRPPGTrwQrIAp8hc8+qEQFBkquAzRxA==@kvack.org X-Gm-Message-State: AOJu0Ywc+Lw/YrOufP9s1su0IwGuYfFYkw0vQNiYWbjkw/Re8365F7OS tIZNwy3CHgepOLYjrZikkIs/+Yz2cZmoboEXK2SJ3xKrVpk0+Q6EZtO4wbvyZocaPIgFxX08uge W+yfEU+i9/Af1TW+e7umxPn7sASYAf8cCncg89WJSUUOVIuk8 X-Gm-Gg: ASbGncsjEHs+Ir1P+vIxdKRRpVanhHlZOpRx1hNhpwLC+sd1wsAWsdm+5QQTFa+ruAP XayLHrszs5qhuJina1+Irl7cPYQnKG1sMR+1uycPD04PoRtNVbzVZt7O8sWToF+j4kxyF5BGCr0 owvyfpqbVLv4KNfGdf7OKKbVetLrL7lK0ebHd4WsyHtX1i+nBbZv4MMcXKmLgkVQwZI7W9N57al aAMTQdYEBHHe49ER101MGE0jhig3t0VbTvPqozZzspqnPJ+/GPIkHpo1B1B1Pk5an3wIgPYXwxM JgRSLZ1d0JeSFIbM/2euUwgDZOfpbFwbCk6UiuNcX9SKcuWs2oeAWXP5Mg== X-Received: by 2002:ad4:5761:0:b0:6e8:9394:cbbe with SMTP id 6a1803df08f44-6f230d5131bmr205147756d6.20.1744657036827; Mon, 14 Apr 2025 11:57:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEkBp5yOR8CV9Mref9tlmLoVKbcRu6nnDmmgKpNQxXrbq2n+nZwM7LZhmozBFDgppDjBjUInw== X-Received: by 2002:ad4:5761:0:b0:6e8:9394:cbbe with SMTP id 6a1803df08f44-6f230d5131bmr205147306d6.20.1744657036270; Mon, 14 Apr 2025 11:57:16 -0700 (PDT) Received: from ?IPV6:2601:408:c101:1d00:6621:a07c:fed4:cbba? ([2601:408:c101:1d00:6621:a07c:fed4:cbba]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6f0de970f5esm87054776d6.33.2025.04.14.11.57.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Apr 2025 11:57:15 -0700 (PDT) From: Waiman Long X-Google-Original-From: Waiman Long Message-ID: Date: Mon, 14 Apr 2025 14:57:14 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 1/2] mm/vmscan: Skip memcg with !usage in shrink_node_memcgs() To: Johannes Weiner , =?UTF-8?Q?Michal_Koutn=C3=BD?= Cc: Waiman Long , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , Tejun Heo , Shuah Khan , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org References: <20250414021249.3232315-1-longman@redhat.com> <20250414021249.3232315-2-longman@redhat.com> <6572da04-d6d6-4f5e-9f17-b22d5a94b9fa@redhat.com> <20250414164721.GA741145@cmpxchg.org> <20250414181014.GB741145@cmpxchg.org> In-Reply-To: <20250414181014.GB741145@cmpxchg.org> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: pZC59vZOG1JSql3JwRyjw5plrCqxoeYiqMyXDSfH3hg_1744657037 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="------------7z1PBV0p7CDcCIp8PJ0wzFUG" Content-Language: en-US X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: F0B37A0006 X-Stat-Signature: x1w6tzwgd95mefsf3kbexocoac4iawzi X-HE-Tag: 1744657041-53164 X-HE-Meta: U2FsdGVkX1/ywZeZXS2KKI8T0YhSWqUQOhoU6udr6oRny2Ex509PpraE/QWoi2b7utfphvwKlHQ9TXcxeqp9IvdxIQKTo7VNVsVpCOq3wZook57Airy6LIWF73Lb+3bsqZdf4HWwnBRFaVgewbeMyorANEpCGgQ8B0PET7WEV84TJgSPt56VPQLYg1juUpTQ7m32XzkpF1vjeht/IFuFteisllp3AUqai5/Oxe1ihJoJI/lEkW5uNpo1VkpPg6FMzvo+rFMqbCudwjwKu4ZvY3giNq+SZo7ZdQEGPM98eGOD/WQfOtnCckStEKWgP82aGJucUQcZrDwmPdkCPEsufbDK+VNa7uoti3BoWSNNCjZtLRD8flGtX6ohiBNZ/5uRU43j1ZLp/Tm7oS/mZlLyoryLfYqVwB+jQ1TX6RFPZv53WJql0LfU2lXD7hN8SmS/NNxZEv2YUVxle/IPALbUsLy4XBwJagH7yeRuTD6sDTDvPTVvxB8Fmd1nWuhxh/K1tLhHmMa6A94eAU1LlByxaG+46YrfBLpGMmOOK30J8KjTVa2SPQ9EY6EzljVzhDCoaEYqSVuQTTP0Dm8pLlw2YeV0v08XxVWpgaozoAOv+G5MaPyVdm5BhGu8xLqA8s/UlgEq3VtUnN7/nKHCvklHosxyTyh9+oFKnBdZnGAMxXui3je0ZHeuN8Rw9gUYYfkTsBVNYaj8dPPwFADr/+vPXjB8bZtsKfIeS5m3uuctLAxXr+EB+BnjJ2Qj4Bbcd8Tc/O0CIZpHwRngZMLPu1a6F1i9ysmZFqQoNeZHtRFxzwlzW9TFnYG5mE60X58TQSBGEPW+0+UfXPPz4tpooXoRfSPLKTnKtSSadpS6P7cI6E9f8d5ER5M4SVPwUZ88e1gb+iVoXBjrrREDoKyCkf0lUQc8GvKYIXSKc93YEC2G23JR1MbuD1hKem2g84cYXAoCtXWcfzk0CJf5loxa3Fa HnyXnT2K xB9yrvjajhBed5dBzWSA0qVtt0hyr+bsRlg3RVNZ77ozrlLC2l9q8Wz2I9hIvYZjm/5UU0qMFi8+OIkYtJJefdsy281IhA/OerfOmTjF/6xhsoEIgrjQ8nxPmxJCojf6jgMwnBcAN1d8Bc/T/xQlQWCyCmHQnD5LqNX/3EVzrcl342C9QCmGC1/+HlOkwpSVHQKhiQ6gBaCBjbJtnR3iJZNeu32vr5cBfIHS1ldu4866TmEIoXcOnBobHHiWmd65XtmZTjgtpgmiGfTM6sNbDY3PFn2TeGb39EALuPXzVBwuQ7hw0/bEtxwN7kxM8KZDppS3WkS54yJqSxWfV5bFsuon6gviMadZyvEliJMhCJys+Ro4tdv+CyI4Bofsp5g1EzICti1OgOxDCH6iN6Sm5sc7+JiFuPJBEKaK3fqYzjGRauGSAEfcZjSaXwHIc0IK5h2hALdmioipkC1+Py1xEvefB/Ah2bS7Mdm7UOqcsvNUFiq7g4u4vNTJlS1YdzTh5WfOVaE111OfEQU71MCzo5vZBeNQPry3x3to5LJ07abCRqO7++VhEUAgZwZ6Vjv0xlIUbtFvkLV5SY2LfYLtcDAR+qVZrkz5CcesPAgy6fu8eI4HMaiQk3VDImrRLiS0VGqTydPjoT9TbMfkyEM/IInfh4+fAFdK64JrUk7dQSdgEXhOEZlT++XbxP8hm/huOXzWP+lSSIyV9JcIn2K3tdd+uTha4iv0i4JpCgOmTDHVxpCnRc3SoqVUU21960Ih26RXyD8h5WpkewslR3uQMSH2dM7N5Crgl7nFB X-Bogosity: Ham, tests=bogofilter, spamicity=0.000230, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: This is a multi-part message in MIME format. --------------7z1PBV0p7CDcCIp8PJ0wzFUG Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 4/14/25 2:10 PM, Johannes Weiner wrote: > On Mon, Apr 14, 2025 at 08:01:42PM +0200, Michal Koutný wrote: >> On Mon, Apr 14, 2025 at 12:47:21PM -0400, Johannes Weiner wrote: >>> It's not a functional change to the protection semantics or the >>> reclaim behavior. >> Yes, that's how I understand it, therefore I'm wondering what does it >> change. >> >> If this is taken: >> if (!mem_cgroup_usage(memcg, false)) >> continue; >> >> this would've been taken too: >> if (mem_cgroup_below_min(target_memcg, memcg)) >> continue; >> (unless target_memcg == memcg but that's not interesting for the events >> here) > D'oh. > >>> The problem is if we go into low_reclaim and encounter an empty group, >>> we'll issue "low-protected group is being reclaimed" events, >> How can this happen when >> page_counter_read(&memcg->memory) <= memcg->memory.emin >> ? (I.e. in this case 0 <= emin and emin >= 0.) >> >>> which is kind of absurd (nothing will be reclaimed) and thus confusing >>> to users (I didn't even configure any protection!) >> Yes. >> >>> I suggested, instead of redefining the protection definitions for that >>> special case, to bypass all the checks and the scan count calculations >>> when we already know the group is empty and none of this applies. >>> >>> https://lore.kernel.org/linux-mm/20250404181308.GA300138@cmpxchg.org/ >> Is this non-functional change to make shrink_node_memcgs() robust >> against possible future redefinitions of mem_cgroup_below_*()? > No, this was really just aimed to stop low events on empty groups. > > But as you rightfully point out, they should not get past the min > check in the first place. So something seems missing here. I think I saw some low events in the !usage case was because my original patch was to remove the '=' from mem_cgroup_below_low() and mem_cgroup_below_min() which made it past the mem_cgroup_below_min() check. Without touching mem_cgroup_below_min/low(), the addition of mem_cgroup_usage() in shrink_node_memcgs() is probably redundant. I can remove it from the patch. Thanks for the detailed review. Cheers, Longman --------------7z1PBV0p7CDcCIp8PJ0wzFUG Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit


On 4/14/25 2:10 PM, Johannes Weiner wrote:
On Mon, Apr 14, 2025 at 08:01:42PM +0200, Michal Koutný wrote:

On Mon, Apr 14, 2025 at 12:47:21PM -0400, Johannes Weiner <hannes@cmpxchg.org> wrote:

It's not a functional change to the protection semantics or the
reclaim behavior.

Yes, that's how I understand it, therefore I'm wondering what does it
change.

If this is taken:
               if (!mem_cgroup_usage(memcg, false))
                       continue;

this would've been taken too:
                if (mem_cgroup_below_min(target_memcg, memcg))
                        continue;
(unless target_memcg == memcg but that's not interesting for the events
here)

D'oh.


The problem is if we go into low_reclaim and encounter an empty group,
we'll issue "low-protected group is being reclaimed" events,

How can this happen when
	page_counter_read(&memcg->memory) <= memcg->memory.emin
? (I.e. in this case 0 <= emin and emin >= 0.)


which is kind of absurd (nothing will be reclaimed) and thus confusing
to users (I didn't even configure any protection!)

Yes.
 

I suggested, instead of redefining the protection definitions for that
special case, to bypass all the checks and the scan count calculations
when we already know the group is empty and none of this applies.

https://lore.kernel.org/linux-mm/20250404181308.GA300138@cmpxchg.org/

Is this non-functional change to make shrink_node_memcgs() robust
against possible future redefinitions of mem_cgroup_below_*()?

No, this was really just aimed to stop low events on empty groups.

But as you rightfully point out, they should not get past the min
check in the first place. So something seems missing here.

I think I saw some low events in the !usage case was because my original patch was to remove the '=' from mem_cgroup_below_low() and mem_cgroup_below_min() which made it past the mem_cgroup_below_min() check. Without touching mem_cgroup_below_min/low(), the addition of mem_cgroup_usage() in shrink_node_memcgs() is probably redundant. I can remove it from the patch.

Thanks for the detailed review.

Cheers,
Longman

--------------7z1PBV0p7CDcCIp8PJ0wzFUG--