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 46AA6CF9C6B for ; Tue, 24 Sep 2024 04:25:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AC3936B0082; Tue, 24 Sep 2024 00:25:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A4B8E6B0083; Tue, 24 Sep 2024 00:25:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8C57E6B0085; Tue, 24 Sep 2024 00:25:39 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 6CE296B0082 for ; Tue, 24 Sep 2024 00:25:39 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E7EB4817D4 for ; Tue, 24 Sep 2024 04:25:38 +0000 (UTC) X-FDA: 82598343156.13.8475A49 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf05.hostedemail.com (Postfix) with ESMTP id 34CEE100003 for ; Tue, 24 Sep 2024 04:25:37 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of dev.jain@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=dev.jain@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727151818; 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; bh=C19gbqoa8icFwS85nqqC9gfzox3JkGdPhtoSwMo5zh4=; b=qxEb/5Ly7qVofuA+LpWmU2fzs1o9hYYjxsyqq09wq6sF6ESAJBmtFyg1MjVnfR7HmqB2om PuBST9Ki6tlJWxnKCn4Df4ai2ucG627K5y/JOo6wWRxyuhluHxmpiQXNXM676RvO5rTmk1 7Aty1UjYeCFgFVCvuSXtqITyFCx75J0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727151818; a=rsa-sha256; cv=none; b=N6XCnKUtYJ65zPgOD0Dhk993lR0EVVhrsaiRU+u96vq5dqh5n8nUeVBGkm6zGeDHpNMiZA mB4v4+xWLcXs83I/c5IzS7Zqa/UHj3mmdyl8FBZT8azGsWo5bqFIkT7n5kP0/HxJirLvA0 cHxKtcG2olpPhl1O2JgmK12CYts3Qlw= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of dev.jain@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=dev.jain@arm.com; dmarc=pass (policy=none) header.from=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C4CF9DA7; Mon, 23 Sep 2024 21:26:05 -0700 (PDT) Received: from [10.162.43.15] (e116581.arm.com [10.162.43.15]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2D75B3F6A8; Mon, 23 Sep 2024 21:25:27 -0700 (PDT) Message-ID: <2200396e-9f70-41ac-b8cb-c207501a7770@arm.com> Date: Tue, 24 Sep 2024 09:55:24 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 1/2] mm: Abstract THP allocation To: David Hildenbrand , akpm@linux-foundation.org, willy@infradead.org, kirill.shutemov@linux.intel.com Cc: ryan.roberts@arm.com, anshuman.khandual@arm.com, catalin.marinas@arm.com, cl@gentwo.org, vbabka@suse.cz, mhocko@suse.com, apopple@nvidia.com, dave.hansen@linux.intel.com, will@kernel.org, baohua@kernel.org, jack@suse.cz, mark.rutland@arm.com, hughd@google.com, aneesh.kumar@kernel.org, yang@os.amperecomputing.com, peterx@redhat.com, ioworker0@gmail.com, jglisse@google.com, wangkefeng.wang@huawei.com, ziy@nvidia.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20240916094309.1226908-1-dev.jain@arm.com> <20240916094309.1226908-2-dev.jain@arm.com> <74089a2f-8918-4c01-8374-494bb8da906a@redhat.com> Content-Language: en-US From: Dev Jain In-Reply-To: <74089a2f-8918-4c01-8374-494bb8da906a@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: jpar9o444hfr7dy49iu84rrezibsh9i5 X-Rspamd-Queue-Id: 34CEE100003 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1727151937-844101 X-HE-Meta: U2FsdGVkX19MfH4VxDoKT1F+Rar9Xp8KFxxg8RDTluqqBmXbDL3Kh656tj+z+AhnzWqbq1LK7Ay4wIGQyWjWZ6lEwM3vYjtciYht3e2CvP8TlN5UQffIO2kOIsPwuQvBmeZplE5QGnM86Ysp8Q4BAscPuzHKmKl/BxNdql+LG3XFbC0y15rbrT/VcQ/kzzCG1Fal3941a+ROYJBduorVMHoRF5rnLnUlYjYF7yzarEn5QelFxf0akSEBDpUPFCbU5sMBf9KStdBBiUFbOgAh3jcXfX9luQEnlL/4iKbggK5pp9wmLFq879F5ZVRAw1gd/3VbLtVLuo7huKzEOW/dyneKZsR7re4zYfRh466aX9vn/+k/o6nz5VlXcR/Bs5f7DqIzxiB1nnfZ4I68hpjgESQ+1TeZWWqOkrdpCnfvJpPDVl7fHRI1hfD6/9CvYgwCmMrPI9gBBupgBwkbGtLN5GFEKB+Ph0IjicBd0Kd4ophcjUNUp+O/A2sIm9FU90sKapeyvpehVpbIVfFRfBWP9CgKHOremyiWy+Rs1MusuFevS+Av4Y+p+zzHHd+pn7fUIf94FJZASwPmT8K9+debokhEuXPamGcOoNJtVqU27Csu/1XucjiB0vjaucm7EzCcgYNGiKk9vD0b6KuvFl+E/eapAg0ZzAbmGdJK6C58B7huuSN3nOGpC821qH7shfhSzVzHQ+1QPvQbz8kyIrrdrI3dp/JsrZN32bx7zizO0WabwA9ftW75+b5rkctSm9I4TU3e/X2UrnirrdkmdOpfsPeeY7UajLXeW+b03TrZxoy/mECZd03IxySKm7gbOswHXn3Z5PT7RrCtLBbU1QU1plDwYPZCJdRdWK8RmsmqaUyKagcVD/unqy4o6yryV1+yDP6Rz3H3ofxOwsXX3fEuoM130PtCI+0vpXj6//X+qcfQCQOztU34w8xdCRJjmioRPGES/DzBOgdTr6TWT4C H/hVOZi/ 2+hp3qVCSL9ePEHXz8qc6I7aXyytI6maPf1XO1qA/8EPqNAadwiLNvi02Hl8suY99Lp/PcTZ0wcX7Dmsq/Nf9eyK44XBzpaDr4q2H6rmxorHneD/QxSmUrlpFh/mipPQgx82usfVnUYJySYgSSOwX2fIcOMuVK5WMpQr8dFA5W5oOfH/y3CVTJI5Ev7INl9+h5b1jzYqwsSHeIj8gxslZlWC6p1qnVhwhzmBWsw3NP4WfSHTe+g5ZLIaXUVIJpyNuSRcKNHrRyc1jKx0YISXT3vbvLA== 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 9/17/24 17:17, David Hildenbrand wrote: > On 16.09.24 11:43, Dev Jain wrote: >> In preparation for the second patch, abstract away the THP allocation >> logic present in the create_huge_pmd() path, which corresponds to the >> faulting case when no page is present. >> >> There should be no functional change as a result of applying this patch, >> except that, as David notes at [1], a PMD-aligned address should >> be passed to update_mmu_cache_pmd(). >> >> [1]: >> https://lore.kernel.org/all/ddd3fcd2-48b3-4170-bcaa-2fe66e093f43@redhat.com/ >> >> Signed-off-by: Dev Jain >> --- >>   mm/huge_memory.c | 108 +++++++++++++++++++++++++++++------------------ >>   1 file changed, 66 insertions(+), 42 deletions(-) >> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index 2a73efea02d7..cdc632b8dc9c 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -1146,47 +1146,88 @@ unsigned long thp_get_unmapped_area(struct >> file *filp, unsigned long addr, >>   } >>   EXPORT_SYMBOL_GPL(thp_get_unmapped_area); >>   -static vm_fault_t __do_huge_pmd_anonymous_page(struct vm_fault *vmf, >> -            struct page *page, gfp_t gfp) >> +static struct folio *vma_alloc_anon_folio_pmd(struct vm_area_struct >> *vma, >> +                          unsigned long addr) >>   { >> -    struct vm_area_struct *vma = vmf->vma; >> -    struct folio *folio = page_folio(page); >> -    pgtable_t pgtable; >> -    unsigned long haddr = vmf->address & HPAGE_PMD_MASK; >> -    vm_fault_t ret = 0; >> +    unsigned long haddr = addr & HPAGE_PMD_MASK; >> +    gfp_t gfp = vma_thp_gfp_mask(vma); >> +    const int order = HPAGE_PMD_ORDER; >> +    struct folio *folio = vma_alloc_folio(gfp, order, vma, haddr, >> true); >>   -    VM_BUG_ON_FOLIO(!folio_test_large(folio), folio); >> +    if (unlikely(!folio)) { >> +        count_vm_event(THP_FAULT_FALLBACK); >> +        count_mthp_stat(order, MTHP_STAT_ANON_FAULT_FALLBACK); >> +        goto out; >> +    } >>   +    VM_BUG_ON_FOLIO(!folio_test_large(folio), folio); >>       if (mem_cgroup_charge(folio, vma->vm_mm, gfp)) { >>           folio_put(folio); >>           count_vm_event(THP_FAULT_FALLBACK); >>           count_vm_event(THP_FAULT_FALLBACK_CHARGE); >> -        count_mthp_stat(HPAGE_PMD_ORDER, >> MTHP_STAT_ANON_FAULT_FALLBACK); >> -        count_mthp_stat(HPAGE_PMD_ORDER, >> MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE); >> -        return VM_FAULT_FALLBACK; >> +        count_mthp_stat(order, MTHP_STAT_ANON_FAULT_FALLBACK); >> +        count_mthp_stat(order, MTHP_STAT_ANON_FAULT_FALLBACK_CHARGE); >> +        return NULL; >>       } >>       folio_throttle_swaprate(folio, gfp); >>   -    pgtable = pte_alloc_one(vma->vm_mm); >> -    if (unlikely(!pgtable)) { >> -        ret = VM_FAULT_OOM; >> -        goto release; >> -    } >> - >> -    folio_zero_user(folio, vmf->address); >> +    folio_zero_user(folio, addr); >>       /* >>        * The memory barrier inside __folio_mark_uptodate makes sure that >>        * folio_zero_user writes become visible before the set_pmd_at() >>        * write. >>        */ >>       __folio_mark_uptodate(folio); >> +out: >> +    return folio; >> +} >> + >> +static void __pmd_thp_fault_success_stats(struct vm_area_struct *vma) >> +{ >> +    count_vm_event(THP_FAULT_ALLOC); >> +    count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_ALLOC); >> +    count_memcg_event_mm(vma->vm_mm, THP_FAULT_ALLOC); >> +} > > just inline that into map_anon_folio_pmd(), please. map_anon_folio_pmd > is perfectly readable ;) If you are asking me to open code it in map_anon_folio_pmd(), I'll do that. > >> + >> +static void map_anon_folio_pmd(struct folio *folio, pmd_t *pmd, >> +                   struct vm_area_struct *vma, unsigned long haddr) > > >> +{ >> +    pmd_t entry; >> + >> +    entry = mk_huge_pmd(&folio->page, vma->vm_page_prot); >> +    entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma); >> +    folio_add_new_anon_rmap(folio, vma, haddr, RMAP_EXCLUSIVE); >> +    folio_add_lru_vma(folio, vma); >> +    set_pmd_at(vma->vm_mm, haddr, pmd, entry); >> +    update_mmu_cache_pmd(vma, haddr, pmd); >> +    add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR); >> +    __pmd_thp_fault_success_stats(vma); >> +} >> + >> +static vm_fault_t __do_huge_pmd_anonymous_page(struct vm_fault *vmf) >> +{ >> +    unsigned long haddr = vmf->address & HPAGE_PMD_MASK; >> +    struct vm_area_struct *vma = vmf->vma; >> +    pgtable_t pgtable = NULL; >> +    struct folio *folio; >> +    vm_fault_t ret = 0; >> + >> +    folio = vma_alloc_anon_folio_pmd(vma, vmf->address); >> +    if (unlikely(!folio)) { >> +        ret = VM_FAULT_FALLBACK; >> +        goto release; > > Why not simply "return VM_FAULT_FALLBACK;" ? There is nothing to > release. Then you can avoid the > > "if (folio)" below and even stop initializing pgtable to NULL. Makes sense. > > > With these things take care of > > Acked-by: David Hildenbrand Thanks!