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 1C66FC71136 for ; Thu, 12 Jun 2025 14:13:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B0BB46B007B; Thu, 12 Jun 2025 10:13:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AE3426B0088; Thu, 12 Jun 2025 10:13:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9FA6D6B0089; Thu, 12 Jun 2025 10:13:49 -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 7CE3C6B007B for ; Thu, 12 Jun 2025 10:13:49 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 47348BF31F for ; Thu, 12 Jun 2025 14:13:49 +0000 (UTC) X-FDA: 83546942178.09.C222612 Received: from out30-119.freemail.mail.aliyun.com (out30-119.freemail.mail.aliyun.com [115.124.30.119]) by imf24.hostedemail.com (Postfix) with ESMTP id E9FE6180009 for ; Thu, 12 Jun 2025 14:13:45 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=pnZ52KGK; spf=pass (imf24.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.119 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1749737627; 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=8dSW0MWx250BT5PDHKGtg7MyrmkvYYx3NnjczjhuDu4=; b=mPHn2q1qzADTIRYmPe4fGErKyy+Fg6uKYwhjRsSaGWdLigS5HcfiWfZLpf14OYapOCLfTP wfwCNmUm6HgLGMvnYkk608cJswgLip+4F9aEiHiBxEkpWMmZdbM10i6Ced0z0+UXEQFBc9 Ryvh6cb4/BY/ZVs5UUSQw91cnlTiQKs= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=pnZ52KGK; spf=pass (imf24.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.119 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1749737627; a=rsa-sha256; cv=none; b=aS24Vz7QrgPSWttuCFXglmqTmEhLrD9w2Op8/nWmS3k7o1yMUjvacACNnRJf/PdC0ZlKZc 9j8g/vKrk/ulyHz9aX4vH0v6KXUAuMI66acmrqbGYQzm7V1rlZTszOkHDJMYe5qnm/QfPc VyNXrPxGtmXnvhFeeEP9a78hBikwPn8= DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1749737622; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=8dSW0MWx250BT5PDHKGtg7MyrmkvYYx3NnjczjhuDu4=; b=pnZ52KGK8IFZuOD8/bJv3kOJ4v2SMQPzddAiqFPx+MTclatjOa/cQV/wdJoi57JFQ2qOtmmTDcriuispo9q32Z1RQl5POPR77GzlkvTbATgi71WawI2xGHtyFjv4kEADfNA6vFzenDUxSQOaT1hFfAlJcWGq0AxRf2mQy0Jk/cA= Received: from 30.39.247.88(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0WdhHBU7_1749737621 cluster:ay36) by smtp.aliyun-inc.com; Thu, 12 Jun 2025 22:13:41 +0800 Message-ID: <519c59e6-20ef-4edf-88b8-a71f38cbd2de@linux.alibaba.com> Date: Thu, 12 Jun 2025 22:13:40 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/2] mm: huge_memory: disallow hugepages if the system-wide THP sysfs settings are disabled To: Lorenzo Stoakes , David Hildenbrand Cc: akpm@linux-foundation.org, hughd@google.com, Liam.Howlett@oracle.com, npache@redhat.com, ryan.roberts@arm.com, dev.jain@arm.com, ziy@nvidia.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <8eefb0809c598fadaa4a022634fba5689a4f3257.1749109709.git.baolin.wang@linux.alibaba.com> <1ec368c4-c4d8-41ea-b8a3-7d1fdb3ec358@redhat.com> <2ff65f37-efa9-4e96-9cdf-534d63ff154e@linux.alibaba.com> <953596b2-8749-493d-97eb-a5d8995d9ef8@redhat.com> <97a67b74-d473-455e-a05e-c85fe45da008@linux.alibaba.com> <201a1cc4-93fc-48e3-aeab-759ba8c8a47c@lucifer.local> From: Baolin Wang In-Reply-To: <201a1cc4-93fc-48e3-aeab-759ba8c8a47c@lucifer.local> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: E9FE6180009 X-Stat-Signature: pbgzffw7sfk9gmrntccaqyuob1hg1qey X-Rspam-User: X-HE-Tag: 1749737625-575245 X-HE-Meta: U2FsdGVkX19HKawWfJIAoZNNwJQ0jLZi501+t9OxVAuc9fkAaQ1fvCbnOe62kgOV2YbBaxSgqgS55Xgmql2W3aQwMrpQBxNlnrSRpkt9pUKw8fl3K+nivKyuUSnN24+gWbfopm9ClRgcSsaW81hQaAbEyVVIx36ffdmvjfuU1knPtKKHNX8YrVy4hlRZpgkkdFIUJyGRoyt28nvseu7nNKMgyz78HSI1GfAZ1G1xqo9Y5PXKKkxRWAi7hd/K93a2ryCnRx1ZuRNth3f2ebjwL9cKlxxAyBTnCQvYDECJuaOzzv9bZDkiYA4kwD+qxybXjWQu/dVVlqLiTMYohWQs3/o1NgsYa4cgn4fRqC1SBXpB64m1itbcBKpq9Mml7V78qqqsns2aCODFM76nW7pt8sS4ocsoCbDmCO3QcWSTTAtnp39rgp04zefIEMBuxHCm4nIsoMsmWQhx8xTbNOtwis/dciPNRT1o81JpxhAj6YbH6eKRKRsiI3/RPHC4ZFPUl2Am2L7/JsmnEtTUv8pNgyYlbdWxu6PA2dazt0RuAdOxEciOwNlu/1AiR3PQBvWpcP9s8L/kvv3V3oV6scgSLZryJAAqY/ANuLMG9jjehiVD9W7bVFORKth1o3l6foD/6KSsWOEkFZf1zF/UqHdWuwTqbwRFuqPXgqNevQOPBaRx0nSMdlhZUCZa2Rt/XIn0XftAhJqSVYDDTFebTtKMsmrhH3SDL0LCfkSoLkz8wG9rux11la2p5g7JxfpLMV4IpFhdUpvbgHt8Cydatt7DcWOJuFfokP5GEZUh230bofSiDJj09vaK9Hup0MwnKFI4xqofeZjHA2/z1zT6Hd4gaHi9kaAR0PiYMuR9YenEbb+N89IWGPRbBqSm7KcHMiVb5VuJrfd176ZlFxxDrNABEw43Xv0W1uhYoTQzigA7MfymK9YT6pyRBelO8HxzlK5GqqJqywbzEcMIFIikclr anCQcC+I iQBwfmxvStM1huoRgKUQS0gf/dvs/6HGkkNXjXky00xLqy4XrjH/r8fwoU7MVKrUdtnfhir3tbiFcTcGlyTmEGR9/qDNR3ad4vbVVnkKFZIWuDMxhf9v8RkhivKuNQJbgAB80XkVy8g1NZceU354NJTDOGN6qG+u3K/7nqeQt+aLtPuJZZMZaShTA+Rmv00m2Y7zCuHHd+m/LN2rcgrIaF4gI7qVu2J8yLZ+ehauteyl4/LnNEMsMf+RtPSoWs1q9zsOQgBBeWMsKVEHpd1TGyN1boZA9h+xxYX3gWJw8tPrWDRYABxIhEmEy3roJI38nGyVSYcWsbO+EAGWRRxHIUZWxQedPliuDfNC82AbMc9AIETGH2Ugdzg879Q== 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 2025/6/12 21:29, Lorenzo Stoakes wrote: > On Thu, Jun 12, 2025 at 02:27:06PM +0100, Lorenzo Stoakes wrote: > [snip] > >> I propose a compromise as I rather like your 'exclude never' negation bit. >> >> So: >> >> /* Strictly mask requested anonymous orders according to sysfs settings. */ >> static inline unsigned long __thp_mask_anon_orders(unsigned long vm_flags, >> unsigned long tva_flags, unsigned long orders) >> { >> const unsigned long always = READ_ONCE(huge_anon_orders_always); >> const unsigned long madvise = READ_ONCE(huge_anon_orders_madvise); >> const unsigned long inherit = READ_ONCE(huge_anon_orders_inherit);; >> const unsigned long never = ~(always | madvise | inherit); >> const bool inherit_enabled = hugepage_global_enabled(); >> >> /* Disallow orders that are set to NEVER directly ... */ >> orders &= ~never; >> >> /* ... or through inheritance (global == NEVER). */ >> if (!inherit_enabled) >> orders &= ~inherit; >> >> /* >> * Otherwise, we only enforce sysfs settings if asked. In addition, >> * if the user sets a sysfs mode of madvise and if TVA_ENFORCE_SYSFS >> * is not set, we don't bother checking whether the VMA has VM_HUGEPAGE >> * set. >> */ >> if (!(tva_flags & TVA_ENFORCE_SYSFS)) >> return orders; >> >> if (hugepage_global_always()) >> return orders & (always | inherit); >> >> /* We already excluded never inherit above. */ >> if (vm_flags & VM_HUGEPAGE) >> return orders & (always | madvise | inherit); > > Of course... I immediately made a mistake... swap these two statements around. I > thought it'd be 'neater' to do the first one first, but of course it means > madvise (rather than inherit) orders don't get selected. > > This WHOLE THING needs refactoring. Personally, I think the 'exclude never' logic becomes more complicated. I made a simpler change without adding a new helper. What do you think? static inline unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma, unsigned long vm_flags, unsigned long tva_flags, unsigned long orders) { /* Optimization to check if required orders are enabled early. */ if (vma_is_anonymous(vma)) { unsigned long mask = READ_ONCE(huge_anon_orders_always); bool huge_enforce = !(tva_flags & TVA_ENFORCE_SYSFS); bool has_madvise = vm_flags & VM_HUGEPAGE; /* * if the user sets a sysfs mode of madvise and if TVA_ENFORCE_SYSFS * is not set, we don't bother checking whether the VMA has VM_HUGEPAGE * set. */ if (huge_enforce || has_madvise) mask |= READ_ONCE(huge_anon_orders_madvise); if (hugepage_global_always() || ((has_madvise || huge_enforce) && hugepage_global_enabled())) mask |= READ_ONCE(huge_anon_orders_inherit); orders &= mask; if (!orders) return 0; } return __thp_vma_allowable_orders(vma, vm_flags, tva_flags, orders); } > >> >> return orders & always; >> } >> >> What do you think? >> >> >>> + return orders; >>> +} >>> + >>> /** >>> * thp_vma_allowable_orders - determine hugepage orders that are allowed for vma >>> * @vma: the vm area to check >>> @@ -287,16 +323,8 @@ unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma, >>> unsigned long orders) >>> { >>> /* Optimization to check if required orders are enabled early. */ >>> - if ((tva_flags & TVA_ENFORCE_SYSFS) && vma_is_anonymous(vma)) { >>> - unsigned long mask = READ_ONCE(huge_anon_orders_always); >>> - >>> - if (vm_flags & VM_HUGEPAGE) >>> - mask |= READ_ONCE(huge_anon_orders_madvise); >>> - if (hugepage_global_always() || >>> - ((vm_flags & VM_HUGEPAGE) && hugepage_global_enabled())) >>> - mask |= READ_ONCE(huge_anon_orders_inherit); >>> - >>> - orders &= mask; >>> + if (vma_is_anonymous(vma)) { >>> + orders = __thp_mask_anon_orders(vm_flags, tva_flags, orders); >>> if (!orders) >>> return 0; >> >> I pointed out to Baolin that __thp_vma_allowable_orders() handles the orders == >> 0 case almost immediately so there's no need to do this, it just makes the code >> noisier. >> >> I mean we _could_ keep it but I think it's better not to for cleanliness, what >> do you think? >> >>> } >>> >>> >>> -- >>> Cheers, >>> >>> David / dhildenb >>> >>