From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-124.freemail.mail.aliyun.com (out30-124.freemail.mail.aliyun.com [115.124.30.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A3333B652 for ; Sat, 21 Mar 2026 05:16:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774070164; cv=none; b=I6rY0lFrCZ/AUdjyicWm2ORgeMDQvuJ95Rk0cs4UuW4nLKLlrI+33Dh80In/VXNjIm0XKi2FpL3cEzj+72oaPhgreg/lwxAh6uoOWjSp4BA0/xe80SiCwscwg5/hPWoUZH3oOvTJIYvFBeGEFAnrtciCAJLZIQ09Ulm5LWTSfiU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774070164; c=relaxed/simple; bh=/jKlkOP8pAkqRpwC7j9dR7i+WMB9kPoLfYyWM11pOHU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=mHLPK/T6DN5c95kO9QIPAGYD2wiv/fKrrDMklkiy6B9jftYzlIZeejtf/p3ti9Bya4HeDX3xvhJ6ZZMwuGPJEkw4nRrI8VT9JMxhoj/0RtmPHOnFor/91KS3Gqhj7Vexqm4UhWhR/12acV45aONejlyLa+JuiXCqrIPDdKKgdfQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=C6Vy2Sps; arc=none smtp.client-ip=115.124.30.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="C6Vy2Sps" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1774070153; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=FvE5u6hIUEMa2cVmHPs2YrFYnl8iGvj61myQt4xkm9o=; b=C6Vy2SpsLroUfczrPpLnX0oY+gR9xUddsigbEGo6mBIxdW1hQcn9QB42R8RBStYeBhvd0pJnYPT/sL+4XUkJlXNct3Gpp84QYerWb5sRYBL1G0IejfF/ylakJE73n0qpWinKcD86Wt6mrge2KHiAQNMg0I/NX3C/D7/q3PqDFJA= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R731e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam011083073210;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=16;SR=0;TI=SMTPD_---0X.O3cfU_1774070150; Received: from 30.42.98.36(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0X.O3cfU_1774070150 cluster:ay36) by smtp.aliyun-inc.com; Sat, 21 Mar 2026 13:15:52 +0800 Message-ID: Date: Sat, 21 Mar 2026 13:15:50 +0800 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 8/9] mm/huge_memory: deduplicate zap_huge_pmd() further by tracking state To: "Lorenzo Stoakes (Oracle)" Cc: Andrew Morton , David Hildenbrand , Zi Yan , "Liam R . Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <440f68edcb597c28918d89b0be279d498561c89a.1773924928.git.ljs@kernel.org> From: Baolin Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 3/20/26 9:51 PM, Lorenzo Stoakes (Oracle) wrote: > On Fri, Mar 20, 2026 at 11:49:18AM +0800, Baolin Wang wrote: >> >> >> On 3/19/26 9:00 PM, Lorenzo Stoakes (Oracle) wrote: >>> The flush_needed boolean is really tracking whether a PMD entry is present, >>> so use it that way directly and rename it to is_present. >>> >>> Deduplicate the folio_remove_rmap_pmd() and folio map count warning between >>> present and device private by tracking where we need to remove the rmap. >>> >>> We can also remove the comment about using flush_needed to track whether a >>> PMD entry is present as it's now irrelevant. >>> >>> Signed-off-by: Lorenzo Stoakes (Oracle) >>> --- >>> mm/huge_memory.c | 28 +++++++++++++--------------- >>> 1 file changed, 13 insertions(+), 15 deletions(-) >>> >>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >>> index c4e00c645e58..22715027e56c 100644 >>> --- a/mm/huge_memory.c >>> +++ b/mm/huge_memory.c >>> @@ -2430,9 +2430,10 @@ static inline void zap_deposited_table(struct mm_struct *mm, pmd_t *pmd) >>> bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, >>> pmd_t *pmd, unsigned long addr) >>> { >>> + bool needs_remove_rmap = false; >>> struct folio *folio = NULL; >>> bool needs_deposit = false; >>> - bool flush_needed = false; >>> + bool is_present = false; >>> spinlock_t *ptl; >>> pmd_t orig_pmd; >>> @@ -2449,6 +2450,7 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, >>> */ >>> orig_pmd = pmdp_huge_get_and_clear_full(vma, addr, pmd, >>> tlb->fullmm); >>> + >>> arch_check_zapped_pmd(vma, orig_pmd); >>> tlb_remove_pmd_tlb_entry(tlb, pmd, addr); >>> if (vma_is_special_huge(vma)) >>> @@ -2458,17 +2460,15 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, >>> goto out; >>> } >>> - if (pmd_present(orig_pmd)) { >>> + is_present = pmd_present(orig_pmd); >>> + if (is_present) { >>> folio = pmd_folio(orig_pmd); >>> - >>> - flush_needed = true; >>> - folio_remove_rmap_pmd(folio, &folio->page, vma); >>> - WARN_ON_ONCE(folio_mapcount(folio) < 0); >>> + needs_remove_rmap = true; >>> } else if (pmd_is_valid_softleaf(orig_pmd)) { >>> const softleaf_t entry = softleaf_from_pmd(orig_pmd); >>> folio = softleaf_to_folio(entry); >>> - >>> + needs_remove_rmap = folio_is_device_private(folio); >>> if (!thp_migration_supported()) >>> WARN_ONCE(1, "Non present huge pmd without pmd migration enabled!"); >>> } else { >>> @@ -2483,27 +2483,25 @@ bool zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, >>> add_mm_counter(tlb->mm, mm_counter_file(folio), >>> -HPAGE_PMD_NR); >>> - /* >>> - * Use flush_needed to indicate whether the PMD entry >>> - * is present, instead of checking pmd_present() again. >>> - */ >>> - if (flush_needed && pmd_young(orig_pmd) && >>> + if (is_present && pmd_young(orig_pmd) && >>> likely(vma_has_recency(vma))) >>> folio_mark_accessed(folio); >>> } >>> - if (folio_is_device_private(folio)) { >>> + if (needs_remove_rmap) { >>> folio_remove_rmap_pmd(folio, &folio->page, vma); >>> WARN_ON_ONCE(folio_mapcount(folio) < 0); >>> - folio_put(folio); >>> } >>> out: >>> if (arch_needs_pgtable_deposit() || needs_deposit) >>> zap_deposited_table(tlb->mm, pmd); >>> + if (needs_remove_rmap && !is_present) >>> + folio_put(folio); >>> + >> >> This kind of deduplication splits the device folio handling into three >> places, which is not easy to understand (at least for me), since the device >> folio has some special handling. > > I think open-coded the exact same thing over and over again is FAR worse. > > It's also actually 2 places for softleaf, because we were duplicating #2 below: > > 1. how do I get a folio? > > 2. do I need to remove this folio from the rmap? (yes for device private) > > 3. Do I need to put the folio (yes for device private) > > You're maybe now just seeing exactly what happens here because the code is > clearer? Because before it was an unfathomable open coded mess with no > explanation. > > Now you explicitly see what's happening :) Yes, I understand your point:) I saw that you changed the code to use the 'is_device_private' variable in the new version, which is more readable for me. Thanks. >> Especially here, without looking closely at the if condition, it is unclear >> why we need to call folio_put(). Maybe some comments? > > I can add one, the original didn't. This is an existing issue :) Thanks.