add mTHP support for anonymous shmem

Add large folio mapping establishment support for finish_fault() as a preparation, to support multi-size THP allocation of anonymous shmem pages in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
 mm/memory.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 10 deletions(-)

mm/memory.c +33 -7

index eef4e482c0c2..435187ff7ea4 100644
@@ -4831,9 +4831,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
 {
        struct vm_area_struct *vma = vmf->vma;
        struct page *page;
+       struct folio *folio;
        vm_fault_t ret;
        bool is_cow = (vmf->flags & FAULT_FLAG_WRITE) &&
                      !(vma->vm_flags & VM_SHARED);
+       int type, nr_pages, i;
+       unsigned long addr = vmf->address;
 
        /* Did we COW the page? */
        if (is_cow)

@@ -4864,24 +4867,59 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
                        return VM_FAULT_OOM;
        }
 
+       folio = page_folio(page);
+       nr_pages = folio_nr_pages(folio);
+
+       /*
+        * Using per-page fault to maintain the uffd semantics, and same
+        * approach also applies to non-anonymous-shmem faults to avoid
+        * inflating the RSS of the process.
+        */
+       if (!vma_is_anon_shmem(vma) || unlikely(userfaultfd_armed(vma))) {
+               nr_pages = 1;
+       } else if (nr_pages > 1) {
+               pgoff_t idx = folio_page_idx(folio, page);
+               /* The page offset of vmf->address within the VMA. */
+               pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
+
+               /*
+                * Fallback to per-page fault in case the folio size in page
+                * cache beyond the VMA limits.
+                */
+               if (unlikely(vma_off < idx ||
+                            vma_off + (nr_pages - idx) > vma_pages(vma))) {
+                       nr_pages = 1;
+               } else {
+                       /* Now we can set mappings for the whole large folio. */
+                       addr = vmf->address - idx * PAGE_SIZE;
+                       page = &folio->page;
+               }
+       }
+
        vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-                                     vmf->address, &vmf->ptl);
+                                      addr, &vmf->ptl);
        if (!vmf->pte)
                return VM_FAULT_NOPAGE;
 
        /* Re-check under ptl */
-       if (likely(!vmf_pte_changed(vmf))) {
-               struct folio *folio = page_folio(page);
-               int type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
-
-               set_pte_range(vmf, folio, page, 1, vmf->address);
-               add_mm_counter(vma->vm_mm, type, 1);

mm/memory.c +9 -1

+               ret = VM_FAULT_NOPAGE;
+               goto unlock;
        }
 
+       folio_ref_add(folio, nr_pages - 1);
+       set_pte_range(vmf, folio, page, nr_pages, addr);
+       type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
+       add_mm_counter(vma->vm_mm, type, nr_pages);
+       ret = 0;
+
+unlock:
        pte_unmap_unlock(vmf->pte, vmf->ptl);
        return ret;
 }
--

2.39.3

Quoted 73 lines · Baolin Wang

Add large folio mapping establishment support for finish_fault() as a preparation, to support multi-size THP allocation of anonymous shmem pages in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
  mm/memory.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 48 insertions(+), 10 deletions(-)

mm/memory.c +33 -7

index eef4e482c0c2..435187ff7ea4 100644
@@ -4831,9 +4831,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
  {
         struct vm_area_struct *vma = vmf->vma;
         struct page *page;
+       struct folio *folio;
         vm_fault_t ret;
         bool is_cow = (vmf->flags & FAULT_FLAG_WRITE) &&
                       !(vma->vm_flags & VM_SHARED);
+       int type, nr_pages, i;
+       unsigned long addr = vmf->address;
 
         /* Did we COW the page? */
         if (is_cow)

@@ -4864,24 +4867,59 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
                         return VM_FAULT_OOM;
         }
 
+       folio = page_folio(page);
+       nr_pages = folio_nr_pages(folio);
+
+       /*
+        * Using per-page fault to maintain the uffd semantics, and same
+        * approach also applies to non-anonymous-shmem faults to avoid
+        * inflating the RSS of the process.
+        */
+       if (!vma_is_anon_shmem(vma) || unlikely(userfaultfd_armed(vma))) {
+               nr_pages = 1;
+       } else if (nr_pages > 1) {
+               pgoff_t idx = folio_page_idx(folio, page);
+               /* The page offset of vmf->address within the VMA. */
+               pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
+
+               /*
+                * Fallback to per-page fault in case the folio size in page
+                * cache beyond the VMA limits.
+                */
+               if (unlikely(vma_off < idx ||
+                            vma_off + (nr_pages - idx) > vma_pages(vma))) {
+                       nr_pages = 1;
+               } else {
+                       /* Now we can set mappings for the whole large folio. */
+                       addr = vmf->address - idx * PAGE_SIZE;
+                       page = &folio->page;
+               }
+       }
+
         vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-                                     vmf->address, &vmf->ptl);
+                                      addr, &vmf->ptl);
         if (!vmf->pte)
                 return VM_FAULT_NOPAGE;
 
         /* Re-check under ptl */
-       if (likely(!vmf_pte_changed(vmf))) {
-               struct folio *folio = page_folio(page);
-               int type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
-
-               set_pte_range(vmf, folio, page, 1, vmf->address);
-               add_mm_counter(vma->vm_mm, type, 1);

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4911

Quoted 10 lines

mm/memory.c +6 -3

-               ret = 0;
-       } else {
-               update_mmu_tlb(vma, vmf->address, vmf->pte);
+       if (nr_pages == 1 && unlikely(vmf_pte_changed(vmf))) {
+               update_mmu_tlb(vma, addr, vmf->pte);
                 ret = VM_FAULT_NOPAGE;
+               goto unlock;
+       } else if (nr_pages > 1 && !pte_range_none(vmf->pte, nr_pages)) {
+               for (i = 0; i < nr_pages; i++)
+                       update_mmu_tlb(vma, addr + PAGE_SIZE * i, vmf->pte + i);

Just a friendly reminder: Bang has added the update_mmu_tlb_range()[1] batch function to update TLB in batches, so we can use it instead of the update_mmu_tlb()
loop.

[1] https://lore.kernel.org/linux-mm/[email protected]/

Add large folio mapping establishment support for finish_fault() as a preparation, to support multi-size THP allocation of anonymous shmem pages in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
 mm/memory.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 10 deletions(-)

mm/memory.c +3 -0

index eef4e482c0c2..435187ff7ea4 100644
@@ -4831,9 +4831,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
 {
        struct vm_area_struct *vma = vmf->vma;
        struct page *page;
+       struct folio *folio;
        vm_fault_t ret;
        bool is_cow = (vmf->flags & FAULT_FLAG_WRITE) &&
                      !(vma->vm_flags & VM_SHARED);
+       int type, nr_pages, i;
+       unsigned long addr = vmf->address;
 
        /* Did we COW the page? */
        if (is_cow)

mm/memory.c +23 -5

+        */
+       if (!vma_is_anon_shmem(vma) || unlikely(userfaultfd_armed(vma))) {
+               nr_pages = 1;
+       } else if (nr_pages > 1) {
+               pgoff_t idx = folio_page_idx(folio, page);
+               /* The page offset of vmf->address within the VMA. */
+               pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
+
+               /*
+                * Fallback to per-page fault in case the folio size in page
+                * cache beyond the VMA limits.
+                */
+               if (unlikely(vma_off < idx ||
+                            vma_off + (nr_pages - idx) > vma_pages(vma))) {
+                       nr_pages = 1;
+               } else {
+                       /* Now we can set mappings for the whole large folio. */
+                       addr = vmf->address - idx * PAGE_SIZE;
+                       page = &folio->page;
+               }
+       }
+
        vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-                                     vmf->address, &vmf->ptl);
+                                      addr, &vmf->ptl);
        if (!vmf->pte)
                return VM_FAULT_NOPAGE;
 
        /* Re-check under ptl */
-       if (likely(!vmf_pte_changed(vmf))) {
-               struct folio *folio = page_folio(page);
-               int type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
-

mm/memory.c +11 -1

+               for (i = 0; i < nr_pages; i++)
+                       update_mmu_tlb(vma, addr + PAGE_SIZE * i, vmf->pte + i);
+               ret = VM_FAULT_NOPAGE;
+               goto unlock;
        }
 
+       folio_ref_add(folio, nr_pages - 1);
+       set_pte_range(vmf, folio, page, nr_pages, addr);
+       type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
+       add_mm_counter(vma->vm_mm, type, nr_pages);
+       ret = 0;
+
+unlock:
        pte_unmap_unlock(vmf->pte, vmf->ptl);
        return ret;
 }
--

2.39.3

Quoted 31 lines · Baolin Wang

Add large folio mapping establishment support for finish_fault() as a preparation, to support multi-size THP allocation of anonymous shmem pages in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
  mm/memory.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 48 insertions(+), 10 deletions(-)

mm/memory.c +3 -0

index eef4e482c0c2..435187ff7ea4 100644
@@ -4831,9 +4831,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
  {
         struct vm_area_struct *vma = vmf->vma;
         struct page *page;
+       struct folio *folio;
         vm_fault_t ret;
         bool is_cow = (vmf->flags & FAULT_FLAG_WRITE) &&
                       !(vma->vm_flags & VM_SHARED);
+       int type, nr_pages, i;
+       unsigned long addr = vmf->address;
 
         /* Did we COW the page? */
         if (is_cow)

@@ -4864,24 +4867,59 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
                         return VM_FAULT_OOM;
         }
 

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4876

Quoted 7 lines

mm/memory.c +7 -0

+       folio = page_folio(page);
+       nr_pages = folio_nr_pages(folio);
+
+       /*
+        * Using per-page fault to maintain the uffd semantics, and same
+        * approach also applies to non-anonymous-shmem faults to avoid
+        * inflating the RSS of the process.

I don't feel the comment explains the root cause.
For non-shmem, anyway we have allocated the memory? Avoiding inflating RSS seems not so useful as we have occupied the memory. the memory footprint

is what we really care about. so we want to rely on read-ahead hints of subpage to determine read-ahead size? that is why we don't map nr_pages for non-shmem files though we can potentially reduce nr_pages - 1 page faults?

Quoted 33 lines

mm/memory.c +23 -5

+        */
+       if (!vma_is_anon_shmem(vma) || unlikely(userfaultfd_armed(vma))) {
+               nr_pages = 1;
+       } else if (nr_pages > 1) {
+               pgoff_t idx = folio_page_idx(folio, page);
+               /* The page offset of vmf->address within the VMA. */
+               pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
+
+               /*
+                * Fallback to per-page fault in case the folio size in page
+                * cache beyond the VMA limits.
+                */
+               if (unlikely(vma_off < idx ||
+                            vma_off + (nr_pages - idx) > vma_pages(vma))) {
+                       nr_pages = 1;
+               } else {
+                       /* Now we can set mappings for the whole large folio. */
+                       addr = vmf->address - idx * PAGE_SIZE;
+                       page = &folio->page;
+               }
+       }
+
         vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-                                     vmf->address, &vmf->ptl);
+                                      addr, &vmf->ptl);
         if (!vmf->pte)
                 return VM_FAULT_NOPAGE;
 
         /* Re-check under ptl */
-       if (likely(!vmf_pte_changed(vmf))) {
-               struct folio *folio = page_folio(page);
-               int type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
-

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4909

Quoted 10 lines

mm/memory.c +4 -5

-               set_pte_range(vmf, folio, page, 1, vmf->address);
-               add_mm_counter(vma->vm_mm, type, 1);
-               ret = 0;
-       } else {
-               update_mmu_tlb(vma, vmf->address, vmf->pte);
+       if (nr_pages == 1 && unlikely(vmf_pte_changed(vmf))) {
+               update_mmu_tlb(vma, addr, vmf->pte);
                 ret = VM_FAULT_NOPAGE;
+               goto unlock;
+       } else if (nr_pages > 1 && !pte_range_none(vmf->pte, nr_pages)) {

In what case we can't use !pte_range_none(vmf->pte, 1) for nr_pages == 1 then unify the code for nr_pages==1 and nr_pages > 1?

It seems this has been discussed before, but I forget the reason.

Quoted 33 lines · Barry Song

Quoted 31 lines · Baolin Wang

Add large folio mapping establishment support for finish_fault() as a preparation, to support multi-size THP allocation of anonymous shmem pages in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
  mm/memory.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 48 insertions(+), 10 deletions(-)

mm/memory.c +3 -0

index eef4e482c0c2..435187ff7ea4 100644
@@ -4831,9 +4831,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
  {
         struct vm_area_struct *vma = vmf->vma;
         struct page *page;
+       struct folio *folio;
         vm_fault_t ret;
         bool is_cow = (vmf->flags & FAULT_FLAG_WRITE) &&
                       !(vma->vm_flags & VM_SHARED);
+       int type, nr_pages, i;
+       unsigned long addr = vmf->address;
 
         /* Did we COW the page? */
         if (is_cow)

@@ -4864,24 +4867,59 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
                         return VM_FAULT_OOM;
         }
 

Quoted 11 lines

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4876

Quoted 7 lines

mm/memory.c +7 -0

+       folio = page_folio(page);
+       nr_pages = folio_nr_pages(folio);
+
+       /*
+        * Using per-page fault to maintain the uffd semantics, and same
+        * approach also applies to non-anonymous-shmem faults to avoid
+        * inflating the RSS of the process.

I don't feel the comment explains the root cause.
For non-shmem, anyway we have allocated the memory? Avoiding inflating RSS seems not so useful as we have occupied the memory. the memory footprint

This is also to keep the same behavior as before for non-anon-shmem, and will be discussed in the future.

Quoted 4 lines

is what we really care about. so we want to rely on read-ahead hints of subpage to determine read-ahead size? that is why we don't map nr_pages for non-shmem files though we can potentially reduce nr_pages - 1 page faults?

IMHO, there is 2 cases for non-anon-shmem:
(1) read mmap() faults: we can rely on the 'fault_around_bytes' interface to determin what size of mapping to build.
(2) writable mmap() faults: I want to keep the same behavior as before (per-page fault), but we can talk about this when I send new patches to use mTHP to control large folio allocation for writable mmap().

Quoted 48 lines

Quoted 34 lines

mm/memory.c +23 -5

+        */
+       if (!vma_is_anon_shmem(vma) || unlikely(userfaultfd_armed(vma))) {
+               nr_pages = 1;
+       } else if (nr_pages > 1) {
+               pgoff_t idx = folio_page_idx(folio, page);
+               /* The page offset of vmf->address within the VMA. */
+               pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
+
+               /*
+                * Fallback to per-page fault in case the folio size in page
+                * cache beyond the VMA limits.
+                */
+               if (unlikely(vma_off < idx ||
+                            vma_off + (nr_pages - idx) > vma_pages(vma))) {
+                       nr_pages = 1;
+               } else {
+                       /* Now we can set mappings for the whole large folio. */
+                       addr = vmf->address - idx * PAGE_SIZE;
+                       page = &folio->page;
+               }
+       }
+
         vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-                                     vmf->address, &vmf->ptl);
+                                      addr, &vmf->ptl);
         if (!vmf->pte)
                 return VM_FAULT_NOPAGE;
 
         /* Re-check under ptl */
-       if (likely(!vmf_pte_changed(vmf))) {
-               struct folio *folio = page_folio(page);
-               int type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
-

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4909

Quoted 10 lines

mm/memory.c +4 -5

-               set_pte_range(vmf, folio, page, 1, vmf->address);
-               add_mm_counter(vma->vm_mm, type, 1);
-               ret = 0;
-       } else {
-               update_mmu_tlb(vma, vmf->address, vmf->pte);
+       if (nr_pages == 1 && unlikely(vmf_pte_changed(vmf))) {
+               update_mmu_tlb(vma, addr, vmf->pte);
                 ret = VM_FAULT_NOPAGE;
+               goto unlock;
+       } else if (nr_pages > 1 && !pte_range_none(vmf->pte, nr_pages)) {

In what case we can't use !pte_range_none(vmf->pte, 1) for nr_pages == 1 then unify the code for nr_pages==1 and nr_pages > 1?

Quoted 1 line

It seems this has been discussed before, but I forget the reason.

IIUC, this is for uffd case, which is not a none pte entry.

Quoted 49 lines · Baolin Wang

Quoted 33 lines · Barry Song

Quoted 31 lines · Baolin Wang

Add large folio mapping establishment support for finish_fault() as a preparation, to support multi-size THP allocation of anonymous shmem pages in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
  mm/memory.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 48 insertions(+), 10 deletions(-)

mm/memory.c +3 -0

index eef4e482c0c2..435187ff7ea4 100644
@@ -4831,9 +4831,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
  {
         struct vm_area_struct *vma = vmf->vma;
         struct page *page;
+       struct folio *folio;
         vm_fault_t ret;
         bool is_cow = (vmf->flags & FAULT_FLAG_WRITE) &&
                       !(vma->vm_flags & VM_SHARED);
+       int type, nr_pages, i;
+       unsigned long addr = vmf->address;
 
         /* Did we COW the page? */
         if (is_cow)

@@ -4864,24 +4867,59 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
                         return VM_FAULT_OOM;
         }
 

Quoted 11 lines

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4876

Quoted 7 lines

mm/memory.c +7 -0

+       folio = page_folio(page);
+       nr_pages = folio_nr_pages(folio);
+
+       /*
+        * Using per-page fault to maintain the uffd semantics, and same
+        * approach also applies to non-anonymous-shmem faults to avoid
+        * inflating the RSS of the process.

I don't feel the comment explains the root cause.
For non-shmem, anyway we have allocated the memory? Avoiding inflating RSS seems not so useful as we have occupied the memory. the memory footprint

Quoted 2 lines

This is also to keep the same behavior as before for non-anon-shmem, and will be discussed in the future.

OK.

Quoted 11 lines

Quoted 4 lines

is what we really care about. so we want to rely on read-ahead hints of subpage to determine read-ahead size? that is why we don't map nr_pages for non-shmem files though we can potentially reduce nr_pages - 1 page faults?

IMHO, there is 2 cases for non-anon-shmem:
(1) read mmap() faults: we can rely on the 'fault_around_bytes' interface to determin what size of mapping to build.
(2) writable mmap() faults: I want to keep the same behavior as before (per-page fault), but we can talk about this when I send new patches to use mTHP to control large folio allocation for writable mmap().

OK.

Quoted 48 lines

Quoted 48 lines

Quoted 34 lines

mm/memory.c +23 -5

+        */
+       if (!vma_is_anon_shmem(vma) || unlikely(userfaultfd_armed(vma))) {
+               nr_pages = 1;
+       } else if (nr_pages > 1) {
+               pgoff_t idx = folio_page_idx(folio, page);
+               /* The page offset of vmf->address within the VMA. */
+               pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
+
+               /*
+                * Fallback to per-page fault in case the folio size in page
+                * cache beyond the VMA limits.
+                */
+               if (unlikely(vma_off < idx ||
+                            vma_off + (nr_pages - idx) > vma_pages(vma))) {
+                       nr_pages = 1;
+               } else {
+                       /* Now we can set mappings for the whole large folio. */
+                       addr = vmf->address - idx * PAGE_SIZE;
+                       page = &folio->page;
+               }
+       }
+
         vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-                                     vmf->address, &vmf->ptl);
+                                      addr, &vmf->ptl);
         if (!vmf->pte)
                 return VM_FAULT_NOPAGE;
 
         /* Re-check under ptl */
-       if (likely(!vmf_pte_changed(vmf))) {
-               struct folio *folio = page_folio(page);
-               int type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
-

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4909

Quoted 10 lines

mm/memory.c +4 -5

-               set_pte_range(vmf, folio, page, 1, vmf->address);
-               add_mm_counter(vma->vm_mm, type, 1);
-               ret = 0;
-       } else {
-               update_mmu_tlb(vma, vmf->address, vmf->pte);
+       if (nr_pages == 1 && unlikely(vmf_pte_changed(vmf))) {
+               update_mmu_tlb(vma, addr, vmf->pte);
                 ret = VM_FAULT_NOPAGE;
+               goto unlock;
+       } else if (nr_pages > 1 && !pte_range_none(vmf->pte, nr_pages)) {

In what case we can't use !pte_range_none(vmf->pte, 1) for nr_pages == 1 then unify the code for nr_pages==1 and nr_pages > 1?

Quoted 3 lines

Quoted 1 line

It seems this has been discussed before, but I forget the reason.

IIUC, this is for uffd case, which is not a none pte entry.

Is it possible to have a COW case for shmem? For example, if someone maps a shmem
file as read-only and then writes to it, would that prevent the use of pte_range_none?

Furthermore, if we encounter a large folio in shmem while reading, does it necessarily
mean we can map the entire folio? Is it possible for some processes to only map part
of large folios? For instance, if process A allocates large folios and process B maps
only part of this shmem file or partially unmaps a large folio, how would that be handled?

Apologies for not debugging this thoroughly, but these two corner cases seem worth
considering. If these scenarios have already been addressed, please disregard my comments.

Thanks
Barry

Quoted 121 lines · Barry Song

Quoted 49 lines · Baolin Wang

Quoted 33 lines · Barry Song

Quoted 31 lines · Baolin Wang

Add large folio mapping establishment support for finish_fault() as a preparation, to support multi-size THP allocation of anonymous shmem pages in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
   mm/memory.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---------
   1 file changed, 48 insertions(+), 10 deletions(-)

mm/memory.c +3 -0

index eef4e482c0c2..435187ff7ea4 100644
@@ -4831,9 +4831,12 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
   {
          struct vm_area_struct *vma = vmf->vma;
          struct page *page;
+       struct folio *folio;
          vm_fault_t ret;
          bool is_cow = (vmf->flags & FAULT_FLAG_WRITE) &&
                        !(vma->vm_flags & VM_SHARED);
+       int type, nr_pages, i;
+       unsigned long addr = vmf->address;
 
          /* Did we COW the page? */
          if (is_cow)

@@ -4864,24 +4867,59 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
                          return VM_FAULT_OOM;
          }
 

Quoted 11 lines

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4876

Quoted 7 lines

mm/memory.c +7 -0

+       folio = page_folio(page);
+       nr_pages = folio_nr_pages(folio);
+
+       /*
+        * Using per-page fault to maintain the uffd semantics, and same
+        * approach also applies to non-anonymous-shmem faults to avoid
+        * inflating the RSS of the process.

I don't feel the comment explains the root cause.
For non-shmem, anyway we have allocated the memory? Avoiding inflating RSS seems not so useful as we have occupied the memory. the memory footprint

Quoted 2 lines

This is also to keep the same behavior as before for non-anon-shmem, and will be discussed in the future.

OK.

Quoted 11 lines

Quoted 4 lines

is what we really care about. so we want to rely on read-ahead hints of subpage to determine read-ahead size? that is why we don't map nr_pages for non-shmem files though we can potentially reduce nr_pages - 1 page faults?

IMHO, there is 2 cases for non-anon-shmem:
(1) read mmap() faults: we can rely on the 'fault_around_bytes' interface to determin what size of mapping to build.
(2) writable mmap() faults: I want to keep the same behavior as before (per-page fault), but we can talk about this when I send new patches to use mTHP to control large folio allocation for writable mmap().

OK.

Quoted 50 lines

Quoted 49 lines

Quoted 34 lines

mm/memory.c +23 -5

+        */
+       if (!vma_is_anon_shmem(vma) || unlikely(userfaultfd_armed(vma))) {
+               nr_pages = 1;
+       } else if (nr_pages > 1) {
+               pgoff_t idx = folio_page_idx(folio, page);
+               /* The page offset of vmf->address within the VMA. */
+               pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
+
+               /*
+                * Fallback to per-page fault in case the folio size in page
+                * cache beyond the VMA limits.
+                */
+               if (unlikely(vma_off < idx ||
+                            vma_off + (nr_pages - idx) > vma_pages(vma))) {
+                       nr_pages = 1;
+               } else {
+                       /* Now we can set mappings for the whole large folio. */
+                       addr = vmf->address - idx * PAGE_SIZE;
+                       page = &folio->page;
+               }
+       }
+
          vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
-                                     vmf->address, &vmf->ptl);
+                                      addr, &vmf->ptl);
          if (!vmf->pte)
                  return VM_FAULT_NOPAGE;
 
          /* Re-check under ptl */
-       if (likely(!vmf_pte_changed(vmf))) {
-               struct folio *folio = page_folio(page);
-               int type = is_cow ? MM_ANONPAGES : mm_counter_file(folio);
-

mm/memory.c vm_fault_t finish_fault(struct vm_fault *vmf) · line 4909

Quoted 10 lines

mm/memory.c +4 -5

-               set_pte_range(vmf, folio, page, 1, vmf->address);
-               add_mm_counter(vma->vm_mm, type, 1);
-               ret = 0;
-       } else {
-               update_mmu_tlb(vma, vmf->address, vmf->pte);
+       if (nr_pages == 1 && unlikely(vmf_pte_changed(vmf))) {
+               update_mmu_tlb(vma, addr, vmf->pte);
                  ret = VM_FAULT_NOPAGE;
+               goto unlock;
+       } else if (nr_pages > 1 && !pte_range_none(vmf->pte, nr_pages)) {

In what case we can't use !pte_range_none(vmf->pte, 1) for nr_pages == 1 then unify the code for nr_pages==1 and nr_pages > 1?

It seems this has been discussed before, but I forget the reason.

Quoted 6 lines

Quoted 1 line

IIUC, this is for uffd case, which is not a none pte entry.

Is it possible to have a COW case for shmem? For example, if someone maps a shmem
file as read-only and then writes to it, would that prevent the use of pte_range_none?

sorry, i mean PRIVATE but not READ-ONLY.

Quoted 3 lines

Furthermore, if we encounter a large folio in shmem while reading, does it necessarily
mean we can map the entire folio? Is it possible for some processes to

Quoted 5 lines

only map part
of large folios? For instance, if process A allocates large folios and process B maps
only part of this shmem file or partially unmaps a large folio, how would that be handled?

Quoted 4 lines

Apologies for not debugging this thoroughly, but these two corner cases seem worth
considering. If these scenarios have already been addressed, please disregard my comments.

To support the use of mTHP with anonymous shmem, add a new sysfs interface 'shmem_enabled' in the '/sys/kernel/mm/transparent_hugepage/hugepages-kB/' directory for each mTHP to control whether shmem is enabled for that mTHP, with a value similar to the top level 'shmem_enabled', which can be set to:
"always", "inherit (to inherit the top level setting)", "within_size", "advise", "never", "deny", "force". These values follow the same semantics as the top level, except the 'deny' is equivalent to 'never', and 'force' is equivalent to 'always' to keep compatibility.

By default, PMD-sized hugepages have enabled="inherit" and all other hugepage sizes have enabled="never" for '/sys/kernel/mm/transparent_hugepage/hugepages-xxkB/shmem_enabled'.

In addition, if top level value is 'force', then only PMD-sized hugepages have enabled="inherit", otherwise configuration will be failed and vice versa. That means now we will avoid using non-PMD sized THP to override the global huge allocation.

Signed-off-by Baolin Wang <[email protected]>

---

Documentation/admin-guide/mm/transhuge.rst | 29 +++++++
include/linux/huge_mm.h                    | 10 +++
mm/huge_memory.c                           | 11 +--
mm/shmem.c                                 | 96 ++++++++++++++++++++++
4 files changed, 138 insertions(+), 8 deletions(-)

Documentation/admin-guide/mm/transhuge.rst +0 -0

index d414d3f5592a..659459374032 100644
@@ -332,6 +332,35 @@ deny

force

    Force the huge option on for all - very useful for testing;

Documentation/admin-guide/mm/transhuge.rst +20 -0

+Anonymous shmem can also use "multi-size THP" (mTHP) by adding a new sysfs knob
+to control mTHP allocation: /sys/kernel/mm/transparent_hugepage/hugepages-<size>kB/shmem_enabled.
+Its value for each mTHP is essentially consistent with the global setting, except
+for the addition of 'inherit' to ensure compatibility with the global settings.
+always
+    Attempt to allocate <size> huge pages every time we need a new page;
+
+inherit
+    Inherit the top-level "shmem_enabled" value. By default, PMD-sized hugepages
+    have enabled="inherit" and all other hugepage sizes have enabled="never";
+
+never
+    Do not allocate <size> huge pages;
+
+within_size
+    Only allocate <size> huge page if it will be fully within i_size.
+    Also respect fadvise()/madvise() hints;
+
+advise
+    Only allocate <size> huge pages if requested with fadvise()/madvise();

Need of application restart
===========================

Quoted 57 lines · Baolin Wang

To support the use of mTHP with anonymous shmem, add a new sysfs interface 'shmem_enabled' in the '/sys/kernel/mm/transparent_hugepage/hugepages-kB/' directory for each mTHP to control whether shmem is enabled for that mTHP, with a value similar to the top level 'shmem_enabled', which can be set to:
"always", "inherit (to inherit the top level setting)", "within_size", "advise", "never", "deny", "force". These values follow the same semantics as the top level, except the 'deny' is equivalent to 'never', and 'force' is equivalent to 'always' to keep compatibility.

By default, PMD-sized hugepages have enabled="inherit" and all other hugepage sizes have enabled="never" for '/sys/kernel/mm/transparent_hugepage/hugepages-xxkB/shmem_enabled'.

In addition, if top level value is 'force', then only PMD-sized hugepages have enabled="inherit", otherwise configuration will be failed and vice versa. That means now we will avoid using non-PMD sized THP to override the global huge allocation.

Signed-off-by Baolin Wang <[email protected]>

---

Documentation/admin-guide/mm/transhuge.rst | 29 +++++++
include/linux/huge_mm.h                    | 10 +++
mm/huge_memory.c                           | 11 +--
mm/shmem.c                                 | 96 ++++++++++++++++++++++
4 files changed, 138 insertions(+), 8 deletions(-)

Documentation/admin-guide/mm/transhuge.rst +0 -0

index d414d3f5592a..659459374032 100644
@@ -332,6 +332,35 @@ deny

force

    Force the huge option on for all - very useful for testing;

Documentation/admin-guide/mm/transhuge.rst +24 -0

+Anonymous shmem can also use "multi-size THP" (mTHP) by adding a new sysfs knob
+to control mTHP allocation: /sys/kernel/mm/transparent_hugepage/hugepages-<size>kB/shmem_enabled.
+Its value for each mTHP is essentially consistent with the global setting, except
+for the addition of 'inherit' to ensure compatibility with the global settings.
+always
+    Attempt to allocate <size> huge pages every time we need a new page;
+
+inherit
+    Inherit the top-level "shmem_enabled" value. By default, PMD-sized hugepages
+    have enabled="inherit" and all other hugepage sizes have enabled="never";
+
+never
+    Do not allocate <size> huge pages;
+
+within_size
+    Only allocate <size> huge page if it will be fully within i_size.
+    Also respect fadvise()/madvise() hints;
+
+advise
+    Only allocate <size> huge pages if requested with fadvise()/madvise();
+
+deny
+    Has the same semantics as 'never', now mTHP allocation policy is only
+    used for anonymous shmem and no not override tmpfs.

Documentation/admin-guide/mm/transhuge.rst deny · line 363

Quoted 5 lines

Documentation/admin-guide/mm/transhuge.rst +5 -0

+
+force
+    Has the same semantics as 'always', now mTHP allocation policy is only
+    used for anonymous shmem and no not override tmpfs.
+

I just briefly reviewed the discussion about the value of hugepages-<size>kB/shmem_enabled in V1 [PATCH 5/8]. Is there a conclusion now? Maybe I left out some important information.

Hi Baolin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on next-20240529]
[cannot apply to linus/master v6.10-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Baolin-Wang/mm-memory-extend-finish_fault-to-support-large-folio/20240530-100805
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/ec35a23026dd016705d211e85163cabe07681516.1717033868.git.baolin.wang%40linux.alibaba.com
patch subject: [PATCH v3 4/6] mm: shmem: add mTHP support for anonymous shmem
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240530/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project bafda89a0944d947fc4b3b5663185e07a397ac30)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240530/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from mm/shmem.c:28:
In file included from include/linux/ramfs.h:5:
In file included from include/linux/fs_parser.h:11:
In file included from include/linux/fs_context.h:14:
In file included from include/linux/security.h:33:
In file included from include/linux/mm.h:2245:
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~

Quoted 1 line

Quoted 1 line

mm/shmem.c:1748:14: warning: variable 'suitable_orders' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]

     1748 |                 } else if (orders & BIT(HPAGE_PMD_ORDER)) {
          |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    mm/shmem.c:1766:25: note: uninitialized use occurs here
     1766 |                 order = highest_order(suitable_orders);
          |                                       ^~~~~~~~~~~~~~~
    mm/shmem.c:1748:10: note: remove the 'if' if its condition is always true
     1748 |                 } else if (orders & BIT(HPAGE_PMD_ORDER)) {
          |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    mm/shmem.c:1736:31: note: initialize the variable 'suitable_orders' to silence this warning
     1736 |         unsigned long suitable_orders;
          |                                      ^
          |                                       = 0
    2 warnings generated.

Hi Baolin,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Baolin-Wang/mm-memory-extend-finish_fault-to-support-large-folio/20240530-100805
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/ec35a23026dd016705d211e85163cabe07681516.1717033868.git.baolin.wang%40linux.alibaba.com
patch subject: [PATCH v3 4/6] mm: shmem: add mTHP support for anonymous shmem
config: powerpc64-randconfig-r071-20240531 (https://download.01.org/0day-ci/archive/20240602/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project bafda89a0944d947fc4b3b5663185e07a397ac30)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags

| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

smatch warnings:
mm/shmem.c:1766 shmem_alloc_and_add_folio() error: uninitialized symbol 'suitable_orders'.

Anonymous pages have already been supported for multi-size (mTHP) allocation through commit 19eaf44954df, that can allow THP to be configured through the sysfs interface located at '/sys/kernel/mm/transparent_hugepage/hugepage-XXkb/enabled'.

However, the anonymous shmem will ignore the anonymous mTHP rule configured through the sysfs interface, and can only use the PMD-mapped THP, that is not reasonable. Many implement anonymous page sharing through mmap(MAP_SHARED | MAP_ANONYMOUS), especially in database usage scenarios, therefore, users expect to apply an unified mTHP strategy for anonymous pages, also including the anonymous shared pages, in order to enjoy the benefits of mTHP. For example, lower latency than PMD-mapped THP, smaller memory bloat than PMD-mapped THP, contiguous PTEs on ARM architecture to reduce TLB miss etc.

The primary strategy is similar to supporting anonymous mTHP. Introduce a new interface '/mm/transparent_hugepage/hugepage-XXkb/shmem_enabled', which can have all the same values as the top-level '/sys/kernel/mm/transparent_hugepage/shmem_enabled', with adding a new additional "inherit" option. By default all sizes will be set to "never" except PMD size, which is set to "inherit". This ensures backward compatibility with the anonymous shmem enabled of the top level, meanwhile also allows independent control of anonymous shmem enabled for each mTHP.

Use the page fault latency tool to measure the performance of 1G anonymous shmem with 32 threads on my machine environment with: ARM64 Architecture, 32 cores, 125G memory:
base: mm-unstable
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.04s 3.10s 83516.416 2669684.890

mm-unstable + patchset, anon shmem mTHP disabled
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.02s 3.14s 82936.359 2630746.027

mm-unstable + patchset, anon shmem 64K mTHP enabled
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.08s 0.31s 678630.231 17082522.495

From the data above, it is observed that the patchset has a minimal impact when
mTHP is not enabled (some fluctuations observed during testing). When enabling 64K mTHP, there is a significant improvement of the page fault latency.

Quoted 37 lines · Baolin Wang

Anonymous pages have already been supported for multi-size (mTHP) allocation through commit 19eaf44954df, that can allow THP to be configured through the sysfs interface located at '/sys/kernel/mm/transparent_hugepage/hugepage-XXkb/enabled'.

However, the anonymous shmem will ignore the anonymous mTHP rule configured through the sysfs interface, and can only use the PMD-mapped THP, that is not reasonable. Many implement anonymous page sharing through mmap(MAP_SHARED | MAP_ANONYMOUS), especially in database usage scenarios, therefore, users expect to apply an unified mTHP strategy for anonymous pages, also including the anonymous shared pages, in order to enjoy the benefits of mTHP. For example, lower latency than PMD-mapped THP, smaller memory bloat than PMD-mapped THP, contiguous PTEs on ARM architecture to reduce TLB miss etc.

The primary strategy is similar to supporting anonymous mTHP. Introduce a new interface '/mm/transparent_hugepage/hugepage-XXkb/shmem_enabled', which can have all the same values as the top-level '/sys/kernel/mm/transparent_hugepage/shmem_enabled', with adding a new additional "inherit" option. By default all sizes will be set to "never" except PMD size, which is set to "inherit". This ensures backward compatibility with the anonymous shmem enabled of the top level, meanwhile also allows independent control of anonymous shmem enabled for each mTHP.

Use the page fault latency tool to measure the performance of 1G anonymous shmem with 32 threads on my machine environment with: ARM64 Architecture, 32 cores, 125G memory:
base: mm-unstable
user-time    sys_time    faults_per_sec_per_cpu     faults_per_sec 0.04s        3.10s         83516.416                  2669684.890

mm-unstable + patchset, anon shmem mTHP disabled user-time    sys_time    faults_per_sec_per_cpu     faults_per_sec 0.02s        3.14s         82936.359                  2630746.027

mm-unstable + patchset, anon shmem 64K mTHP enabled user-time    sys_time    faults_per_sec_per_cpu     faults_per_sec 0.08s        0.31s         678630.231                 17082522.495

Quoted 3 lines

 From the data above, it is observed that the patchset has a minimal impact when mTHP is not enabled (some fluctuations observed during testing). When enabling 64K mTHP, there is a significant improvement of the page fault latency.

Let me summarize the takeaway from the bi-weekly MM meeting as I understood it, that includes Hugh's feedback on per-block tracking vs.

mTHP:

(1) Per-block tracking

Per-block tracking is currently considered unwarranted complexity in shmem.c. We should try to get it done without that. For any test cases that fail, we should consider if they are actually valid for shmem.

To optimize FALLOC_FL_PUNCH_HOLE for the cases where splitting+freeing is not possible at fallcoate() time, detecting zeropages later and retrying to split+free might be an option, without per-block tracking.

(2) mTHP controls

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that. >
(4) force/disable

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

@Baolin, this series likely does not consider (4) yet. And I suggest we
have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

@Daniel, Pankaj, what are your plans regarding that? It would be great if we could get an understanding on the next steps on !anon shmem.

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Quoted 13 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that. >
(4) force/disable

Quoted 3 lines

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

Sorry, I missed this. So I thould remove the 'force' and 'deny' option for each mTHP, right?

Quoted 14 lines

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

Quoted 3 lines

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

Sure. I will remove the "anonymous thp" terminology from the documentation, but want to still keep it in the commit message, cause I want to start from the anonymous shmem.

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

Quoted 13 lines

Quoted 13 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that. >
(4) force/disable

Quoted 6 lines

Quoted 3 lines

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

Sorry, I missed this. So I thould remove the 'force' and 'deny' option for each mTHP, right?

Yes, that's my understanding. But we have to keep them on the top level for any possible user out there.

Quoted 17 lines

Quoted 16 lines

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

Quoted 3 lines

Sure. I will remove the "anonymous thp" terminology from the documentation, but want to still keep it in the commit message, cause I want to start from the anonymous shmem.

For commit message and friends makes sense. The story should be "controls all of shmem/tmpfs, but support will be added iteratively. The first step is anonymous shmem."

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

Quoted 14 lines

Quoted 14 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that. >
(4) force/disable

Quoted 6 lines

Quoted 3 lines

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

Sorry, I missed this. So I thould remove the 'force' and 'deny' option for each mTHP, right?

Yes, that's my understanding. But we have to keep them on the top level for any possible user out there.

Quoted 14 lines

Quoted 14 lines

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

Quoted 7 lines

Quoted 3 lines

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

Sure. I will remove the "anonymous thp" terminology from the documentation, but want to still keep it in the commit message, cause I want to start from the anonymous shmem.

For commit message and friends makes sense. The story should be "controls all of shmem/tmpfs, but support will be added iteratively. The first step is anonymous shmem."

Signature · 3 lines

Cheers,

David / dhildenb

Quoted 11 lines · David Hildenbrand

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's v3 patches. You may find a version in my integration branch here [2]. I can attach them here if it's preferred.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://gitlab.com/dkruces/linux-next/-/commits/next-20240604-shmem-mthp

The point here is to combine the large folios strategy I proposed with mTHP user controls. Would it make sense to limit the orders to the mapping order calculated based on the size and index?

(quoted diff) +4 -0

@@ -1765,6 +1798,10 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 
                 order = highest_order(suitable_orders);
                 while (suitable_orders) {
+                       if (order > mapping_order) {
+                               order = next_order(&suitable_orders, order);
+                               continue;
+                       }
                         pages = 1UL << order;
                         index = round_down(index, pages);
                         folio = shmem_alloc_folio(gfp, order, info, index);

Note: The branch still need to be adapted to include !anon mm.

Quoted 20 lines · Daniel Gomez

Quoted 11 lines · David Hildenbrand

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's v3 patches. You may find a version in my integration branch here [2]. I can attach them here if it's preferred.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://protect2.fireeye.com/v1/url?k=a23e7c06-c3b56926-a23ff749-74fe485fb347-371ca2bfd5d9869f&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommits%2Fnext-20240604-shmem-mthp

Quoted 3 lines

The point here is to combine the large folios strategy I proposed with mTHP user controls. Would it make sense to limit the orders to the mapping order calculated based on the size and index?

IMO, for !anon shmem, this change makes sense to me. We should respect the size and mTHP should act as a order filter.

For anon shmem, we should ignore the length, which you always set it to PAGE_SIZE in patch [1].

[1] https://protect2.fireeye.com/v1/url?k=0d75a0c6-6cfeb5e6-0d742b89-74fe485fb347-904fa75c8efebdc2&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommit%2Fedf02311fd6d86b355d3aeb74e67c8da6de3c569

Quoted 14 lines

(quoted diff) +4 -0

@@ -1765,6 +1798,10 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 
                 order = highest_order(suitable_orders);
                 while (suitable_orders) {
+                       if (order > mapping_order) {
+                               order = next_order(&suitable_orders, order);
+                               continue;
+                       }
                         pages = 1UL << order;
                         index = round_down(index, pages);
                         folio = shmem_alloc_folio(gfp, order, info, index);

Note: The branch still need to be adapted to include !anon mm.

Quoted 27 lines · Baolin Wang

Quoted 23 lines · Daniel Gomez

Quoted 11 lines · David Hildenbrand

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's v3 patches. You may find a version in my integration branch here [2]. I can attach them here if it's preferred.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://protect2.fireeye.com/v1/url?k=a23e7c06-c3b56926-a23ff749-74fe485fb347-371ca2bfd5d9869f&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommits%2Fnext-20240604-shmem-mthp

The point here is to combine the large folios strategy I proposed with mTHP user controls. Would it make sense to limit the orders to the mapping order calculated based on the size and index?

Quoted 2 lines

IMO, for !anon shmem, this change makes sense to me. We should respect the size and mTHP should act as a order filter.

What about respecing the size when within_size flag is enabled? Then, 'always' would allocate mTHP enabled folios, regardless of the size. And 'never' would ignore mTHP and size. So, 'never' can be used for this 'safe' boot case mentioned in the discussion.

Quoted 4 lines

For anon shmem, we should ignore the length, which you always set it to PAGE_SIZE in patch [1].

[1] https://protect2.fireeye.com/v1/url?k=0d75a0c6-6cfeb5e6-0d742b89-74fe485fb347-904fa75c8efebdc2&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommit%2Fedf02311fd6d86b355d3aeb74e67c8da6de3c569

Since we are ignoring the length, we should ignore any value being passed.

Quoted 14 lines

Quoted 14 lines

(quoted diff) +4 -0

@@ -1765,6 +1798,10 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 
                  order = highest_order(suitable_orders);
                  while (suitable_orders) {
+                       if (order > mapping_order) {
+                               order = next_order(&suitable_orders, order);
+                               continue;
+                       }
                          pages = 1UL << order;
                          index = round_down(index, pages);
                          folio = shmem_alloc_folio(gfp, order, info, index);

Note: The branch still need to be adapted to include !anon mm.

Quoted 31 lines · Daniel Gomez

Quoted 29 lines · Baolin Wang

Quoted 17 lines · Daniel Gomez

Quoted 11 lines · David Hildenbrand

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's v3 patches. You may find a version in my integration branch here [2]. I can attach them here if it's preferred.

Quoted 6 lines

[1] https://lore.kernel.org/all/[email protected]/
[2] https://protect2.fireeye.com/v1/url?k=a23e7c06-c3b56926-a23ff749-74fe485fb347-371ca2bfd5d9869f&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommits%2Fnext-20240604-shmem-mthp

The point here is to combine the large folios strategy I proposed with mTHP user controls. Would it make sense to limit the orders to the mapping order calculated based on the size and index?

IMO, for !anon shmem, this change makes sense to me. We should respect the size and mTHP should act as a order filter.

Quoted 4 lines

What about respecing the size when within_size flag is enabled? Then, 'always' would allocate mTHP enabled folios, regardless of the size. And 'never' would ignore mTHP and size. So, 'never' can be used for this 'safe' boot case mentioned in the discussion.

Looks reasonable to me. What do you think, David?

And what about 'advise' option? Silimar to 'within_size'?

Quoted 39 lines · Baolin Wang

Quoted 35 lines · Daniel Gomez

Quoted 23 lines · Baolin Wang

Quoted 20 lines · Daniel Gomez

Quoted 11 lines · David Hildenbrand

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's v3 patches. You may find a version in my integration branch here [2]. I can attach them here if it's preferred.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://protect2.fireeye.com/v1/url?k=a23e7c06-c3b56926-a23ff749-74fe485fb347-371ca2bfd5d9869f&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommits%2Fnext-20240604-shmem-mthp

Quoted 6 lines

Quoted 3 lines

The point here is to combine the large folios strategy I proposed with mTHP user controls. Would it make sense to limit the orders to the mapping order calculated based on the size and index?

IMO, for !anon shmem, this change makes sense to me. We should respect the size and mTHP should act as a order filter.

What about respecing the size when within_size flag is enabled? Then, 'always' would allocate mTHP enabled folios, regardless of the size. And 'never' would ignore mTHP and size. So, 'never' can be used for this 'safe' boot case mentioned in the discussion.

Quoted 2 lines

Looks reasonable to me. What do you think, David?

That mimics existing PMD-THP behavior, right?

With "within_size", we must not exceed the size, with "always", we may exceed the size.

Quoted 1 line

And what about 'advise' option? Silimar to 'within_size'?

Good question. What's the behavior of PMD-THP? I would assume it behaves like "within_size", because in the common case we mmap (+advise) only within the size of the file, not exceeding it.

(the always option, as I learned during the meeting, is primarily helpful when writing to tmpfs files in an append-fashion. With mmap()+madvise() that doesn't quite happen)

Quoted 39 lines · Baolin Wang

Quoted 35 lines · Daniel Gomez

Quoted 23 lines · Baolin Wang

Quoted 20 lines · Daniel Gomez

Quoted 11 lines · David Hildenbrand

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's v3 patches. You may find a version in my integration branch here [2]. I can attach them here if it's preferred.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://protect2.fireeye.com/v1/url?k=a23e7c06-c3b56926-a23ff749-74fe485fb347-371ca2bfd5d9869f&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommits%2Fnext-20240604-shmem-mthp

Quoted 6 lines

Quoted 3 lines

The point here is to combine the large folios strategy I proposed with mTHP user controls. Would it make sense to limit the orders to the mapping order calculated based on the size and index?

IMO, for !anon shmem, this change makes sense to me. We should respect the size and mTHP should act as a order filter.

What about respecing the size when within_size flag is enabled? Then, 'always' would allocate mTHP enabled folios, regardless of the size. And 'never' would ignore mTHP and size. So, 'never' can be used for this 'safe' boot case mentioned in the discussion.

Quoted 2 lines

Looks reasonable to me. What do you think, David?

That mimics existing PMD-THP behavior, right?

With "within_size", we must not exceed the size, with "always", we may exceed the size.

Quoted 2 lines

And what about 'advise' option? Silimar to 'within_size'?

Good question. What's the behavior of PMD-THP? I would assume it behaves like "within_size", because in the common case we mmap (+advise) only within the size of the file, not exceeding it.

(the always option, as I learned during the meeting, is primarily helpful when writing to tmpfs files in an append-fashion. With mmap()+madvise() that doesn't quite happen)

Signature · 4 lines

-- 
Cheers,

David / dhildenb

Quoted 40 lines · David Hildenbrand

Quoted 39 lines · Baolin Wang

Quoted 35 lines · Daniel Gomez

Quoted 23 lines · Baolin Wang

Quoted 20 lines · Daniel Gomez

Quoted 11 lines · David Hildenbrand

Quoted 9 lines

Quoted 7 lines

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Good, it would be helpful to coordinate with Daniel and Pankaj.

I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's v3 patches. You may find a version in my integration branch here [2]. I can attach them here if it's preferred.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://protect2.fireeye.com/v1/url?k=a23e7c06-c3b56926-a23ff749-74fe485fb347-371ca2bfd5d9869f&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommits%2Fnext-20240604-shmem-mthp

Quoted 6 lines

Quoted 3 lines

The point here is to combine the large folios strategy I proposed with mTHP user controls. Would it make sense to limit the orders to the mapping order calculated based on the size and index?

IMO, for !anon shmem, this change makes sense to me. We should respect the size and mTHP should act as a order filter.

What about respecing the size when within_size flag is enabled? Then, 'always' would allocate mTHP enabled folios, regardless of the size. And 'never' would ignore mTHP and size. So, 'never' can be used for this 'safe' boot case mentioned in the discussion.

Quoted 7 lines

Quoted 2 lines

Looks reasonable to me. What do you think, David?

That mimics existing PMD-THP behavior, right?

With "within_size", we must not exceed the size, with "always", we may exceed the size.

But right now we only check the inode size. With large folio support in write_iter() we can have access to the length as well. I think this would solve (paratially?) the cases where we don't have access to the file size and if we perform writes in bigger chunks.

E.g. xfs_io -t -f -c "pwrite -b 2M -S 0x58 0 2M" /mnt/test/file

For 'within_size', the example above would allocate 512 pages instead of one huge page. After patches [1] [2] we can get the size of the write to allocate whatever mTHP/THP makes more sense for the length being passed.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

Here a quick hack for THP:

(quoted diff) +2 -1

@@ -561,7 +561,8 @@ bool shmem_is_huge(struct inode *inode, pgoff_t index, bool shmem_huge_force,
         case SHMEM_HUGE_WITHIN_SIZE:
                 index = round_up(index + 1, HPAGE_PMD_NR);
                 i_size = round_up(i_size_read(inode), PAGE_SIZE);
-               if (i_size >> PAGE_SHIFT >= index)
+               if ((i_size >> PAGE_SHIFT >= index) ||
+                   (len >> PAGE_SHIFT >= index))
                         return true;
                 fallthrough;
 

Quoted 2 lines

Quoted 5 lines

Quoted 1 line

And what about 'advise' option? Silimar to 'within_size'?

Good question. What's the behavior of PMD-THP? I would assume it behaves like "within_size", because in the common case we mmap (+advise) only within the size of the file, not exceeding it.

It allocates a huge page on request when MADV_HUGEPAGE (regardless of the size).

Quoted 50 lines · David Hildenbrand

Quoted 49 lines · Baolin Wang

Anonymous pages have already been supported for multi-size (mTHP) allocation
through commit 19eaf44954df, that can allow THP to be configured through the
sysfs interface located at '/sys/kernel/mm/transparent_hugepage/hugepage-XXkb/enabled'.

However, the anonymous shmem will ignore the anonymous mTHP rule configured
through the sysfs interface, and can only use the PMD-mapped THP, that is not
reasonable. Many implement anonymous page sharing through mmap(MAP_SHARED |
MAP_ANONYMOUS), especially in database usage scenarios, therefore, users expect
to apply an unified mTHP strategy for anonymous pages, also including the anonymous shared pages, in order to enjoy the benefits of mTHP. For example,
lower latency than PMD-mapped THP, smaller memory bloat than PMD-mapped THP,
contiguous PTEs on ARM architecture to reduce TLB miss etc.

The primary strategy is similar to supporting anonymous mTHP. Introduce a new interface '/mm/transparent_hugepage/hugepage-XXkb/shmem_enabled', which can have all the same values as the top-level '/sys/kernel/mm/transparent_hugepage/shmem_enabled', with adding a new additional "inherit" option. By default all sizes will be set to "never" except PMD size, which is set to "inherit". This ensures backward compatibility
with the anonymous shmem enabled of the top level, meanwhile also allows independent control of anonymous shmem enabled for each mTHP.

Use the page fault latency tool to measure the performance of 1G anonymous shmem
with 32 threads on my machine environment with: ARM64 Architecture, 32 cores,
125G memory:
base: mm-unstable
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.04s 3.10s 83516.416 2669684.890

mm-unstable + patchset, anon shmem mTHP disabled
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.02s 3.14s 82936.359 2630746.027

mm-unstable + patchset, anon shmem 64K mTHP enabled
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.08s 0.31s 678630.231 17082522.495

Quoted 8 lines

Quoted 5 lines

From the data above, it is observed that the patchset has a minimal
impact when
mTHP is not enabled (some fluctuations observed during testing). When enabling 64K
mTHP, there is a significant improvement of the page fault latency.

Let me summarize the takeaway from the bi-weekly MM meeting as I understood it, that includes Hugh's feedback on per-block tracking vs.

Thanks David for the summarization.

Quoted 12 lines

mTHP:

(1) Per-block tracking

Per-block tracking is currently considered unwarranted complexity in shmem.c. We should try to get it done without that. For any test cases that fail, we should consider if they are actually valid for shmem.

To optimize FALLOC_FL_PUNCH_HOLE for the cases where splitting+freeing is not possible at fallcoate() time, detecting zeropages later and retrying to split+free might be an option, without per-block tracking.

Quoted 8 lines

(2) mTHP controls

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Yes, that's what I thought and in my TODO list.

Quoted 12 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that. >
(4) force/disable

Quoted 3 lines

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

Sorry, I missed this. So I thould remove the 'force' and 'deny' option for each mTHP, right?

Quoted 37 lines · Baolin Wang

Anonymous pages have already been supported for multi-size (mTHP) allocation through commit 19eaf44954df, that can allow THP to be configured through the sysfs interface located at '/sys/kernel/mm/transparent_hugepage/hugepage-XXkb/enabled'.

However, the anonymous shmem will ignore the anonymous mTHP rule configured through the sysfs interface, and can only use the PMD-mapped THP, that is not reasonable. Many implement anonymous page sharing through mmap(MAP_SHARED | MAP_ANONYMOUS), especially in database usage scenarios, therefore, users expect to apply an unified mTHP strategy for anonymous pages, also including the anonymous shared pages, in order to enjoy the benefits of mTHP. For example, lower latency than PMD-mapped THP, smaller memory bloat than PMD-mapped THP, contiguous PTEs on ARM architecture to reduce TLB miss etc.

The primary strategy is similar to supporting anonymous mTHP. Introduce a new interface '/mm/transparent_hugepage/hugepage-XXkb/shmem_enabled', which can have all the same values as the top-level '/sys/kernel/mm/transparent_hugepage/shmem_enabled', with adding a new additional "inherit" option. By default all sizes will be set to "never" except PMD size, which is set to "inherit". This ensures backward compatibility with the anonymous shmem enabled of the top level, meanwhile also allows independent control of anonymous shmem enabled for each mTHP.

Use the page fault latency tool to measure the performance of 1G anonymous shmem with 32 threads on my machine environment with: ARM64 Architecture, 32 cores, 125G memory:
base: mm-unstable
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.04s 3.10s 83516.416 2669684.890

mm-unstable + patchset, anon shmem mTHP disabled
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.02s 3.14s 82936.359 2630746.027

mm-unstable + patchset, anon shmem 64K mTHP enabled
user-time sys_time faults_per_sec_per_cpu faults_per_sec
0.08s 0.31s 678630.231 17082522.495

Quoted 3 lines

From the data above, it is observed that the patchset has a minimal impact when
mTHP is not enabled (some fluctuations observed during testing). When enabling 64K mTHP, there is a significant improvement of the page fault latency.

Let me summarize the takeaway from the bi-weekly MM meeting as I understood it, that includes Hugh's feedback on per-block tracking vs. mTHP:

(1) Per-block tracking

Per-block tracking is currently considered unwarranted complexity in shmem.c. We should try to get it done without that. For any test cases that fail, we should consider if they are actually valid for shmem.

To optimize FALLOC_FL_PUNCH_HOLE for the cases where splitting+freeing is not possible at fallcoate() time, detecting zeropages later and retrying to split+free might be an option, without per-block tracking.

(2) mTHP controls

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

Further, the mTHP controls should control all of shmem, not only "anonymous shmem".

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that.

(4) force/disable

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

@Daniel, Pankaj, what are your plans regarding that? It would be great if we could get an understanding on the next steps on !anon shmem.

Signature · 4 lines

-- 
Cheers,

David / dhildenb

Quoted 2 lines

Let me summarize the takeaway from the bi-weekly MM meeting as I understood it, that includes Hugh's feedback on per-block tracking vs. mTHP:

Thanks David for the summary. Please, find below some follow up questions.

I want understand if zeropage scanning overhead is preferred over per-block tracking complexity or if we still need to verify this.

Quoted 6 lines

(1) Per-block tracking

Per-block tracking is currently considered unwarranted complexity in shmem.c. We should try to get it done without that. For any test cases that fail, we should consider if they are actually valid for shmem.

I agree it was unwarranted complexity but only if this is just to fix lseek() as we can simply make the test pass by checking if holes are reported as data. That would be the minimum required for lseek() to be compliant with the syscall.

How can we use per-block tracking for reclaiming memory and what changes would be needed? Or is per-block really a non-viable option?

Clearly, if per-block is viable option, shmem_fault() bug would required to be fixed first. Any ideas on how to make it reproducible?

The alternatives discussed where sub-page refcounting and zeropage scanning.

The first one is not possible (IIUC) because of a refactor years back that simplified the code and also requires extra complexity. The second option would require additional overhead as we would involve scanning.

Quoted 7 lines

To optimize FALLOC_FL_PUNCH_HOLE for the cases where splitting+freeing is not possible at fallcoate() time, detecting zeropages later and retrying to split+free might be an option, without per-block tracking.

Quoted 5 lines

(2) mTHP controls

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

That was clear for me too. But what is the reason we want to boot in 'safe mode'? What are the implications of not respecing that?

As I understood from the call, mTHP with sysctl knobs is preferred over optimistic falloc/write allocation? But is still unclear to me why the former is preferred.

Is large folios a non-viable option?

Quoted 33 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that.

(4) force/disable

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

Quoted 2 lines

@Daniel, Pankaj, what are your plans regarding that? It would be great if we could get an understanding on the next steps on !anon shmem.

I realize I've raised so many questions, but it's essential for us to grasp the mm concerns and usage scenarios. This understanding will provide clarity on the direction regarding folios for !anon shmem.

Hi Daniel,

Quoted 19 lines

Quoted 2 lines

Let me summarize the takeaway from the bi-weekly MM meeting as I understood it, that includes Hugh's feedback on per-block tracking vs. mTHP:

Thanks David for the summary. Please, find below some follow up questions.

I want understand if zeropage scanning overhead is preferred over per-block tracking complexity or if we still need to verify this.

Quoted 6 lines

(1) Per-block tracking

Per-block tracking is currently considered unwarranted complexity in shmem.c. We should try to get it done without that. For any test cases that fail, we should consider if they are actually valid for shmem.

I agree it was unwarranted complexity but only if this is just to fix lseek() as we can simply make the test pass by checking if holes are reported as data. That would be the minimum required for lseek() to be compliant with the syscall.

Quoted 2 lines

How can we use per-block tracking for reclaiming memory and what changes would be needed? Or is per-block really a non-viable option?

The interesting thing is: with mTHP toggles it is opt-in -- like for PMD-sized THP with shmem_enabled -- and we don't have to be that concerned about this problem right now.

Quoted 6 lines

Clearly, if per-block is viable option, shmem_fault() bug would required to be fixed first. Any ideas on how to make it reproducible?

The alternatives discussed where sub-page refcounting and zeropage scanning.

Yeah, I don't think sub-page refcounting is a feasible (and certainly not desired ;) ) option in the folio world.

Quoted 3 lines

The first one is not possible (IIUC) because of a refactor years back that simplified the code and also requires extra complexity. The second option would require additional overhead as we would involve scanning.

We'll likely need something similar (scanning, tracking?) for anonymous memory as well. There was a proposal for a THP shrinker some time ago, that would solve part of the problem.

For example, for shmem you could simply flag folios that failed splitting during fallocate() as reclaim candidates and try to reclaim memory later. So you don't have to scan arbitrary folios (which might also be desired, though).

Quoted 16 lines

Quoted 8 lines

To optimize FALLOC_FL_PUNCH_HOLE for the cases where splitting+freeing is not possible at fallcoate() time, detecting zeropages later and retrying to split+free might be an option, without per-block tracking.

Quoted 5 lines

(2) mTHP controls

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

That was clear for me too. But what is the reason we want to boot in 'safe mode'? What are the implications of not respecing that?

[...]

Quoted 1 line

Quoted 3 lines

As I understood from the call, mTHP with sysctl knobs is preferred over optimistic falloc/write allocation? But is still unclear to me why the former is preferred.

I think Hugh's point was that this should be an opt-in feature, just like PMD-sized THP started out, and still is, an opt-in feature.

Problematic interaction with khugepaged (that will be fixed) is one thing, interaction with memory reclaim (without any kind of memory reclaim mechanisms in place) might be another one. Controlling and tuning for specific folio sizes might be another one Hugh raised. [summarizing what I recall from the discussion, there might be more].

Quoted 2 lines

Is large folios a non-viable option?

I think you mean "allocating large folios without user space control".

Because mTHP gives user space full control, to the point where you can enable all sizes and obtain the same result.

Quoted 37 lines

Quoted 36 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that.

(4) force/disable

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

@Daniel, Pankaj, what are your plans regarding that? It would be great if we could get an understanding on the next steps on !anon shmem.

Quoted 3 lines

I realize I've raised so many questions, but it's essential for us to grasp the mm concerns and usage scenarios. This understanding will provide clarity on the direction regarding folios for !anon shmem.

If I understood correctly, Hugh had strong feelings against not respecting mTHP toggles for shmem. Without per-block tracking, I agree with that.

Signature · 4 lines

-- 
Cheers,

David / dhildenb

Quoted 6 lines

Quoted 2 lines

How can we use per-block tracking for reclaiming memory and what changes would be needed? Or is per-block really a non-viable option?

The interesting thing is: with mTHP toggles it is opt-in -- like for PMD-sized THP with shmem_enabled -- and we don't have to be that concerned about this problem right now.

Without respecting the size when allocating large folios, mTHP toggles would over allocate. My proposal added earlier to this thread is to combine the 2 to avoid that case. Otherwise, shouldn't we try to find a solution for the reclaiming path?

Quoted 15 lines

Quoted 7 lines

Clearly, if per-block is viable option, shmem_fault() bug would required to be fixed first. Any ideas on how to make it reproducible?

The alternatives discussed where sub-page refcounting and zeropage scanning.

Yeah, I don't think sub-page refcounting is a feasible (and certainly not desired ;) ) option in the folio world.

Quoted 3 lines

The first one is not possible (IIUC) because of a refactor years back that simplified the code and also requires extra complexity. The second option would require additional overhead as we would involve scanning.

Quoted 3 lines

We'll likely need something similar (scanning, tracking?) for anonymous memory as well. There was a proposal for a THP shrinker some time ago, that would solve part of the problem.

It's good to know we have the same problem in different places. I'm more inclined to keep the information rather than adding an extra overhead. Unless the complexity is really overwhelming. Considering the concerns here, not sure how much should we try merging with iomap as per Ritesh's suggestion.

The THP shrinker, could you please confirm if it is this following thread?

https://lore.kernel.org/all/[email protected]/

Quoted 21 lines

Quoted 13 lines

Quoted 10 lines

To optimize FALLOC_FL_PUNCH_HOLE for the cases where splitting+freeing is not possible at fallcoate() time, detecting zeropages later and retrying to split+free might be an option, without per-block tracking.

(2) mTHP controls

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

That was clear for me too. But what is the reason we want to boot in 'safe mode'? What are the implications of not respecing that?

[...]

Quoted 4 lines

As I understood from the call, mTHP with sysctl knobs is preferred over optimistic falloc/write allocation? But is still unclear to me why the former is preferred.

Quoted 2 lines

I think Hugh's point was that this should be an opt-in feature, just like PMD-sized THP started out, and still is, an opt-in feature.

I'd be keen to understand the use case for this. Even the current THP controls we have in tmpfs. I guess these are just scenarios with no swap involved? Are these use cases the same for both tmpfs and shmem anon mm?

Quoted 40 lines

Quoted 39 lines

Quoted 29 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that.

(4) force/disable

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

Quoted 6 lines

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

@Daniel, Pankaj, what are your plans regarding that? It would be great if we could get an understanding on the next steps on !anon shmem.

I realize I've raised so many questions, but it's essential for us to grasp the mm concerns and usage scenarios. This understanding will provide clarity on the direction regarding folios for !anon shmem.

Quoted 2 lines

If I understood correctly, Hugh had strong feelings against not respecting mTHP toggles for shmem. Without per-block tracking, I agree with that.

My understanding was the same. But I have this follow-up question: should we respect mTHP toggles without considering mapping constraints (size and index)? Or perhaps we should use within_size where we can fit this intermediate approach, as long as mTHP granularity is respected?

[...]

Quoted 11 lines

Quoted 6 lines

Quoted 2 lines

How can we use per-block tracking for reclaiming memory and what changes would be needed? Or is per-block really a non-viable option?

The interesting thing is: with mTHP toggles it is opt-in -- like for PMD-sized THP with shmem_enabled -- and we don't have to be that concerned about this problem right now.

Without respecting the size when allocating large folios, mTHP toggles would over allocate. My proposal added earlier to this thread is to combine the 2 to avoid that case. Otherwise, shouldn't we try to find a solution for the reclaiming path?

I think at some point we'll really have to do a better job at reclaiming (either memory-overallocation, PUNCHHOLE that couldn't split, but maybe also pages that are now all-zero again and could be reclaimed again).

Quoted 15 lines

Quoted 15 lines

Quoted 7 lines

Clearly, if per-block is viable option, shmem_fault() bug would required to be fixed first. Any ideas on how to make it reproducible?

The alternatives discussed where sub-page refcounting and zeropage scanning.

Yeah, I don't think sub-page refcounting is a feasible (and certainly not desired ;) ) option in the folio world.

Quoted 3 lines

The first one is not possible (IIUC) because of a refactor years back that simplified the code and also requires extra complexity. The second option would require additional overhead as we would involve scanning.

Quoted 8 lines

Quoted 3 lines

We'll likely need something similar (scanning, tracking?) for anonymous memory as well. There was a proposal for a THP shrinker some time ago, that would solve part of the problem.

It's good to know we have the same problem in different places. I'm more inclined to keep the information rather than adding an extra overhead. Unless the complexity is really overwhelming. Considering the concerns here, not sure how much should we try merging with iomap as per Ritesh's suggestion.

As raised in the meeting, I do see value in maintaining the information; but I also see why Hugh and Kirill think this might be unwarranted complexity in shmem.c. Meaning: we might be able to achieve something similar without it, and we don't have to solve the problem right now to benefit from large folios.

Quoted 4 lines

The THP shrinker, could you please confirm if it is this following thread?

https://lore.kernel.org/all/[email protected]/

Yes, although there was no follow up so far. Possibly, because in the current khugepaged approach, there will be a constant back-and-forth between khugepaged collapsing memory (and wasting memory in the default setting), and the THP shrinker reclaiming memory; doesn't sound quite effective :) That needs more thought IMHO.

[...]

Quoted 21 lines

Quoted 21 lines

Quoted 13 lines

Quoted 10 lines

To optimize FALLOC_FL_PUNCH_HOLE for the cases where splitting+freeing is not possible at fallcoate() time, detecting zeropages later and retrying to split+free might be an option, without per-block tracking.

(2) mTHP controls

As a default, we should not be using large folios / mTHP for any shmem, just like we did with THP via shmem_enabled. This is what this series currently does, and is aprt of the whole mTHP user-space interface design.

That was clear for me too. But what is the reason we want to boot in 'safe mode'? What are the implications of not respecing that?

[...]

Quoted 4 lines

As I understood from the call, mTHP with sysctl knobs is preferred over optimistic falloc/write allocation? But is still unclear to me why the former is preferred.

Quoted 6 lines

Quoted 2 lines

I think Hugh's point was that this should be an opt-in feature, just like PMD-sized THP started out, and still is, an opt-in feature.

I'd be keen to understand the use case for this. Even the current THP controls we have in tmpfs. I guess these are just scenarios with no swap involved? Are these use cases the same for both tmpfs and shmem anon mm?

Systems without swap are one case I think. The other reason for a toggle in the past was to be able to disable it to troubleshoot issues (Hugh mentioned in the meeting about unlocking new code paths in shmem.c only with a toggle).

Staring at my Fedora system:

$ cat /sys/kernel/mm/transparent_hugepage/shmem_enabled
always within_size advise [never] deny force

Maybe because it uses tmpfs to mount /tmp (interesting article on lwn.net about that) and people are concerned about the side-effects (that can currently waste memory, or result in more reclaim work being required when exceeding file sizes).

For VMs, I know that shmem+THP with memory ballooning is problematic and not really recommended.

[...]

Quoted 40 lines

Quoted 40 lines

Quoted 39 lines

Quoted 29 lines

Also, we should properly fallback within the configured sizes, and not jump "over" configured sizes. Unless there is a good reason.

(3) khugepaged

khugepaged needs to handle larger folios properly as well. Until fixed, using smaller THP sizes as fallback might prohibit collapsing a PMD-sized THP later. But really, khugepaged needs to be fixed to handle that.

(4) force/disable

These settings are rather testing artifacts from the old ages. We should not add them to the per-size toggles. We might "inherit" it from the global one, though.

"within_size" might have value, and especially for consistency, we should have them per size.

So, this series only tackles anonymous shmem, which is a good starting point. Ideally, we'd get support for other shmem (especially during fault time) soon afterwards, because we won't be adding separate toggles for that from the interface POV, and having inconsistent behavior between kernel versions would be a bit unfortunate.

Quoted 6 lines

@Baolin, this series likely does not consider (4) yet. And I suggest we have to take a lot of the "anonymous thp" terminology out of this series, especially when it comes to documentation.

@Daniel, Pankaj, what are your plans regarding that? It would be great if we could get an understanding on the next steps on !anon shmem.

I realize I've raised so many questions, but it's essential for us to grasp the mm concerns and usage scenarios. This understanding will provide clarity on the direction regarding folios for !anon shmem.

Quoted 7 lines

Quoted 2 lines

If I understood correctly, Hugh had strong feelings against not respecting mTHP toggles for shmem. Without per-block tracking, I agree with that.

My understanding was the same. But I have this follow-up question: should we respect mTHP toggles without considering mapping constraints (size and index)? Or perhaps we should use within_size where we can fit this intermediate approach, as long as mTHP granularity is respected?

Likely we should do exactly the same as we would do with the existing PMD-sized THPs.

I recall in the meeting that we discussed that always vs. within_size seems to have some value, and that we should respect that setting like we did for PMD-sized THP.

Which other mapping constraints could we have?

Signature · 4 lines

-- 
Cheers,

David / dhildenb