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 2C1B5C433F5 for ; Fri, 6 May 2022 16:40:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6F5826B0072; Fri, 6 May 2022 12:40:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6A53B6B0073; Fri, 6 May 2022 12:40:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 56BE46B0074; Fri, 6 May 2022 12:40:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 4731F6B0072 for ; Fri, 6 May 2022 12:40:19 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 0E2C51CE7 for ; Fri, 6 May 2022 16:40:19 +0000 (UTC) X-FDA: 79435880958.05.92B42BD Received: from mail-qk1-f173.google.com (mail-qk1-f173.google.com [209.85.222.173]) by imf09.hostedemail.com (Postfix) with ESMTP id 238E6140033 for ; Fri, 6 May 2022 16:40:11 +0000 (UTC) Received: by mail-qk1-f173.google.com with SMTP id b20so6262366qkc.6 for ; Fri, 06 May 2022 09:40:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=tFroZ33tNBSAN1tcZa2Cw4zWBMi2iBtFH4Bzz4YsosU=; b=gtTyDx0/RS3tjJSqQS6Db1hZVZaTk0Ki/526Yh5Xt/UaYyO782yNQmGZdGvmIdYjOD 5IiIUOv4zBpSgDUygfM6Hyrs1yAsiapyToZ5GmlDYZQBBnFEDVD4kksTOgD7GyAmcokv aMZ3ZBRGll5DZ/y+B+z4HkocmXG+eWbXv3yZcYrgiZSpa9Q474eoXfvtnES8IjGL/JDk pzSjYhFzt7kBlE0hzi5CHrS6bIfHElPYKCJ5hdK/v8ndV/6DnytEr2F9DnRVRNho7oX6 25KzQtbjvbkf0eavRnpkdQuJxVEyeW6XP3tfPN3O7F5dmvP4bZj73YvG7LW0gbH/v709 Q7JQ== X-Gm-Message-State: AOAM532NWqXNdcQp0hbojpQ5V+GUQLW7ML6XUz6bd6DWW9PqrysIpaX+ RX0W9MwfzhEOhWDc5s2cZ1bA4V7CWHJ8AA== X-Google-Smtp-Source: ABdhPJxyETxODGJra6xsGRAaLJvcj4Q8KJz+eh5+aO7lgt55G5ib6uq66idy09jNJJ0Ht6o/l8Hj+A== X-Received: by 2002:a05:620a:4045:b0:69f:e555:3fdf with SMTP id i5-20020a05620a404500b0069fe5553fdfmr3016293qko.365.1651855217645; Fri, 06 May 2022 09:40:17 -0700 (PDT) Received: from dev0025.ash9.facebook.com (fwdproxy-ash-012.fbsv.net. [2a03:2880:20ff:c::face:b00c]) by smtp.gmail.com with ESMTPSA id o22-20020ac84296000000b002f39b99f6a0sm2717130qtl.58.2022.05.06.09.40.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 May 2022 09:40:17 -0700 (PDT) Date: Fri, 6 May 2022 09:40:15 -0700 From: David Vernet To: Michal =?utf-8?Q?Koutn=C3=BD?= Cc: akpm@linux-foundation.org, tj@kernel.org, roman.gushchin@linux.dev, linux-kernel@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org, hannes@cmpxchg.org, mhocko@kernel.org, shakeelb@google.com, kernel-team@fb.com, Richard Palethorpe Subject: Re: [PATCH v2 2/5] cgroup: Account for memory_recursiveprot in test_memcg_low() Message-ID: <20220506164015.fsdsuv226nhllos5@dev0025.ash9.facebook.com> References: <20220423155619.3669555-1-void@manifault.com> <20220423155619.3669555-3-void@manifault.com> <20220427140928.GD9823@blackbody.suse.cz> <20220429010333.5rt2jwpiumnbuapf@dev0025.ash9.facebook.com> <20220429092620.GA23621@blackbody.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220429092620.GA23621@blackbody.suse.cz> User-Agent: NeoMutt/20211029 Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf09.hostedemail.com: domain of dcvernet@gmail.com designates 209.85.222.173 as permitted sender) smtp.mailfrom=dcvernet@gmail.com X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 238E6140033 X-Rspam-User: X-Stat-Signature: t139ohp1cscawfenba8cnt77xex5du8c X-HE-Tag: 1651855211-693871 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: Sorry for the delayed reply, Michal. I've been at LSFMM this week. On Fri, Apr 29, 2022 at 11:26:20AM +0200, Michal Koutný wrote: > I still think that the behavior when there's no protection left for the > memory.low == 0 child, there should be no memory.low events (not just > uncounted but not happening) and test should not accept this (even > though it's the current behavior). That's fair. I think part of the problem here is that in general, the memcontroller itself is quite heuristic, so it's tough to write tests that provide useful coverage while also being sufficiently flexible to avoid flakiness and over-prescribing expected behavior. In this case I think it's probably correct that the memory.low == 0 child shouldn't inherit protection from its parent under any circumstances due to its siblings overcommitting the parent's protection, but I also wonder if it's really necessary to enforce that. If you look at how much memory A/B/E gets at the end of the reclaim, it's still far less than 1MB (though should it be 0?). I'd be curious to hear what Johannes thinks. > What might improve the test space would be to have two configs like > > Original one (simplified here) > parent memory.low=50M memory.current=100M > ` child1 memory.low=50M memory.current=50M > ` child2 memory.low=0M memory.current=50M > > New one (checks events due to recursive protection) > parent memory.low=50M memory.current=100M > ` child1 memory.low=40M memory.current=50M > ` child2 memory.low=0M memory.current=50M > > The second config assigns recursive protection to child2 and should > therefore cause memory.low events in child2 (with memory_recursiveprot > enabled of course). Something like this would work, though I think it's useful to specifically validate the behavior of the memcontroller when the children overcommit the parent's memory.low protection, which the current test does. So I'm inclined to keep this testcase, and add your next suggestion: > Or alternative new one (checks events due to recursive protection) > parent memory.low=50M memory.current=100M > ` child1 memory.low=0M memory.current=50M > ` child2 memory.low=0M memory.current=50M This definitely sounds to me like a useful testcase to add, and I'm happy to do so in a follow-on patch. If we added this, do you think we need to keep the check for memory.low events for the memory.low == 0 child in the overcommit testcase? It arguably helped to catch the SWAP_CLUSTER_MAX rounding issue you pointed out. Again, curious to hear what Johannes thinks as well. Thanks, David