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 11CE7C3DA49 for ; Fri, 26 Jul 2024 11:00:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8E84A6B00A7; Fri, 26 Jul 2024 07:00:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 897C46B00A8; Fri, 26 Jul 2024 07:00:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 760D26B00A9; Fri, 26 Jul 2024 07:00:52 -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 5B6076B00A7 for ; Fri, 26 Jul 2024 07:00:52 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id EB570A69F3 for ; Fri, 26 Jul 2024 11:00:51 +0000 (UTC) X-FDA: 82381611102.10.57DB36F Received: from mail-ua1-f52.google.com (mail-ua1-f52.google.com [209.85.222.52]) by imf01.hostedemail.com (Postfix) with ESMTP id 07E8640023 for ; Fri, 26 Jul 2024 11:00:49 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=mbETM8Yr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.52 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721991594; 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=6XYZNhl+Yme+X3WfgiEKKEcqAduYmi2Zfs8O87+eZtU=; b=alnFGxaT+XweBN3S5S0l7uh9qfeXthqQ6Gic4Ql66apjl+rau3VTjONiQ2TN14i2qZ6AKd fuCxxKz0aFpsLESp2sRcxhwflnCV7rwXvJSSsTw8hSmjbCTtX0g67Vv4xzMHSVmLnS10Mf s0cN/uhfthMzAqCND+7KsEfr5n8fvJ0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721991594; a=rsa-sha256; cv=none; b=OObuNZxC7tQ1n2mJJh/VKfIonPV+0CohP9jXcFP2aeLMdxbC82ZXAnYkpuOYdvN1uuadsk n1Aeqqbm8+6F+75t7g6RFf22g5mzN6TpiN00a6/XYWei0pgA0X69GCYVi/L/mA6k9RF2CU jQYTe9hGHiJBG9AVQ9iLv5pWw7gRz2c= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=mbETM8Yr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.52 as permitted sender) smtp.mailfrom=21cnbao@gmail.com Received: by mail-ua1-f52.google.com with SMTP id a1e0cc1a2514c-82175064454so200654241.2 for ; Fri, 26 Jul 2024 04:00:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721991649; x=1722596449; 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=6XYZNhl+Yme+X3WfgiEKKEcqAduYmi2Zfs8O87+eZtU=; b=mbETM8YrAgzpzo/RJm7hBTseaVWkbwzJnqTvH07FAPkWFt6GkIvQPiGJtgWgNlsLhf gpliTqfjw3cMamX9Rptixai2yXRZoUzeMjqIGXB0m3OryG9tb79W9WvOyLG2Fjk/tr0X eMbxbfpf3UafwQpufa3t2JQ/udqWhyrz574ehfumYyb0Gr+5r37WeMB1UK4hgOB2942X QK3u0aCqW8cuTFY0k1Iq8y52cxZSD81wPxK9h/1zc6SjBeTygO/dZXhXLYKC82IFWwW4 f8iWF3+ACTut2grhLE6DXtONiLFyCnzqMIhdfZKxDv8b+PWSkF8R9MSatGi23iER0ahx Lb1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721991649; x=1722596449; 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=6XYZNhl+Yme+X3WfgiEKKEcqAduYmi2Zfs8O87+eZtU=; b=jcJu/1nzk9+coSh8oNsFWChaHeHlylQU7gSwaLeWvAoxVE35ELEtW9FWWpvmDXG3kD S7bhwqspdsi86oXm6JPVjqF2aMAOvRo5MI4yaRBmjBTQLCxPnaK7TGwhi4PBQk4yQAIt lryrzCXZxGKgvX5qnDu1vWtXKwD5zhDj5Zljz8lyjdwP+hjDNNcJPlLcGZ/izOlfHVaR R4O7dhDCdBta1D9VOuklUW9rDaYqRhAHyKzFvIAF9AS0DePemiDcPZTPiqkFJq27TKDq 5BuG9XIjBNsu79oqx0FAfugdu1cj6DOFxfWb7OoPCQx0joUW+vfaC59PYhY6fZrfhvVh dGgQ== X-Forwarded-Encrypted: i=1; AJvYcCXhRniuZjMpWZ66ECSffkpNMHi+Xw3BzTEfxEfdXhLXbhzlAKlAPz95eDNep0D3kfaDNBiF7uap9YjymCscJh3XQRc= X-Gm-Message-State: AOJu0Yxqo9gMi6+A3A3h4tX4xCo2tW8vvjZ9GMlMiXXJ6/ABm2C/cW/J c9z1wfUB+JdnFagv5dSrdX3h2SaBOVvOncwjgeCv/KOa5z4DM2XwWgwizM5u/2MiPqZ3XV/Dpv2 tg6JkeHkAIVY8/VN7/EfwzE0Z/sA= X-Google-Smtp-Source: AGHT+IH0bNJVtUQX7W2OOtHhUPLoQ95PN08Igg9J6vofkYHuC+lp9Nu1CbLdHYyppJ8XrZdJ5JMgBlM9ndnruv0cJ/k= X-Received: by 2002:a05:6102:50aa:b0:493:e1ba:123b with SMTP id ada2fe7eead31-493e1ba1b33mr3801058137.16.1721991648966; Fri, 26 Jul 2024 04:00:48 -0700 (PDT) MIME-Version: 1.0 References: <20240726082818.260008-1-21cnbao@gmail.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Fri, 26 Jul 2024 23:00:37 +1200 Message-ID: Subject: Re: [PATCH] mm: huge_memory: don't start_stop_khugepaged for non-PMD THP To: Ryan Roberts Cc: David Hildenbrand , akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Barry Song , Lance Yang , Baolin Wang , Yang Shi , Zi Yan Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 07E8640023 X-Stat-Signature: wtcjhuzurpgzij35d3oszgdehmocxrya X-Rspam-User: X-HE-Tag: 1721991649-401675 X-HE-Meta: U2FsdGVkX196B286XXxpJ54661w8SnvUrxIou5VFOB70ns5ezKqLxz5gbjXmFpGinlUrEeyWgnLaGE6UNPWYU6r/Ts6I+tLjOCgpBfUhZ98eCBYs/j+rD5bdceJO5nRZIX3jP6b14NBy6jszxM3+RV2CBxDy8f/f56Wf9ctMalXE71bfUOR93appzhke6NUErozvhqE4QKnK9DTpWDixU9c4H04809MzWCLPKfUfNCW7b5F5dYDZuuc91H/xrzpLBDb7LVkE463Nwi8imb4ae1zN/9TBvq4NlnUDt/cIA4wYlEUojtk6QeYsnI8C3x4sgpqeZV2bHqjOD14xlet+A7+HT2aWJX8UBo2LiRg6nRO050nDHrQTnu29E8J/b7KGu/PGpAKUvzUn0NeAq6Loyuv5CA6BbWZaRPaXIbf9p6yeaPwEDwSkQ/xrp9AVotgxBObIcgKbCYqiXQ1wsMIWZri9mfDS9dAFff+xaQV1rT6l5P2tlzVqmLnM1DvSx8V4PrhXZAI4A1/Gcjx2110PsIsFccEG2Ru5ZzyFHpMZYqGb5UYn/TytPIz8J8xxu29OWeky2UXFhJ5eI0txUXGtykPxjFlfoW3dlIAz3DtS4qa8AGZ8FrjPoB9hoEPlSTbnN7AhEsXsypHoytIf6tWzS1nzsdNPsztQIpGk8SpXjWzPMfw7LDRGl3exEUrRrsYEoQbTE++YXA8heQ/NhhJKLtaGIbH4doa5cv5WWe4VJ2UFto4jv2JyaNVJCYn3bSmDopxuBloUNNNqhtGxb/QIL/wakBGXcqTBXBLV5trdI+hSIwJvjYlyzbhGTEzRLBQbWc4CJhPm6e3ilzJdmu0RUSqoyA1+38Ds+3elSYq04sz7Pil6DooAesoE6LgVM0PWT/s1MVQ7HmYS3g20RCnlvSAR31WbhmIKeUWY2sxEYPGXD340U4G6+qrMuFv+FAdFhm0v5JfTlArC7w+/JWj waPRuRnW KxSGueE/cNbZ3HDElVEAGEAohiddxnLYsRG3f5oxDEOEVz7FzuQnLytQs1dj0Tu0GpkFtDEV5L2YkLQI+vPDGyZM3nNwE/1l8eas94gsKlzY9NuzgIrroud3P9y4LhKpKDy4mZgleUXNWQohEMZhLOGRN4IiJ8b4An/WxATOruAKFRw0yeubbFLpD7FnA2ttDkfqktlkx9z9tfwjB7cGIFLA8PcbId2ci6PQ7B6YSrdsVubqdw4ywp3gN8V/LPIcypYY1+G+lWgIUaUv6mNszwxPmMZI5H7+JD+C7LlrhnSuH3ksIMdyYnpspxgKLIx0QoOcaleTD4aWPzrYRiusG0nqGoNvoD6T/XMJ5TEpjBFEpnIiht856pzOsi15fItkgtYWmFGLcUufDsm5SrjDTmxwkxjncCC1E3184PUmn+4sO9AD46sa/i+aTrmGFuKqf4/1XujMJQW0g/nGeeeY2kirnj+aDf5EZwNunD1RYPIowNuBTXXBjPKbDmfXXS76aGxVb/l/PV5kjFZVFtBQb7Udymb2CMBvq/BqT2qc/OI11hPehjiRDW+T+4IxG9W38lI1n8SRWrAHa5XUlpMUABZ3Zci8q7TseTgo67MaSFajxrBF5I9icJJxwKuWXTP/gsxlQ6fDTvGAhvVAqD1jSbDQQBh9ivw8ZkLnY 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 Fri, Jul 26, 2024 at 10:45=E2=80=AFPM Ryan Roberts wrote: > > On 26/07/2024 11:04, Barry Song wrote: > > On Fri, Jul 26, 2024 at 9:48=E2=80=AFPM David Hildenbrand wrote: > >> > >> On 26.07.24 11:43, Ryan Roberts wrote: > >>> On 26/07/2024 09:28, Barry Song wrote: > >>>> From: Barry Song > >>>> > >>>> khugepaged will be automatically started when PMD-sized THP is enabl= ed > >>>> (either of the per-size anon control or the top-level control are se= t > >>>> to "always" or "madvise"), and it'll be automatically shutdown when > >>>> PMD-sized THP is disabled (when both the per-size anon control and t= he > >>>> top-level control are "never"). > >>>> > >>>> It seems unnecessary to call start_stop_khugepaged() for non-PMD THP= , > >>>> as it would only waste CPU time. > >>>> > >>>> Cc: Lance Yang > >>>> Cc: Ryan Roberts > >>>> Cc: Baolin Wang > >>>> Cc: David Hildenbrand > >>>> Cc: Yang Shi > >>>> Cc: Zi Yan > >>>> Signed-off-by: Barry Song > >>>> --- > >>>> mm/huge_memory.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c > >>>> index 41460847988c..bd365e35acf7 100644 > >>>> --- a/mm/huge_memory.c > >>>> +++ b/mm/huge_memory.c > >>>> @@ -514,7 +514,7 @@ static ssize_t thpsize_enabled_store(struct kobj= ect *kobj, > >>>> } else > >>>> ret =3D -EINVAL; > >>>> > >>>> - if (ret > 0) { > >>>> + if (ret > 0 && order =3D=3D HPAGE_PMD_ORDER) { > >>>> int err; > >>>> > >>>> err =3D start_stop_khugepaged(); > >>> > >>> Personally I see this as a bit of a layering violation; its > >>> start_stop_khugepaged() that should decide the policy for when to sta= rt and stop > >>> the daemon. thpsize_enabled_store() should just be calling > >>> start_stop_khugepaged() to notify that something potentially pertinen= t to the a > >>> policy decision has changed. > >> > > > > My impression is that it slightly deviates from the huge page documenta= tion in > > Documentation/admin-guide/mm/transhuge.rst. > > > > khugepaged will be automatically started when PMD-sized THP is enabled > > (either of the per-size anon control or the top-level control are set > > to "always" or "madvise"), and it'll be automatically shutdown when > > PMD-sized THP is disabled (when both the per-size anon control and the > > top-level control are "never"). > > But start_stop_khugepaged() doesn't unconditionally start khugepaged, it = takes > action based on hugepage_pmd_enabled() which only returns true if there a= re any > pmd sized THP enabled (currently looking at anon and file, but should als= o look > at shmem in future; that's a known bug that's been there forever). So I d= on't > think it is inconsistent with the documentation? My point is that, as a code reader, if non-PMD sizes are never involved with khugepaged, we can proceed without needing to check the lower-layer code. otherwise it is a bit confusing, especially since start_stop_khugepaged() unconditionally calls set_recommended_min_free_kbytes() for all sizes, regardless of whether they are PMD. However, I agree with your point that if non-PMD sizes are eventually manag= ed by khugepaged, then this approach is flawed. This is the downside of this chan= ge. > > > > > non-PMD size is not involved in khugepaged, but I agree the policy migh= t change > > in the future. > > > >> Agreed, skimming the subject I was under the impression that we would = be > >> fixing something here. > > > > working on another swapin_enabled and reviewing the enabled source code= . > > I don't need this startstop for all sizes in that case, so I made a > > quick adjustment > > to this part as well. If neither of you likes it, that's fine with me := -) > > > >> > >> -- > >> Cheers, > >> > >> David / dhildenb > >> Thanks Barry