From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id BB89726E70E for ; Sat, 11 Apr 2026 16:08:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775923728; cv=none; b=CN1QmvryyDdd/HdYc7Y4RdYtpTHG7+GBmXLjV08PY7dcAbA5yRUVGrq+DxhPRUlEPGgEs2gbyFbgqDyjMSASaOD4xKWpnfRUPO3kQpNk3FCopJMxDOemH6FuPfsUNJMz74vNqgF99JpML1jAkq2rj90Tl/AisHEiSCLmdg5STxU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775923728; c=relaxed/simple; bh=FgB9t2z6USw7hVbVXkFXwPgw+4W6TEY8h/PvWm/9i2c=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=USu9lOthOwK2UdUA4yBAr1r5Ol3JEbmmUujBMhMnwtIWJchPZNHEpTcCkxL/vn4EYx5DK7ffIRnieHj5oP0WCCGyOrX7F94UcSA0HDTAgnATZgm1TQ9wr7Iphg5ICmsW7Gl3YkZ43VAeTB2Vf6LiGh32XcWucurG3Mj4hSMmtHM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=R15M9uy/; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="R15M9uy/" 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 93F0A2A68; Sat, 11 Apr 2026 09:08:38 -0700 (PDT) Received: from [10.163.141.179] (unknown [10.163.141.179]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 479703F641; Sat, 11 Apr 2026 09:08:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1775923723; bh=FgB9t2z6USw7hVbVXkFXwPgw+4W6TEY8h/PvWm/9i2c=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=R15M9uy/mzzjDL05m4Jv+OX9L3X19RGD52hCgSrZBhdd9wHW/Un8lJ8D8QROuDnJz mmpl5wAtQkd8BpP4UYPLqShrTdMyUYI9yXLa4h5d6hdL7V6rXEKbS+Fw2KUZE2WLY4 9DTJS4qSvASwyIgTHvC5c0FIIdwPPiAuSTQW/rlA= Message-ID: Date: Sat, 11 Apr 2026 21:38:26 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 2/9] mm/rmap: refactor hugetlb pte clearing in try_to_unmap_one To: Jie Gan , akpm@linux-foundation.org, david@kernel.org, hughd@google.com, chrisl@kernel.org Cc: ljs@kernel.org, Liam.Howlett@oracle.com, vbabka@kernel.org, rppt@kernel.org, surenb@google.com, mhocko@suse.com, kasong@tencent.com, qi.zheng@linux.dev, shakeel.butt@linux.dev, baohua@kernel.org, axelrasmussen@google.com, yuanchu@google.com, weixugc@google.com, riel@surriel.com, harry@kernel.org, jannh@google.com, pfalcato@suse.de, baolin.wang@linux.alibaba.com, shikemeng@huaweicloud.com, nphamcs@gmail.com, bhe@redhat.com, youngjun.park@lge.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, ryan.roberts@arm.com, anshuman.khandual@arm.com References: <20260410103204.120409-1-dev.jain@arm.com> <20260410103204.120409-3-dev.jain@arm.com> <2bfb7fd3-fc0c-47f7-8450-6180b0251ae5@oss.qualcomm.com> Content-Language: en-US From: Dev Jain In-Reply-To: <2bfb7fd3-fc0c-47f7-8450-6180b0251ae5@oss.qualcomm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 11/04/26 5:15 pm, Jie Gan wrote: > > > On 4/10/2026 6:31 PM, Dev Jain wrote: >> Simplify the code by refactoring the folio_test_hugetlb() branch into >> a new function. >> >> No functional change is intended. >> >> Signed-off-by: Dev Jain >> --- >>   mm/rmap.c | 116 +++++++++++++++++++++++++++++++----------------------- >>   1 file changed, 67 insertions(+), 49 deletions(-) >> >> diff --git a/mm/rmap.c b/mm/rmap.c >> index 62a8c912fd788..a9c43e2f6e695 100644 >> --- a/mm/rmap.c >> +++ b/mm/rmap.c >> @@ -1978,6 +1978,67 @@ static inline unsigned int >> folio_unmap_pte_batch(struct folio *folio, >>                        FPB_RESPECT_WRITE | FPB_RESPECT_SOFT_DIRTY); >>   } >>   +static inline bool unmap_hugetlb_folio(struct vm_area_struct *vma, >> +        struct folio *folio, struct page_vma_mapped_walk *pvmw, >> +        struct page *page, enum ttu_flags flags, pte_t *pteval, >> +        struct mmu_notifier_range *range, bool *walk_done) >> +{ >> +    /* >> +     * The try_to_unmap() is only passed a hugetlb page >> +     * in the case where the hugetlb page is poisoned. >> +     */ >> +    VM_WARN_ON_PAGE(!PageHWPoison(page), page); > > As you mentioned "No functional change is intended." in commit message, but > you have changed VM_BUG_ON_PAGE to VM_WARN_ON_PAGE here? Forgot to mention this in the description : ) ... "While at it, as BUG_ONs are discouraged, convert them to WARN_ON." > >> +    /* >> +     * huge_pmd_unshare may unmap an entire PMD page. >> +     * There is no way of knowing exactly which PMDs may >> +     * be cached for this mm, so we must flush them all. >> +     * start/end were already adjusted above to cover this >> +     * range. >> +     */ >> +    flush_cache_range(vma, range->start, range->end); >> + >> +    /* >> +     * To call huge_pmd_unshare, i_mmap_rwsem must be >> +     * held in write mode.  Caller needs to explicitly >> +     * do this outside rmap routines. >> +     * >> +     * We also must hold hugetlb vma_lock in write mode. >> +     * Lock order dictates acquiring vma_lock BEFORE >> +     * i_mmap_rwsem.  We can only try lock here and fail >> +     * if unsuccessful. >> +     */ >> +    if (!folio_test_anon(folio)) { >> +        struct mmu_gather tlb; >> + >> +        VM_WARN_ON(!(flags & TTU_RMAP_LOCKED)); > > ditto > > Thanks, > Jie > >> +        if (!hugetlb_vma_trylock_write(vma)) { >> +            *walk_done = true; >> +            return false; >> +        } >> + >> +        tlb_gather_mmu_vma(&tlb, vma); >> +        if (huge_pmd_unshare(&tlb, vma, pvmw->address, pvmw->pte)) { >> +            hugetlb_vma_unlock_write(vma); >> +            huge_pmd_unshare_flush(&tlb, vma); >> +            tlb_finish_mmu(&tlb); >> +            /* >> +             * The PMD table was unmapped, >> +             * consequently unmapping the folio. >> +             */ >> +            *walk_done = true; >> +            return true; >> +        } >> +        hugetlb_vma_unlock_write(vma); >> +        tlb_finish_mmu(&tlb); >> +    } >> +    *pteval = huge_ptep_clear_flush(vma, pvmw->address, pvmw->pte); >> +    if (pte_dirty(*pteval)) >> +        folio_mark_dirty(folio); >> + >> +    *walk_done = false; >> +    return true; >> +} >> + >>   /* >>    * @arg: enum ttu_flags will be passed to this argument >>    */ >> @@ -2115,56 +2176,13 @@ static bool try_to_unmap_one(struct folio *folio, >> struct vm_area_struct *vma, >>                    PageAnonExclusive(subpage); >>             if (folio_test_hugetlb(folio)) { >> -            bool anon = folio_test_anon(folio); >> - >> -            /* >> -             * The try_to_unmap() is only passed a hugetlb page >> -             * in the case where the hugetlb page is poisoned. >> -             */ >> -            VM_BUG_ON_PAGE(!PageHWPoison(subpage), subpage); >> -            /* >> -             * huge_pmd_unshare may unmap an entire PMD page. >> -             * There is no way of knowing exactly which PMDs may >> -             * be cached for this mm, so we must flush them all. >> -             * start/end were already adjusted above to cover this >> -             * range. >> -             */ >> -            flush_cache_range(vma, range.start, range.end); >> +            bool walk_done; >>   -            /* >> -             * To call huge_pmd_unshare, i_mmap_rwsem must be >> -             * held in write mode.  Caller needs to explicitly >> -             * do this outside rmap routines. >> -             * >> -             * We also must hold hugetlb vma_lock in write mode. >> -             * Lock order dictates acquiring vma_lock BEFORE >> -             * i_mmap_rwsem.  We can only try lock here and fail >> -             * if unsuccessful. >> -             */ >> -            if (!anon) { >> -                struct mmu_gather tlb; >> - >> -                VM_BUG_ON(!(flags & TTU_RMAP_LOCKED)); >> -                if (!hugetlb_vma_trylock_write(vma)) >> -                    goto walk_abort; >> - >> -                tlb_gather_mmu_vma(&tlb, vma); >> -                if (huge_pmd_unshare(&tlb, vma, address, pvmw.pte)) { >> -                    hugetlb_vma_unlock_write(vma); >> -                    huge_pmd_unshare_flush(&tlb, vma); >> -                    tlb_finish_mmu(&tlb); >> -                    /* >> -                     * The PMD table was unmapped, >> -                     * consequently unmapping the folio. >> -                     */ >> -                    goto walk_done; >> -                } >> -                hugetlb_vma_unlock_write(vma); >> -                tlb_finish_mmu(&tlb); >> -            } >> -            pteval = huge_ptep_clear_flush(vma, address, pvmw.pte); >> -            if (pte_dirty(pteval)) >> -                folio_mark_dirty(folio); >> +            ret = unmap_hugetlb_folio(vma, folio, &pvmw, subpage, >> +                          flags, &pteval, &range, >> +                          &walk_done); >> +            if (walk_done) >> +                goto walk_done; >>           } else if (likely(pte_present(pteval))) { >>               nr_pages = folio_unmap_pte_batch(folio, &pvmw, flags, pteval); >>               end_addr = address + nr_pages * PAGE_SIZE; >