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 34131D15DA8 for ; Mon, 21 Oct 2024 14:52:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9D4656B007B; Mon, 21 Oct 2024 10:51:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9847D6B0083; Mon, 21 Oct 2024 10:51:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 84CDF6B0088; Mon, 21 Oct 2024 10:51:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 627A26B007B for ; Mon, 21 Oct 2024 10:51:59 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 9F391C19C5 for ; Mon, 21 Oct 2024 14:51:42 +0000 (UTC) X-FDA: 82697898444.03.7343C66 Received: from mail-lj1-f177.google.com (mail-lj1-f177.google.com [209.85.208.177]) by imf01.hostedemail.com (Postfix) with ESMTP id 417F440019 for ; Mon, 21 Oct 2024 14:51:45 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aS7Sq7vq; spf=pass (imf01.hostedemail.com: domain of joshua.hahnjy@gmail.com designates 209.85.208.177 as permitted sender) smtp.mailfrom=joshua.hahnjy@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729522166; 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=5eABMIx9lnMto4OhJZ3h1jo2zuiX/EYRQgb1Gk/pqN0=; b=EKR+DFkmfAH1CcoVoGS5/l+3gbt5URnHUwcrDpAkWKM04mi86O1vm1jVOJtzAyzOsiwMro AuSR+EQIYQjaG9BkKzwx4UcfYab874FGNUKA1ZF6nIO2ncxbNLWG++VICL5RYJRCwnayAU i5q1ua7febsMMkwbCtkB0ZIGeB0aMwo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729522166; a=rsa-sha256; cv=none; b=pUE6UDeCvo+pz8yKmaBjRWlDLb75qUD212dsR1nEXmRvsHq5jC7NeAes4JJJdIWF9ACsc+ SBqWblN1GckDJAyz3q1i+yKu2tE0y9LdiEMNIrjmQmkJ4Q0B3kXta1XgYMKnXKTPsHW2kj a0cxsqYorTmQu3aK9yCwXJTyRqpXhao= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=aS7Sq7vq; spf=pass (imf01.hostedemail.com: domain of joshua.hahnjy@gmail.com designates 209.85.208.177 as permitted sender) smtp.mailfrom=joshua.hahnjy@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-lj1-f177.google.com with SMTP id 38308e7fff4ca-2f7657f9f62so43360161fa.3 for ; Mon, 21 Oct 2024 07:51:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729522315; x=1730127115; darn=kvack.org; 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=5eABMIx9lnMto4OhJZ3h1jo2zuiX/EYRQgb1Gk/pqN0=; b=aS7Sq7vqTERH7dQ+ugM+Lz0GXmNcrOlRMNsbxz2T2jWYbwVWePsL84nastOqIs6pTB Hty3ekoSfoiAXoYXNkFDuYceb2R6GNRbDKcGv+0+9MqCgW81hX2RX7Vekyses1uJQ8oI 12RdIPsNDWYZgAtBz3S2Tl9MZaLW+l2vFfWwGKYMXuoCj3JvuWWr4utd3anahgxIXq8X 23vCSFOcdq6AustAZvpFZgqzj//rbnu6ndPKWckjUkPjb3Le1IaqmqCwdmHGjMzbHNqG fSGWvTS9f1RGLBed9oYatYFJeqeEGueakRbE6MwEM+rPbiO1/b1NYVCLJJm2KjkyNFVo bWkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729522315; x=1730127115; 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=5eABMIx9lnMto4OhJZ3h1jo2zuiX/EYRQgb1Gk/pqN0=; b=MpkO7NKOGDc25VXHSprGOhbUVVn0u5oPJpUlYjjm7XuEWvkOnosoh/SZyM8b7EfB5M 7Cq9/oHljJDa+RReQUKlePUT/97379i8Db7bUNXYznFwvXixhqj/dlS6vs7GXdl+9zVz 0a1BzxHeanzr2MNTN+Vg4WNDjXhd2DXErhaefpfKCQ3yoCx8BWydAG1S0MHq8ODhkmPV N7X/hqpqhohWplyTDS+IhQ5OQOK+UQC99KZ6S9GfON6FiE4swS/OH6NfrSYRLPDdMxyc vvPACSS7Aryn33yYv4ZuOf8Jc8zP7GUXWY/n1dt47NSKpgYTrt/glyop3AiZGr4ZgjPD WZPQ== X-Forwarded-Encrypted: i=1; AJvYcCUPT/Bj2zCucsmgviw3S6VIi/6C0LiJMT6c8EfudveYje2nFXS08CmgzWVuAkcmnFRJgATZPInhww==@kvack.org X-Gm-Message-State: AOJu0YxGbrbIrXt9d+z7SJT/sfkUqZCOpQJnh1YLVNtS2CEkNmMVhE6Y tbGTr6MYqyQLytZ5p3iThiJSqHsT7G5G7VfXFBpMyH2IOBp1Jx7IKy6uWZixNuZB0LTRYwOLT6i 6kynWvrBuyQ2CVd22OuOS7CEdrss= X-Google-Smtp-Source: AGHT+IFXPlzL373ISjpa3KTVBnHtlsi6xEMamxxcHC6fm0VA8xoQMx7OABNdmlt0t9yTrBxGrENWyIGVCihHQEte1SE= X-Received: by 2002:a2e:b987:0:b0:2fb:3bc0:9c7c with SMTP id 38308e7fff4ca-2fb82eb3506mr45509321fa.25.1729522315068; Mon, 21 Oct 2024 07:51:55 -0700 (PDT) MIME-Version: 1.0 References: <20241017160438.3893293-1-joshua.hahnjy@gmail.com> <20241018123122.GB71939@cmpxchg.org> In-Reply-To: From: Joshua Hahn Date: Mon, 21 Oct 2024 10:51:43 -0400 Message-ID: Subject: Re: [PATCH 0/1] memcg/hugetlb: Adding hugeTLB counters to memory controller To: Michal Hocko Cc: Shakeel Butt , Johannes Weiner , nphamcs@gmail.com, roman.gushchin@linux.dev, muchun.song@linux.dev, akpm@linux-foundation.org, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, lnyng@meta.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: u84n98p6jqngdec1i8ju57d1onpf5oe4 X-Rspamd-Queue-Id: 417F440019 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1729522305-234385 X-HE-Meta: U2FsdGVkX1+UHK7hbJRKqcSkOybDbUlHhP/MJJX8mvuKFnCJHymuVQZyPfaUUrmhPUKnIdBwEqsgodzeIZ5pJ916QtbT2mYLg/njnUhitTDqGf+MNcX+CS7O5Loac39k4/KG6z9/oaX3KZZU2Cybs1JxGNicGb5C87ANXLdAhbNj8DV42VdIbp3ezag05pSPJ+Z0HgRTLykBIhrFKomp7wMblzSTpg+qmPwNzrM2H28j51HW13cDaZSjxXSVwprU2B1zxqfJxE8sD17Hg9DCJSx0hjwnqhZWxPWzHwek8lnH++oiOO1D5tPp1YRsFHxG6DjDztL3fU2rbRZt6s/95q13J6qUJgeaBdcLhHze+v8JQhKHkj09t3LkPMn1h8NW9ZPTJDTZUzhMAca7d+MN0dTBaI1U+vGMlS/101gV/+PYjwTQhZROHpAbvlOMEUwr8uJDo6D3r7mDB3+tt0t1GY45wMA0onFR19N2q8/sQEDmjRsA8oppZQtqbyZnYX8sB9WktFKi+7PAH+lkxz/XhcxOvnqtuf6gJg9GzriqATNhXywFmd25srxdO+vOiRCeXOkizZK9E5g+IndnpWeHxYaoaucoR3Kxb9SMpmfPRmis2/sQQIiVOpkaDJFzRPmqU6GzMwmhsfkzVddl5fOPTBefsgRRhpOTMYB7wtQP6mKSGDcL8RsKQqj5KZRAVAnYW6gkJWvwIDFEvEDpNMbhAydwuM2atPwRvkaKmrD/kKc6z7DkrYED5LeRpFnAtOUAaeqHgf04BQaMqBzDsOp+etlrFuu620o3ZiCVzg5MLKctah9rC+ug/ktaTL1y6uSNcI80a/e6GxVZPJvD/zQnCmizguDZ/70BvO+giUIfWpoM21K2GfUVZNm1sp0e1hb2OReArbU+WTmwOxq2FoCkZeZ5I2/JNQYelWQZnQWgbUIISa6Yey+wi1HofN3LAi5WyMJ+9hWi8K2iAyfun9H wCHS1yOU V7Jr9KmTdVwrFn2kKmM3voEf9Ypf8AQ6fp11B2TH6qmIyfb3/fdsJr8lZgR7SM9LMTaygTBe7PiyKyl77lUWdv1ilt/MkyJPZtWVE3DIPYXAv3kvTV3P4hfoQX1zocooWv6VNyBgEve2oxuf7F/Cx8NnUAYtgt6M32jehW+4rzWHHYUYMcoDFjALTPCOQXEgj1OSFfroWLVg6dilNhiMAmpWOfFBlLX8zHKdmQcVCOG0nGc7Mf/U7Cu30yuA9sA4E/QN8BgQxtKFe1bpZd7DXiVhXEIwNgrsiPBWlf9t8EtpPYAE+1lXcBU5gaVzQQN4ORZH+B2U+W9apxHXemZd8WVyng3VNpAhIqbZ2CNmgQkevDVLYlCJ/VTkTgkCuNlQ1erPGiAnXkH1iJWbocsN+H0w9n7rU900RaPLM 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 Mon, Oct 21, 2024 at 3:15=E2=80=AFAM Michal Hocko wr= ote: > > On Fri 18-10-24 14:38:48, Joshua Hahn wrote: > > But even if we are okay with this, I think it might be overkill to > > enable the hugeTLB controller for the convenience of being able to insp= ect > > the hugeTLB usage for cgroups. This is especially true in workloads whe= re > > we can predict what usage patterns will be like, and we do not need to = enforce > > specific limits on hugeTLB usage. > > I am sorry but I do not understand the overkill part of the argument. > Is there any runtime or configuration cost that is visible? I think an argument could be made that any amount of incremental overhead is unnecessary. With that said however, I think a bigger reason why this is overkill is that a user who wishes to use the hugeTLB counter (which this patch achieves in ~10 lines) should not have to enable a ~1000 line feature= , as Johannes suggested. A diligent user will have to spend time learning how the hugeTLB controller works and figuring out the settings that will basically make the controller do no enforcing (and basically, the same as if the feature was not enabled)= . A not-so-diligent user will not spend the time to make sure that the config= s make sense, and may run into unexpected & unwanted hugeTLB behavior [1]. > TL;DR > 1) you need to make the stats accounting aligned with the existing > charge accounting. > 2) make the stat visible only when feature is enabled > 3) work more on the justification - i.e. changelog part and give us a > better insight why a) hugetlb cgroup is seen is a bad choice and b) wh= y > the original limitation hasn't proven good since the feature was > introduced. > > Makes sense? > -- > Michal Hocko > SUSE Labs Hi Michal, Thank you for your input. Yes -- this makes sense to me. I apologize, as it seems that I definitely left a lot to be assumed & inferred, based on my original patch changelog. In my next version of this patch, I am planning to add the changes that hav= e been suggested by Johannes, write code with the try_charge cleanup that Shakeel suggested in mind, and change the behavior to make sense only when hugeTLB accounting is enabled, as you suggested as well. I'll also update the changelog & commit message and add any information that will hopefully make future reviewers aware of the motivation for this patch. Please let me know if you have any remaining concerns with the implementati= on or motivation, and I will be happy to incorporate your ideas into the next version as well. Joshua [1] Of course, this argument isn't really generalizable to *all* features, we can't make a separate config for every small feature that a user might want to enable with the smallest granularity. However, given that the exist= ing solution of enabling the hugeTLB controller is an imperfect solution that still leaves a discrepancy between memory.stat and memory.current when huge= TLB accounting is enabled, I think it is reasonable to isolate this feature.