mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Muchun Song <muchun.song@linux.dev>
To: Qi Zheng <qi.zheng@linux.dev>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	Christophe Leroy <chleroy@kernel.org>,
	Lorenzo Stoakes <ljs@kernel.org>,
	"Liam R . Howlett" <liam@infradead.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Muchun Song <songmuchun@bytedance.com>,
	Madhavan Srinivasan <maddy@linux.ibm.com>,
	Mike Rapoport <rppt@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@kernel.org>
Subject: Re: [PATCH 2/6] mm/sparse-vmemmap: support device DAX in common vmemmap path
Date: Tue, 22 Sep 2026 19:34:18 +0800	[thread overview]
Message-ID: <9ac5cba5-0044-4f57-9ec0-d5631b7b178c@linux.dev> (raw)
In-Reply-To: <dad55fc6-56ff-4375-893b-938404238ca8@linux.dev>



On 2026/9/22 16:02, Qi Zheng wrote:
>
>
> On 9/13/26 4:37 PM, Muchun Song wrote:
>> The common vmemmap population path cannot yet handle optimized Device 
>> DAX
>> mappings on its own. It uses pfn_to_zone() to find the shared tail page,
>> but Device DAX populates its vmemmap at runtime before the 
>> ZONE_DEVICE span
>> is initialized.
>>
>> Teach the common path to use device_zone() for runtime optimized vmemmap
>> population while retaining pfn_to_zone() for early boot. This allows the
>> same path to support both early boot mappings and Device DAX.
>>
>> The backing PFN supplied by the Device DAX-specific population path 
>> is no
>> longer used, allowing the redundant lookup and population code to be
>> removed later.
>>
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>>   mm/sparse-vmemmap.c | 44 +++++++++++++++++++-------------------------
>>   1 file changed, 19 insertions(+), 25 deletions(-)
>>
>> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
>> index 878d29a4e862..e83821768c12 100644
>> --- a/mm/sparse-vmemmap.c
>> +++ b/mm/sparse-vmemmap.c
>> @@ -208,18 +208,27 @@ static __meminit void 
>> *vmemmap_alloc_pte(unsigned long pfn, int node,
>>       struct page *page;
>>       const unsigned int order = pfn_to_section_compound_order(pfn);
>>   -    /*
>> -     * Device DAX still relies on vmemmap_populate_compound_pages() for
>> -     * head/first-tail allocation and tail-page reuse.
>> -     */
>>       if (!vmemmap_optimizable_pfn(pfn))
>>           return vmemmap_alloc_block_buf(PAGE_SIZE, node, altmap);
>>   -    zone = pfn_to_zone(pfn, node);
>> +    /*
>> +     * At runtime (slab available), only ZONE_DEVICE pages trigger 
>> vmemmap
>> +     * optimization, so device_zone() suffices. Note that pfn_to_zone()
>> +     * cannot be used at runtime because the zone span is not set up 
>> now.
>> +     */
>> +    zone = slab_is_available() ? device_zone(node) : 
>> pfn_to_zone(pfn, node);
>>       page = vmemmap_shared_tail_page(order, zone);
>>       if (!page)
>>           return NULL;
>>   +    /*
>> +     * When a PTE entry is freed, a free_pages() call occurs. This 
>> get_page()
>> +     * pairs with put_page_testzero() on the freeing path. This can 
>> only occur
>> +     * when slab is available.
>> +     */
>> +    if (slab_is_available())
>
> Would it make sense to introduce a helper function that wraps
> slab_is_available() for better readability? Also, it might be worth
> adding a comment to the helper function as well.

Thanks for the suggestion. I considered introducing a helper, but the
two uses of slab_is_available() depend on different properties of the
same initialization boundary. One determines how the zone is obtained,
while the other determines whether the shared vmemmap backing page can
participate in page refcounting. Any helper name would therefore be
either too generic or specific to only one of these properties.

I would prefer to keep slab_is_available() explicit and expand the
comments at both call sites. The first comment explains why early
system RAM can use pfn_to_zone(), whereas ZONE_DEVICE population after
slab becomes available must use device_zone(). The second explains why
early shared backing pages, which are allocated from memblock, cannot
yet be refcounted. Once slab becomes available, shared backing pages
are allocated from the buddy allocator and can hold one reference for
each shared PTE mapping.

I will adopt the following revisions. Does this make sense to you?

diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
index 5e0e30c77431..406d6f7918f7 100644
--- a/mm/sparse-vmemmap.c
+++ b/mm/sparse-vmemmap.c
@@ -212,9 +212,13 @@ static __meminit void *vmemmap_alloc_pte(unsigned 
long pfn, int node,
                 return vmemmap_alloc_block_buf(PAGE_SIZE, node, altmap);

         /*
-        * At runtime (slab available), only ZONE_DEVICE pages trigger 
vmemmap
-        * optimization, so device_zone() suffices. Note that pfn_to_zone()
-        * cannot be used at runtime because the zone span is not set up 
now.
+        * Before slab is available, vmemmap optimization is used for early
+        * system RAM, whose zone can be determined from the PFN.
+        *
+        * Once slab is available, only ZONE_DEVICE memory reaches this
+        * optimized population path. Its zone span has not been initialized
+        * while its vmemmap is being populated, so pfn_to_zone() cannot be
+        * used. Obtain ZONE_DEVICE directly from the node instead.
          */
         zone = slab_is_available() ? device_zone(node) : 
pfn_to_zone(pfn, node);
         page = vmemmap_shared_tail_page(order, zone);
@@ -222,9 +226,17 @@ static __meminit void *vmemmap_alloc_pte(unsigned 
long pfn, int node,
                 return NULL;

         /*
-        * When a PTE entry is freed, a free_pages() call occurs. This 
get_page()
-        * pairs with put_page_testzero() on the freeing path. This can 
only occur
-        * when slab is available.
+        * During early vmemmap population, the shared tail vmemmap backing
+        * page is allocated from memblock before its struct page can safely
+        * participate in page refcounting. Therefore, no reference can be
+        * held for each shared PTE mapping, and the mappings must be 
unshared
+        * before the vmemmap is depopulated.
+        *
+        * Once slab is available, the shared backing page is allocated from
+        * the buddy allocator and can be refcounted. Hold one reference for
+        * each shared PTE mapping. The architecture vmemmap teardown drops
+        * the reference through __free_pages() when removing the mapping,
+        * preventing the backing page from being freed while it is shared.
          */
         if (slab_is_available())
                 get_page(page);

>
> At least for me, encountering this always causes a moment of confusion.
>
>> +        get_page(page);
>> +
>>       return page_address(page);
>>   }
>>   @@ -231,27 +240,12 @@ static pte_t * __meminit 
>> vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, in
>>         if (pte_none(ptep_get(pte))) {
>>           pte_t entry;
>> +        void *p = vmemmap_alloc_pte(pfn, node, altmap);
>>   -        if (ptpfn == (unsigned long)-1) {
>> -            void *p = vmemmap_alloc_pte(pfn, node, altmap);
>> -
>> -            if (!p)
>> -                return NULL;
>> -            ptpfn = PHYS_PFN(__pa(p));
>> -        } else {
>> -            /*
>> -             * When a PTE/PMD entry is freed from the init_mm
>> -             * there's a free_pages() call to this page allocated
>> -             * above. Thus this get_page() is paired with the
>> -             * put_page_testzero() on the freeing path.
>> -             * This can only called by certain ZONE_DEVICE path,
>> -             * and through vmemmap_populate_compound_pages() when
>> -             * slab is available.
>> -             */
>> -            if (slab_is_available())
>> -                get_page(pfn_to_page(ptpfn));
>> -        }
>> -        entry = pfn_pte(ptpfn, PAGE_KERNEL);
>> +        if (!p)
>> +            return NULL;
>> +
>> +        entry = pfn_pte(PHYS_PFN(__pa(p)), PAGE_KERNEL);
>>           set_pte_at(&init_mm, addr, pte, entry);
>>       } else if (WARN_ON_ONCE(vmemmap_optimizable_pfn(pfn)))
>>           return NULL;
>


  reply	other threads:[~2026-09-22 11:34 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-13  8:37 [PATCH 0/6] mm: Unify Device DAX and HugeTLB vmemmap population paths Muchun Song
2026-09-13  8:37 ` [PATCH 1/6] mm/sparse-vmemmap: drop VMEMMAP_POPULATE_DAX Muchun Song
2026-09-19 14:01   ` Qi Zheng
2026-09-19 14:07     ` Muchun Song
2026-09-21  3:43       ` Qi Zheng
2026-09-21  3:49         ` Muchun Song
2026-09-22  7:54           ` Qi Zheng
2026-09-13  8:37 ` [PATCH 2/6] mm/sparse-vmemmap: support device DAX in common vmemmap path Muchun Song
2026-09-22  8:02   ` Qi Zheng
2026-09-22 11:34     ` Muchun Song [this message]
2026-09-13  8:37 ` [PATCH 3/6] mm/sparse-vmemmap: drop Device DAX-specific population path Muchun Song
2026-09-22  8:06   ` Qi Zheng
2026-09-13  8:37 ` [PATCH 4/6] mm/sparse-vmemmap: remove the unused ptpfn argument Muchun Song
2026-09-22  8:12   ` Qi Zheng
2026-09-13  8:37 ` [PATCH 5/6] powerpc/mm: make vmemmap_populate_compound_pages() static Muchun Song
2026-09-22  8:14   ` Qi Zheng
2026-09-13  8:37 ` [PATCH 6/6] mm/sparse-vmemmap: open-code vmemmap_populate_address() Muchun Song
2026-09-22  8:18   ` Qi Zheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9ac5cba5-0044-4f57-9ec0-d5631b7b178c@linux.dev \
    --to=muchun.song@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=chleroy@kernel.org \
    --cc=david@kernel.org \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=ljs@kernel.org \
    --cc=maddy@linux.ibm.com \
    --cc=mhocko@suse.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=qi.zheng@linux.dev \
    --cc=rppt@kernel.org \
    --cc=songmuchun@bytedance.com \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®