anon_rmap is used to determine whether the new allocated folio is anonymous.
Rename it to something more meaningul like new_anon_folio and make it boolean,
as we use it like that.
While we are at it, drop 'new_pagecache_folio' as 'new_anon_folio' is enough to
check whether we need to restore the consumed reservation.
Signed-off-by: Oscar Salvador <osalvador@suse.de>
---
mm/hugetlb.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 1a5f713c1e4c..57bb8b2dce21 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6427,17 +6427,16 @@ static bool hugetlb_pte_stable(struct hstate *h, struct mm_struct *mm, unsigned
static vm_fault_t hugetlb_no_page(struct address_space *mapping,
struct vm_fault *vmf)
{
+ u32 hash = hugetlb_fault_mutex_hash(mapping, vmf->pgoff);
+ bool new_folio, new_anon_folio = false;
struct vm_area_struct *vma = vmf->vma;
struct mm_struct *mm = vma->vm_mm;
struct hstate *h = hstate_vma(vma);
vm_fault_t ret = VM_FAULT_SIGBUS;
- int anon_rmap = 0;
- unsigned long size;
+ bool folio_locked = true;
struct folio *folio;
+ unsigned long size;
pte_t new_pte;
- bool new_folio, new_pagecache_folio = false;
- u32 hash = hugetlb_fault_mutex_hash(mapping, vmf->pgoff);
- bool folio_locked = true;
/*
* Currently, we are forced to kill the process in the event the
@@ -6518,6 +6517,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
folio_zero_user(folio, vmf->real_address);
__folio_mark_uptodate(folio);
new_folio = true;
+ new_anon_folio = !(vma->vm_flags & VM_MAYSHARE);
if (vma->vm_flags & VM_MAYSHARE) {
int err = hugetlb_add_to_page_cache(folio, mapping,
@@ -6536,10 +6536,8 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
ret = VM_FAULT_SIGBUS;
goto out;
}
- new_pagecache_folio = true;
} else {
folio_lock(folio);
- anon_rmap = 1;
}
} else {
/*
@@ -6588,7 +6586,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
if (!pte_same(huge_ptep_get(mm, vmf->address, vmf->pte), vmf->orig_pte))
goto backout;
- if (anon_rmap)
+ if (new_anon_folio)
hugetlb_add_new_anon_rmap(folio, vma, vmf->address);
else
hugetlb_add_file_rmap(folio);
@@ -6604,7 +6602,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
hugetlb_count_add(pages_per_huge_page(h), mm);
if ((vmf->flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
/* No need to lock file folios. See comment in hugetlb_fault() */
- if (!anon_rmap) {
+ if (!new_anon_folio) {
folio_locked = false;
folio_unlock(folio);
}
@@ -6640,7 +6638,8 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
backout:
spin_unlock(vmf->ptl);
backout_unlocked:
- if (new_folio && !new_pagecache_folio)
+ /* We only need to restore reservations for private mappings */
+ if (new_folio && new_anon_folio)
restore_reserve_on_error(h, vma, vmf->address, folio);
if (folio_locked)
--
2.50.0On 20.06.25 14:30, Oscar Salvador wrote:
> anon_rmap is used to determine whether the new allocated folio is anonymous.
> Rename it to something more meaningul like new_anon_folio and make it boolean,
> as we use it like that.
> While we are at it, drop 'new_pagecache_folio' as 'new_anon_folio' is enough to
> check whether we need to restore the consumed reservation.
>
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> ---
}
> @@ -6640,7 +6638,8 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
> backout:
> spin_unlock(vmf->ptl);
> backout_unlocked:
> - if (new_folio && !new_pagecache_folio)
> + /* We only need to restore reservations for private mappings */
> + if (new_folio && new_anon_folio)
Could this be simplified to
if (new_anon_folio) {
...
--
Cheers,
David / dhildenb
On Mon, Jun 23, 2025 at 04:13:17PM +0200, David Hildenbrand wrote:
> > - if (new_folio && !new_pagecache_folio)
> > + /* We only need to restore reservations for private mappings */
> > + if (new_folio && new_anon_folio)
>
> Could this be simplified to
>
> if (new_anon_folio) {
Yes, definitely, will do in the next version.
--
Oscar Salvador
SUSE Labs
On Fri, Jun 20, 2025 at 02:30:12PM +0200, Oscar Salvador wrote:
> anon_rmap is used to determine whether the new allocated folio is anonymous.
> Rename it to something more meaningul like new_anon_folio and make it boolean,
> as we use it like that.
> While we are at it, drop 'new_pagecache_folio' as 'new_anon_folio' is enough to
> check whether we need to restore the consumed reservation.
>
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> ---
> mm/hugetlb.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
[...]
> @@ -6518,6 +6517,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
> folio_zero_user(folio, vmf->real_address);
> __folio_mark_uptodate(folio);
> new_folio = true;
> + new_anon_folio = !(vma->vm_flags & VM_MAYSHARE);
>
> if (vma->vm_flags & VM_MAYSHARE) {
> int err = hugetlb_add_to_page_cache(folio, mapping,
> @@ -6536,10 +6536,8 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
> ret = VM_FAULT_SIGBUS;
> goto out;
> }
> - new_pagecache_folio = true;
> } else {
> folio_lock(folio);
> - anon_rmap = 1;
Let's just on top:
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 57bb8b2dce21..f6ea1864ce5c 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6517,7 +6517,6 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
folio_zero_user(folio, vmf->real_address);
__folio_mark_uptodate(folio);
new_folio = true;
- new_anon_folio = !(vma->vm_flags & VM_MAYSHARE);
if (vma->vm_flags & VM_MAYSHARE) {
int err = hugetlb_add_to_page_cache(folio, mapping,
@@ -6537,6 +6536,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
goto out;
}
} else {
+ new_anon_folio = true;
folio_lock(folio);
}
} else {
which is more explicit :-)
--
Oscar Salvador
SUSE Labs
© 2016 - 2025 Red Hat, Inc.