* [PATCH v5 19/21] swap: simplify swapoff using virtual swap
From: Nhat Pham @ 2026-03-20 19:27 UTC (permalink / raw)
To: kasong
Cc: Liam.Howlett, akpm, apopple, axelrasmussen, baohua, baolin.wang,
bhe, byungchul, cgroups, chengming.zhou, chrisl, corbet, david,
dev.jain, gourry, hannes, hughd, jannh, joshua.hahnjy, lance.yang,
lenb, linux-doc, linux-kernel, linux-mm, linux-pm,
lorenzo.stoakes, matthew.brost, mhocko, muchun.song, npache,
nphamcs, pavel, peterx, peterz, pfalcato, rafael, rakie.kim,
roman.gushchin, rppt, ryan.roberts, shakeel.butt, shikemeng,
surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed, yuanchu,
zhengqi.arch, ziy, kernel-team, riel
In-Reply-To: <20260320192735.748051-1-nphamcs@gmail.com>
This patch presents the second applications of virtual swap design -
simplifying and optimizing swapoff.
With virtual swap slots stored at page table entries and used as indices
to various swap-related data structures, we no longer have to perform a
page table walk in swapoff. Simply iterate through all the allocated
swap slots on the swapfile, find their corresponding virtual swap slots,
and fault them in.
This is significantly cleaner, as well as slightly more performant,
especially when there are a lot of unrelated VMAs (since the old swapoff
code would have to traverse through all of them).
In a simple benchmark, in which we swapoff a 32 GB swapfile that is 50%
full, and in which there is a process that maps a 128GB file into
memory:
Baseline:
sys: 11.48s
New Design:
sys: 9.96s
Disregarding the real time reduction (which is mostly due to more IO
asynchrony), the new design reduces the kernel CPU time by about 13%.
Signed-off-by: Nhat Pham <nphamcs@gmail.com>
---
include/linux/shmem_fs.h | 7 +-
mm/filemap.c | 14 +-
mm/shmem.c | 196 +---------------
mm/swapfile.c | 474 +++++++++------------------------------
4 files changed, 126 insertions(+), 565 deletions(-)
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index e2069b3179c41..bac6b6cafe89c 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -41,17 +41,13 @@ struct shmem_inode_info {
unsigned long swapped; /* subtotal assigned to swap */
union {
struct offset_ctx dir_offsets; /* stable directory offsets */
- struct {
- struct list_head shrinklist; /* shrinkable hpage inodes */
- struct list_head swaplist; /* chain of maybes on swap */
- };
+ struct list_head shrinklist; /* shrinkable hpage inodes */
};
struct timespec64 i_crtime; /* file creation time */
struct shared_policy policy; /* NUMA memory alloc policy */
struct simple_xattrs xattrs; /* list of xattrs */
pgoff_t fallocend; /* highest fallocate endindex */
unsigned int fsflags; /* for FS_IOC_[SG]ETFLAGS */
- atomic_t stop_eviction; /* hold when working on inode */
#ifdef CONFIG_TMPFS_QUOTA
struct dquot __rcu *i_dquot[MAXQUOTAS];
#endif
@@ -127,7 +123,6 @@ struct page *shmem_read_mapping_page_gfp(struct address_space *mapping,
int shmem_writeout(struct folio *folio, struct swap_iocb **plug,
struct list_head *folio_list);
void shmem_truncate_range(struct inode *inode, loff_t start, uoff_t end);
-int shmem_unuse(unsigned int type);
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
unsigned long shmem_allowable_huge_orders(struct inode *inode,
diff --git a/mm/filemap.c b/mm/filemap.c
index ebd75684cb0a7..53aad273ea2f1 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -4614,13 +4614,13 @@ static void filemap_cachestat(struct address_space *mapping,
/*
* Getting a swap entry from the shmem
- * inode means we beat
- * shmem_unuse(). rcu_read_lock()
- * ensures swapoff waits for us before
- * freeing the swapper space. However,
- * we can race with swapping and
- * invalidation, so there might not be
- * a shadow in the swapcache (yet).
+ * inode means we beat swapoff.
+ * rcu_read_lock() ensures swapoff waits
+ * for us before freeing the swapper
+ * space. However, we can race with
+ * swapping and invalidation, so there
+ * might not be a shadow in the swapcache
+ * (yet).
*/
shadow = swap_cache_get_shadow(swp);
if (!shadow)
diff --git a/mm/shmem.c b/mm/shmem.c
index 3a346cca114ab..984e01ea88d3c 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -290,9 +290,6 @@ bool vma_is_shmem(const struct vm_area_struct *vma)
return vma_is_anon_shmem(vma) || vma->vm_ops == &shmem_vm_ops;
}
-static LIST_HEAD(shmem_swaplist);
-static DEFINE_SPINLOCK(shmem_swaplist_lock);
-
#ifdef CONFIG_TMPFS_QUOTA
static int shmem_enable_quotas(struct super_block *sb,
@@ -1413,16 +1410,6 @@ static void shmem_evict_inode(struct inode *inode)
}
spin_unlock(&sbinfo->shrinklist_lock);
}
- while (!list_empty(&info->swaplist)) {
- /* Wait while shmem_unuse() is scanning this inode... */
- wait_var_event(&info->stop_eviction,
- !atomic_read(&info->stop_eviction));
- spin_lock(&shmem_swaplist_lock);
- /* ...but beware of the race if we peeked too early */
- if (!atomic_read(&info->stop_eviction))
- list_del_init(&info->swaplist);
- spin_unlock(&shmem_swaplist_lock);
- }
}
simple_xattrs_free(&info->xattrs, sbinfo->max_inodes ? &freed : NULL);
@@ -1435,153 +1422,6 @@ static void shmem_evict_inode(struct inode *inode)
#endif
}
-static unsigned int shmem_find_swap_entries(struct address_space *mapping,
- pgoff_t start, struct folio_batch *fbatch,
- pgoff_t *indices, unsigned int type)
-{
- XA_STATE(xas, &mapping->i_pages, start);
- struct folio *folio;
- swp_entry_t entry;
- swp_slot_t slot;
-
- rcu_read_lock();
- xas_for_each(&xas, folio, ULONG_MAX) {
- if (xas_retry(&xas, folio))
- continue;
-
- if (!xa_is_value(folio))
- continue;
-
- entry = radix_to_swp_entry(folio);
- slot = swp_entry_to_swp_slot(entry);
-
- /*
- * swapin error entries can be found in the mapping. But they're
- * deliberately ignored here as we've done everything we can do.
- */
- if (!slot.val || swp_slot_type(slot) != type)
- continue;
-
- indices[folio_batch_count(fbatch)] = xas.xa_index;
- if (!folio_batch_add(fbatch, folio))
- break;
-
- if (need_resched()) {
- xas_pause(&xas);
- cond_resched_rcu();
- }
- }
- rcu_read_unlock();
-
- return folio_batch_count(fbatch);
-}
-
-/*
- * Move the swapped pages for an inode to page cache. Returns the count
- * of pages swapped in, or the error in case of failure.
- */
-static int shmem_unuse_swap_entries(struct inode *inode,
- struct folio_batch *fbatch, pgoff_t *indices)
-{
- int i = 0;
- int ret = 0;
- int error = 0;
- struct address_space *mapping = inode->i_mapping;
-
- for (i = 0; i < folio_batch_count(fbatch); i++) {
- struct folio *folio = fbatch->folios[i];
-
- error = shmem_swapin_folio(inode, indices[i], &folio, SGP_CACHE,
- mapping_gfp_mask(mapping), NULL, NULL);
- if (error == 0) {
- folio_unlock(folio);
- folio_put(folio);
- ret++;
- }
- if (error == -ENOMEM)
- break;
- error = 0;
- }
- return error ? error : ret;
-}
-
-/*
- * If swap found in inode, free it and move page from swapcache to filecache.
- */
-static int shmem_unuse_inode(struct inode *inode, unsigned int type)
-{
- struct address_space *mapping = inode->i_mapping;
- pgoff_t start = 0;
- struct folio_batch fbatch;
- pgoff_t indices[PAGEVEC_SIZE];
- int ret = 0;
-
- do {
- folio_batch_init(&fbatch);
- if (!shmem_find_swap_entries(mapping, start, &fbatch,
- indices, type)) {
- ret = 0;
- break;
- }
-
- ret = shmem_unuse_swap_entries(inode, &fbatch, indices);
- if (ret < 0)
- break;
-
- start = indices[folio_batch_count(&fbatch) - 1];
- } while (true);
-
- return ret;
-}
-
-/*
- * Read all the shared memory data that resides in the swap
- * device 'type' back into memory, so the swap device can be
- * unused.
- */
-int shmem_unuse(unsigned int type)
-{
- struct shmem_inode_info *info, *next;
- int error = 0;
-
- if (list_empty(&shmem_swaplist))
- return 0;
-
- spin_lock(&shmem_swaplist_lock);
-start_over:
- list_for_each_entry_safe(info, next, &shmem_swaplist, swaplist) {
- if (!info->swapped) {
- list_del_init(&info->swaplist);
- continue;
- }
- /*
- * Drop the swaplist mutex while searching the inode for swap;
- * but before doing so, make sure shmem_evict_inode() will not
- * remove placeholder inode from swaplist, nor let it be freed
- * (igrab() would protect from unlink, but not from unmount).
- */
- atomic_inc(&info->stop_eviction);
- spin_unlock(&shmem_swaplist_lock);
-
- error = shmem_unuse_inode(&info->vfs_inode, type);
- cond_resched();
-
- spin_lock(&shmem_swaplist_lock);
- if (atomic_dec_and_test(&info->stop_eviction))
- wake_up_var(&info->stop_eviction);
- if (error)
- break;
- if (list_empty(&info->swaplist))
- goto start_over;
- next = list_next_entry(info, swaplist);
- if (!info->swapped)
- list_del_init(&info->swaplist);
- }
- spin_unlock(&shmem_swaplist_lock);
-
- return error;
-}
-
/**
* shmem_writeout - Write the folio to swap
* @folio: The folio to write
@@ -1668,24 +1508,9 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug,
}
if (!folio_alloc_swap(folio)) {
- bool first_swapped = shmem_recalc_inode(inode, 0, nr_pages);
int error;
- /*
- * Add inode to shmem_unuse()'s list of swapped-out inodes,
- * if it's not already there. Do it now before the folio is
- * removed from page cache, when its pagelock no longer
- * protects the inode from eviction. And do it now, after
- * we've incremented swapped, because shmem_unuse() will
- * prune a !swapped inode from the swaplist.
- */
- if (first_swapped) {
- spin_lock(&shmem_swaplist_lock);
- if (list_empty(&info->swaplist))
- list_add(&info->swaplist, &shmem_swaplist);
- spin_unlock(&shmem_swaplist_lock);
- }
-
+ shmem_recalc_inode(inode, 0, nr_pages);
swap_shmem_alloc(folio->swap, nr_pages);
shmem_delete_from_page_cache(folio, swp_to_radix_entry(folio->swap));
@@ -2116,12 +1941,12 @@ static struct folio *shmem_swap_alloc_folio(struct inode *inode,
}
/*
- * When a page is moved from swapcache to shmem filecache (either by the
- * usual swapin of shmem_get_folio_gfp(), or by the less common swapoff of
- * shmem_unuse_inode()), it may have been read in earlier from swap, in
- * ignorance of the mapping it belongs to. If that mapping has special
- * constraints (like the gma500 GEM driver, which requires RAM below 4GB),
- * we may need to copy to a suitable page before moving to filecache.
+ * When a page is moved from swapcache to shmem filecache (by the usual
+ * swapin of shmem_get_folio_gfp()), it may have been read in earlier from
+ * swap, in ignorance of the mapping it belongs to. If that mapping has
+ * special constraints (like the gma500 GEM driver, which requires RAM
+ * below 4GB), we may need to copy to a suitable page before moving to
+ * filecache.
*
* In a future release, this may well be extended to respect cpuset and
* NUMA mempolicy, and applied also to anonymous pages in do_swap_page();
@@ -3106,7 +2931,6 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,
info = SHMEM_I(inode);
memset(info, 0, (char *)inode - (char *)info);
spin_lock_init(&info->lock);
- atomic_set(&info->stop_eviction, 0);
info->seals = F_SEAL_SEAL;
info->flags = (flags & VM_NORESERVE) ? SHMEM_F_NORESERVE : 0;
info->i_crtime = inode_get_mtime(inode);
@@ -3115,7 +2939,6 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,
if (info->fsflags)
shmem_set_inode_flags(inode, info->fsflags, NULL);
INIT_LIST_HEAD(&info->shrinklist);
- INIT_LIST_HEAD(&info->swaplist);
simple_xattrs_init(&info->xattrs);
cache_no_acl(inode);
if (sbinfo->noswap)
@@ -5785,11 +5608,6 @@ void __init shmem_init(void)
BUG_ON(IS_ERR(shm_mnt));
}
-int shmem_unuse(unsigned int type)
-{
- return 0;
-}
-
int shmem_lock(struct file *file, int lock, struct ucounts *ucounts)
{
return 0;
diff --git a/mm/swapfile.c b/mm/swapfile.c
index aeb3575df8a0b..b553652125d11 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1741,300 +1741,12 @@ unsigned int count_swap_pages(int type, int free)
}
#endif /* CONFIG_HIBERNATION */
-static inline int pte_same_as_swp(pte_t pte, pte_t swp_pte)
+static bool swap_slot_allocated(struct swap_info_struct *si,
+ unsigned long offset)
{
- return pte_same(pte_swp_clear_flags(pte), swp_pte);
-}
-
-/*
- * No need to decide whether this PTE shares the swap entry with others,
- * just let do_wp_page work it out if a write is requested later - to
- * force COW, vm_page_prot omits write permission from any private vma.
- */
-static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
- unsigned long addr, swp_entry_t entry, struct folio *folio)
-{
- struct page *page;
- struct folio *swapcache;
- spinlock_t *ptl;
- pte_t *pte, new_pte, old_pte;
- bool hwpoisoned = false;
- int ret = 1;
-
- /*
- * If the folio is removed from swap cache by others, continue to
- * unuse other PTEs. try_to_unuse may try again if we missed this one.
- */
- if (!folio_matches_swap_entry(folio, entry))
- return 0;
-
- swapcache = folio;
- folio = ksm_might_need_to_copy(folio, vma, addr);
- if (unlikely(!folio))
- return -ENOMEM;
- else if (unlikely(folio == ERR_PTR(-EHWPOISON))) {
- hwpoisoned = true;
- folio = swapcache;
- }
-
- page = folio_file_page(folio, swp_offset(entry));
- if (PageHWPoison(page))
- hwpoisoned = true;
-
- pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
- if (unlikely(!pte || !pte_same_as_swp(ptep_get(pte),
- swp_entry_to_pte(entry)))) {
- ret = 0;
- goto out;
- }
-
- old_pte = ptep_get(pte);
-
- if (unlikely(hwpoisoned || !folio_test_uptodate(folio))) {
- swp_entry_t swp_entry;
-
- dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
- if (hwpoisoned) {
- swp_entry = make_hwpoison_entry(page);
- } else {
- swp_entry = make_poisoned_swp_entry();
- }
- new_pte = swp_entry_to_pte(swp_entry);
- ret = 0;
- goto setpte;
- }
-
- /*
- * Some architectures may have to restore extra metadata to the page
- * when reading from swap. This metadata may be indexed by swap entry
- * so this must be called before swap_free().
- */
- arch_swap_restore(folio_swap(entry, folio), folio);
-
- dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
- inc_mm_counter(vma->vm_mm, MM_ANONPAGES);
- folio_get(folio);
- if (folio == swapcache) {
- rmap_t rmap_flags = RMAP_NONE;
-
- /*
- * See do_swap_page(): writeback would be problematic.
- * However, we do a folio_wait_writeback() just before this
- * call and have the folio locked.
- */
- VM_BUG_ON_FOLIO(folio_test_writeback(folio), folio);
- if (pte_swp_exclusive(old_pte))
- rmap_flags |= RMAP_EXCLUSIVE;
- /*
- * We currently only expect small !anon folios, which are either
- * fully exclusive or fully shared. If we ever get large folios
- * here, we have to be careful.
- */
- if (!folio_test_anon(folio)) {
- VM_WARN_ON_ONCE(folio_test_large(folio));
- VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
- folio_add_new_anon_rmap(folio, vma, addr, rmap_flags);
- } else {
- folio_add_anon_rmap_pte(folio, page, vma, addr, rmap_flags);
- }
- } else { /* ksm created a completely new copy */
- folio_add_new_anon_rmap(folio, vma, addr, RMAP_EXCLUSIVE);
- folio_add_lru_vma(folio, vma);
- }
- new_pte = pte_mkold(mk_pte(page, vma->vm_page_prot));
- if (pte_swp_soft_dirty(old_pte))
- new_pte = pte_mksoft_dirty(new_pte);
- if (pte_swp_uffd_wp(old_pte))
- new_pte = pte_mkuffd_wp(new_pte);
-setpte:
- set_pte_at(vma->vm_mm, addr, pte, new_pte);
- swap_free(entry);
-out:
- if (pte)
- pte_unmap_unlock(pte, ptl);
- if (folio != swapcache) {
- folio_unlock(folio);
- folio_put(folio);
- }
- return ret;
-}
-
-static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
- unsigned long addr, unsigned long end,
- unsigned int type)
-{
- pte_t *pte = NULL;
- struct swap_info_struct *si;
-
- si = swap_info[type];
- do {
- struct folio *folio;
- unsigned long offset;
- unsigned char swp_count;
- softleaf_t entry;
- swp_slot_t slot;
- int ret;
- pte_t ptent;
-
- if (!pte++) {
- pte = pte_offset_map(pmd, addr);
- if (!pte)
- break;
- }
-
- ptent = ptep_get_lockless(pte);
- entry = softleaf_from_pte(ptent);
-
- if (!softleaf_is_swap(entry))
- continue;
-
- slot = swp_entry_to_swp_slot(entry);
- if (swp_slot_type(slot) != type)
- continue;
-
- offset = swp_slot_offset(slot);
- pte_unmap(pte);
- pte = NULL;
-
- folio = swap_cache_get_folio(entry);
- if (!folio) {
- struct vm_fault vmf = {
- .vma = vma,
- .address = addr,
- .real_address = addr,
- .pmd = pmd,
- };
-
- folio = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
- &vmf);
- }
- if (!folio) {
- swp_count = READ_ONCE(si->swap_map[offset]);
- if (swp_count == 0 || swp_count == SWAP_MAP_BAD)
- continue;
- return -ENOMEM;
- }
-
- folio_lock(folio);
- folio_wait_writeback(folio);
- ret = unuse_pte(vma, pmd, addr, entry, folio);
- if (ret < 0) {
- folio_unlock(folio);
- folio_put(folio);
- return ret;
- }
-
- folio_free_swap(folio);
- folio_unlock(folio);
- folio_put(folio);
- } while (addr += PAGE_SIZE, addr != end);
-
- if (pte)
- pte_unmap(pte);
- return 0;
-}
-
-static inline int unuse_pmd_range(struct vm_area_struct *vma, pud_t *pud,
- unsigned long addr, unsigned long end,
- unsigned int type)
-{
- pmd_t *pmd;
- unsigned long next;
- int ret;
-
- pmd = pmd_offset(pud, addr);
- do {
- cond_resched();
- next = pmd_addr_end(addr, end);
- ret = unuse_pte_range(vma, pmd, addr, next, type);
- if (ret)
- return ret;
- } while (pmd++, addr = next, addr != end);
- return 0;
-}
-
-static inline int unuse_pud_range(struct vm_area_struct *vma, p4d_t *p4d,
- unsigned long addr, unsigned long end,
- unsigned int type)
-{
- pud_t *pud;
- unsigned long next;
- int ret;
-
- pud = pud_offset(p4d, addr);
- do {
- next = pud_addr_end(addr, end);
- if (pud_none_or_clear_bad(pud))
- continue;
- ret = unuse_pmd_range(vma, pud, addr, next, type);
- if (ret)
- return ret;
- } while (pud++, addr = next, addr != end);
- return 0;
-}
-
-static inline int unuse_p4d_range(struct vm_area_struct *vma, pgd_t *pgd,
- unsigned long addr, unsigned long end,
- unsigned int type)
-{
- p4d_t *p4d;
- unsigned long next;
- int ret;
-
- p4d = p4d_offset(pgd, addr);
- do {
- next = p4d_addr_end(addr, end);
- if (p4d_none_or_clear_bad(p4d))
- continue;
- ret = unuse_pud_range(vma, p4d, addr, next, type);
- if (ret)
- return ret;
- } while (p4d++, addr = next, addr != end);
- return 0;
-}
-
-static int unuse_vma(struct vm_area_struct *vma, unsigned int type)
-{
- pgd_t *pgd;
- unsigned long addr, end, next;
- int ret;
-
- addr = vma->vm_start;
- end = vma->vm_end;
-
- pgd = pgd_offset(vma->vm_mm, addr);
- do {
- next = pgd_addr_end(addr, end);
- if (pgd_none_or_clear_bad(pgd))
- continue;
- ret = unuse_p4d_range(vma, pgd, addr, next, type);
- if (ret)
- return ret;
- } while (pgd++, addr = next, addr != end);
- return 0;
-}
+ unsigned char count = READ_ONCE(si->swap_map[offset]);
-static int unuse_mm(struct mm_struct *mm, unsigned int type)
-{
- struct vm_area_struct *vma;
- int ret = 0;
- VMA_ITERATOR(vmi, mm, 0);
-
- mmap_read_lock(mm);
- if (check_stable_address_space(mm))
- goto unlock;
- for_each_vma(vmi, vma) {
- if (vma->anon_vma && !is_vm_hugetlb_page(vma)) {
- ret = unuse_vma(vma, type);
- if (ret)
- break;
- }
-
- cond_resched();
- }
-unlock:
- mmap_read_unlock(mm);
- return ret;
+ return count && swap_count(count) != SWAP_MAP_BAD;
}
/*
@@ -2046,7 +1758,6 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
unsigned int prev)
{
unsigned int i;
- unsigned char count;
/*
* No need for swap_lock here: we're just looking
@@ -2055,8 +1766,7 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
* allocations from this area (while holding swap_lock).
*/
for (i = prev + 1; i < si->max; i++) {
- count = READ_ONCE(si->swap_map[i]);
- if (count && swap_count(count) != SWAP_MAP_BAD)
+ if (swap_slot_allocated(si, i))
break;
if ((i % LATENCY_LIMIT) == 0)
cond_resched();
@@ -2068,101 +1778,139 @@ static unsigned int find_next_to_unuse(struct swap_info_struct *si,
return i;
}
+#define for_each_allocated_offset(si, offset) \
+ while (swap_usage_in_pages(si) && \
+ !signal_pending(current) && \
+ (offset = find_next_to_unuse(si, offset)) != 0)
+
+static struct folio *pagein(swp_entry_t entry, struct swap_iocb **splug,
+ struct mempolicy *mpol)
+{
+ bool folio_was_allocated;
+ struct folio *folio = __read_swap_cache_async(entry, GFP_KERNEL, mpol,
+ NO_INTERLEAVE_INDEX, &folio_was_allocated, false);
+
+ if (folio_was_allocated)
+ swap_read_folio(folio, splug);
+ return folio;
+}
+
static int try_to_unuse(unsigned int type)
{
- struct mm_struct *prev_mm;
- struct mm_struct *mm;
- struct list_head *p;
- int retval = 0;
struct swap_info_struct *si = swap_info[type];
+ struct swap_iocb *splug = NULL;
+ struct mempolicy *mpol;
+ struct blk_plug plug;
+ unsigned long offset;
struct folio *folio;
swp_entry_t entry;
swp_slot_t slot;
- unsigned int i;
+ int ret = 0;
if (!swap_usage_in_pages(si))
goto success;
-retry:
- retval = shmem_unuse(type);
- if (retval)
- return retval;
-
- prev_mm = &init_mm;
- mmget(prev_mm);
-
- spin_lock(&mmlist_lock);
- p = &init_mm.mmlist;
- while (swap_usage_in_pages(si) &&
- !signal_pending(current) &&
- (p = p->next) != &init_mm.mmlist) {
+ mpol = get_task_policy(current);
+ blk_start_plug(&plug);
- mm = list_entry(p, struct mm_struct, mmlist);
- if (!mmget_not_zero(mm))
+ /* first round - submit the reads */
+ offset = 0;
+ for_each_allocated_offset(si, offset) {
+ slot = swp_slot(type, offset);
+ entry = swp_slot_to_swp_entry(slot);
+ if (!entry.val)
continue;
- spin_unlock(&mmlist_lock);
- mmput(prev_mm);
- prev_mm = mm;
- retval = unuse_mm(mm, type);
- if (retval) {
- mmput(prev_mm);
- return retval;
- }
- /*
- * Make sure that we aren't completely killing
- * interactive performance.
- */
- cond_resched();
- spin_lock(&mmlist_lock);
+ folio = pagein(entry, &splug, mpol);
+ if (folio)
+ folio_put(folio);
}
- spin_unlock(&mmlist_lock);
+ blk_finish_plug(&plug);
+ swap_read_unplug(splug);
+ splug = NULL;
+ lru_add_drain();
+
+ /* second round - updating the virtual swap slots' backing state */
+ offset = 0;
+ for_each_allocated_offset(si, offset) {
+ slot = swp_slot(type, offset);
+retry:
+ entry = swp_slot_to_swp_entry(slot);
+ if (!entry.val) {
+ if (!swap_slot_allocated(si, offset))
+ continue;
- mmput(prev_mm);
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto out;
+ }
- i = 0;
- while (swap_usage_in_pages(si) &&
- !signal_pending(current) &&
- (i = find_next_to_unuse(si, i)) != 0) {
+ /* we might be racing with zswap writeback or disk swapout */
+ schedule_timeout_uninterruptible(1);
+ goto retry;
+ }
- slot = swp_slot(type, i);
- entry = swp_slot_to_swp_entry(slot);
- folio = swap_cache_get_folio(entry);
- if (!folio)
- continue;
+ /* try to allocate swap cache folio */
+ folio = pagein(entry, &splug, mpol);
+ if (!folio) {
+ if (!swp_slot_to_swp_entry(swp_slot(type, offset)).val)
+ continue;
+ ret = -ENOMEM;
+ pr_err("swapoff: unable to allocate swap cache folio for %lu\n",
+ entry.val);
+ goto out;
+ }
+
+ folio_lock(folio);
/*
- * It is conceivable that a racing task removed this folio from
- * swap cache just before we acquired the page lock. The folio
- * might even be back in swap cache on another swap area. But
- * that is okay, folio_free_swap() only removes stale folios.
+ * We need to check if the folio is still in swap cache, and is still
+ * backed by the physical swap slot we are trying to release.
+ *
+ * We can, for instance, race with zswap writeback, obtaining the
+ * temporary folio it allocated for decompression and writeback, which
+ * would be promptly deleted from swap cache. By the time we lock that
+ * folio, it might have already contained stale data.
+ *
+ * Concurrent swap operations might have also come in before we
+ * reobtain the folio's lock, deleting the folio from swap cache,
+ * invalidating the virtual swap slot, then swapping out the folio
+ * again to a different swap backends.
+ *
+ * In all of these cases, we must retry the physical -> virtual lookup.
*/
- folio_lock(folio);
+ if (!folio_matches_swap_slot(folio, entry, slot)) {
+ folio_unlock(folio);
+ folio_put(folio);
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto out;
+ }
+ schedule_timeout_uninterruptible(1);
+ goto retry;
+ }
+
folio_wait_writeback(folio);
- folio_free_swap(folio);
+ vswap_store_folio(entry, folio);
+ folio_mark_dirty(folio);
folio_unlock(folio);
folio_put(folio);
}
- /*
- * Lets check again to see if there are still swap entries in the map.
- * If yes, we would need to do retry the unuse logic again.
- * Under global memory pressure, swap entries can be reinserted back
- * into process space after the mmlist loop above passes over them.
- *
- * Limit the number of retries? No: when mmget_not_zero()
- * above fails, that mm is likely to be freeing swap from
- * exit_mmap(), which proceeds at its own independent pace;
- * and even shmem_writeout() could have been preempted after
- * folio_alloc_swap(), temporarily hiding that swap. It's easy
- * and robust (though cpu-intensive) just to keep retrying.
- */
- if (swap_usage_in_pages(si)) {
- if (!signal_pending(current))
- goto retry;
- return -EINTR;
+ /* concurrent swappers might still be releasing physical swap slots... */
+ while (swap_usage_in_pages(si)) {
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto out;
+ }
+ schedule_timeout_uninterruptible(1);
}
+out:
+ swap_read_unplug(splug);
+ if (ret)
+ return ret;
+
success:
/*
* Make sure that further cleanups after try_to_unuse() returns happen
--
2.52.0
^ permalink raw reply related
* [PATCH v5 20/21] swapfile: replace the swap map with bitmaps
From: Nhat Pham @ 2026-03-20 19:27 UTC (permalink / raw)
To: kasong
Cc: Liam.Howlett, akpm, apopple, axelrasmussen, baohua, baolin.wang,
bhe, byungchul, cgroups, chengming.zhou, chrisl, corbet, david,
dev.jain, gourry, hannes, hughd, jannh, joshua.hahnjy, lance.yang,
lenb, linux-doc, linux-kernel, linux-mm, linux-pm,
lorenzo.stoakes, matthew.brost, mhocko, muchun.song, npache,
nphamcs, pavel, peterx, peterz, pfalcato, rafael, rakie.kim,
roman.gushchin, rppt, ryan.roberts, shakeel.butt, shikemeng,
surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed, yuanchu,
zhengqi.arch, ziy, kernel-team, riel
In-Reply-To: <20260320192735.748051-1-nphamcs@gmail.com>
Now that we have moved the swap count state to virtual swap layer, each
swap map entry only has 3 possible states: free, allocated, and bad.
Replace the swap map with 2 bitmaps (one for allocated state and one for
bad state), saving 6 bits per swap entry.
Signed-off-by: Nhat Pham <nphamcs@gmail.com>
---
include/linux/swap.h | 3 +-
mm/swapfile.c | 81 +++++++++++++++++++++++---------------------
2 files changed, 44 insertions(+), 40 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 21e528d8d3480..3c789149996c5 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -259,7 +259,8 @@ struct swap_info_struct {
struct plist_node list; /* entry in swap_active_head */
signed char type; /* strange name for an index */
unsigned int max; /* extent of the swap_map */
- unsigned char *swap_map; /* vmalloc'ed array of usage counts */
+ unsigned long *swap_map; /* bitmap for allocated state */
+ unsigned long *bad_map; /* bitmap for bad state */
struct swap_cluster_info *cluster_info; /* cluster info. Only for SSD */
struct list_head free_clusters; /* free clusters list */
struct list_head full_clusters; /* full clusters list */
diff --git a/mm/swapfile.c b/mm/swapfile.c
index b553652125d11..3e2bfcf1aa789 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -760,25 +760,19 @@ static bool cluster_reclaim_range(struct swap_info_struct *si,
struct swap_cluster_info *ci,
unsigned long start, unsigned long end)
{
- unsigned char *map = si->swap_map;
unsigned long offset = start;
int nr_reclaim;
spin_unlock(&ci->lock);
do {
- switch (READ_ONCE(map[offset])) {
- case 0:
+ if (!test_bit(offset, si->swap_map)) {
offset++;
- break;
- case SWAP_MAP_ALLOCATED:
+ } else {
nr_reclaim = __try_to_reclaim_swap(si, offset, TTRS_ANYWAY);
if (nr_reclaim > 0)
offset += nr_reclaim;
else
goto out;
- break;
- default:
- goto out;
}
} while (offset < end);
out:
@@ -787,11 +781,7 @@ static bool cluster_reclaim_range(struct swap_info_struct *si,
* Recheck the range no matter reclaim succeeded or not, the slot
* could have been be freed while we are not holding the lock.
*/
- for (offset = start; offset < end; offset++)
- if (READ_ONCE(map[offset]))
- return false;
-
- return true;
+ return find_next_bit(si->swap_map, end, start) >= end;
}
static bool cluster_scan_range(struct swap_info_struct *si,
@@ -800,15 +790,16 @@ static bool cluster_scan_range(struct swap_info_struct *si,
bool *need_reclaim)
{
unsigned long offset, end = start + nr_pages;
- unsigned char *map = si->swap_map;
- unsigned char count;
if (cluster_is_empty(ci))
return true;
for (offset = start; offset < end; offset++) {
- count = READ_ONCE(map[offset]);
- if (!count)
+ /* Bad slots cannot be used for allocation */
+ if (test_bit(offset, si->bad_map))
+ return false;
+
+ if (!test_bit(offset, si->swap_map))
continue;
if (swap_cache_only(si, offset)) {
@@ -841,7 +832,7 @@ static bool cluster_alloc_range(struct swap_info_struct *si, struct swap_cluster
if (cluster_is_empty(ci))
ci->order = order;
- memset(si->swap_map + start, usage, nr_pages);
+ bitmap_set(si->swap_map, start, nr_pages);
swap_range_alloc(si, nr_pages);
ci->count += nr_pages;
@@ -1407,7 +1398,7 @@ static struct swap_info_struct *_swap_info_get(swp_slot_t slot)
offset = swp_slot_offset(slot);
if (offset >= si->max)
goto bad_offset;
- if (data_race(!si->swap_map[swp_slot_offset(slot)]))
+ if (data_race(!test_bit(offset, si->swap_map)))
goto bad_free;
return si;
@@ -1521,8 +1512,7 @@ static void swap_slots_free(struct swap_info_struct *si,
swp_slot_t slot, unsigned int nr_pages)
{
unsigned long offset = swp_slot_offset(slot);
- unsigned char *map = si->swap_map + offset;
- unsigned char *map_end = map + nr_pages;
+ unsigned long end = offset + nr_pages;
/* It should never free entries across different clusters */
VM_BUG_ON(ci != __swap_offset_to_cluster(si, offset + nr_pages - 1));
@@ -1530,10 +1520,8 @@ static void swap_slots_free(struct swap_info_struct *si,
VM_BUG_ON(ci->count < nr_pages);
ci->count -= nr_pages;
- do {
- VM_BUG_ON(!swap_is_last_ref(*map));
- *map = 0;
- } while (++map < map_end);
+ VM_BUG_ON(find_next_zero_bit(si->swap_map, end, offset) < end);
+ bitmap_clear(si->swap_map, offset, nr_pages);
swap_range_free(si, offset, nr_pages);
@@ -1744,9 +1732,7 @@ unsigned int count_swap_pages(int type, int free)
static bool swap_slot_allocated(struct swap_info_struct *si,
unsigned long offset)
{
- unsigned char count = READ_ONCE(si->swap_map[offset]);
-
- return count && swap_count(count) != SWAP_MAP_BAD;
+ return test_bit(offset, si->swap_map);
}
/*
@@ -2067,7 +2053,7 @@ static int setup_swap_extents(struct swap_info_struct *sis, sector_t *span)
}
static void setup_swap_info(struct swap_info_struct *si, int prio,
- unsigned char *swap_map,
+ unsigned long *swap_map,
struct swap_cluster_info *cluster_info)
{
si->prio = prio;
@@ -2095,7 +2081,7 @@ static void _enable_swap_info(struct swap_info_struct *si)
}
static void enable_swap_info(struct swap_info_struct *si, int prio,
- unsigned char *swap_map,
+ unsigned long *swap_map,
struct swap_cluster_info *cluster_info)
{
spin_lock(&swap_lock);
@@ -2188,7 +2174,8 @@ static void flush_percpu_swap_cluster(struct swap_info_struct *si)
SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
{
struct swap_info_struct *p = NULL;
- unsigned char *swap_map;
+ unsigned long *swap_map;
+ unsigned long *bad_map;
struct swap_cluster_info *cluster_info;
struct file *swap_file, *victim;
struct address_space *mapping;
@@ -2283,6 +2270,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
p->swap_file = NULL;
swap_map = p->swap_map;
p->swap_map = NULL;
+ bad_map = p->bad_map;
+ p->bad_map = NULL;
maxpages = p->max;
cluster_info = p->cluster_info;
p->max = 0;
@@ -2293,7 +2282,8 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)
mutex_unlock(&swapon_mutex);
kfree(p->global_cluster);
p->global_cluster = NULL;
- vfree(swap_map);
+ kvfree(swap_map);
+ kvfree(bad_map);
free_cluster_info(cluster_info, maxpages);
inode = mapping->host;
@@ -2641,18 +2631,20 @@ static unsigned long read_swap_header(struct swap_info_struct *si,
static int setup_swap_map(struct swap_info_struct *si,
union swap_header *swap_header,
- unsigned char *swap_map,
+ unsigned long *swap_map,
+ unsigned long *bad_map,
unsigned long maxpages)
{
unsigned long i;
- swap_map[0] = SWAP_MAP_BAD; /* omit header page */
+ set_bit(0, bad_map); /* omit header page */
+
for (i = 0; i < swap_header->info.nr_badpages; i++) {
unsigned int page_nr = swap_header->info.badpages[i];
if (page_nr == 0 || page_nr > swap_header->info.last_page)
return -EINVAL;
if (page_nr < maxpages) {
- swap_map[page_nr] = SWAP_MAP_BAD;
+ set_bit(page_nr, bad_map);
si->pages--;
}
}
@@ -2756,7 +2748,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
int nr_extents;
sector_t span;
unsigned long maxpages;
- unsigned char *swap_map = NULL;
+ unsigned long *swap_map = NULL, *bad_map = NULL;
struct swap_cluster_info *cluster_info = NULL;
struct folio *folio = NULL;
struct inode *inode = NULL;
@@ -2852,16 +2844,24 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
maxpages = si->max;
/* OK, set up the swap map and apply the bad block list */
- swap_map = vzalloc(maxpages);
+ swap_map = kvcalloc(BITS_TO_LONGS(maxpages), sizeof(long), GFP_KERNEL);
if (!swap_map) {
error = -ENOMEM;
goto bad_swap_unlock_inode;
}
- error = setup_swap_map(si, swap_header, swap_map, maxpages);
+ bad_map = kvcalloc(BITS_TO_LONGS(maxpages), sizeof(long), GFP_KERNEL);
+ if (!bad_map) {
+ error = -ENOMEM;
+ goto bad_swap_unlock_inode;
+ }
+
+ error = setup_swap_map(si, swap_header, swap_map, bad_map, maxpages);
if (error)
goto bad_swap_unlock_inode;
+ si->bad_map = bad_map;
+
if (si->bdev && bdev_stable_writes(si->bdev))
si->flags |= SWP_STABLE_WRITES;
@@ -2955,7 +2955,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
si->swap_file = NULL;
si->flags = 0;
spin_unlock(&swap_lock);
- vfree(swap_map);
+ if (swap_map)
+ kvfree(swap_map);
+ if (bad_map)
+ kvfree(bad_map);
if (cluster_info)
free_cluster_info(cluster_info, maxpages);
if (inced_nr_rotate_swap)
--
2.52.0
^ permalink raw reply related
* [PATCH v5 21/21] vswap: batch contiguous vswap free calls
From: Nhat Pham @ 2026-03-20 19:27 UTC (permalink / raw)
To: kasong
Cc: Liam.Howlett, akpm, apopple, axelrasmussen, baohua, baolin.wang,
bhe, byungchul, cgroups, chengming.zhou, chrisl, corbet, david,
dev.jain, gourry, hannes, hughd, jannh, joshua.hahnjy, lance.yang,
lenb, linux-doc, linux-kernel, linux-mm, linux-pm,
lorenzo.stoakes, matthew.brost, mhocko, muchun.song, npache,
nphamcs, pavel, peterx, peterz, pfalcato, rafael, rakie.kim,
roman.gushchin, rppt, ryan.roberts, shakeel.butt, shikemeng,
surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed, yuanchu,
zhengqi.arch, ziy, kernel-team, riel
In-Reply-To: <20260320192735.748051-1-nphamcs@gmail.com>
In vswap_free(), we release and reacquire the cluster lock for every
single entry, even for non-disk-swap backends where the lock drop is
unnecessary. Batch consecutive free operations to avoid this overhead.
Signed-off-by: Nhat Pham <nphamcs@gmail.com>
---
include/linux/memcontrol.h | 6 ++
mm/memcontrol.c | 2 +-
mm/vswap.c | 185 ++++++++++++++++++++++++-------------
3 files changed, 126 insertions(+), 67 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 0651865a4564f..0f7f5489e1675 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -827,6 +827,7 @@ static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
return memcg->id.id;
}
struct mem_cgroup *mem_cgroup_from_id(unsigned short id);
+void mem_cgroup_id_put_many(struct mem_cgroup *memcg, unsigned int n);
#ifdef CONFIG_SHRINKER_DEBUG
static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg)
@@ -1289,6 +1290,11 @@ static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id)
return NULL;
}
+static inline void mem_cgroup_id_put_many(struct mem_cgroup *memcg,
+ unsigned int n)
+{
+}
+
#ifdef CONFIG_SHRINKER_DEBUG
static inline unsigned long mem_cgroup_ino(struct mem_cgroup *memcg)
{
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4525c21754e7f..c6d307b8127a8 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3597,7 +3597,7 @@ void __maybe_unused mem_cgroup_id_get_many(struct mem_cgroup *memcg,
refcount_add(n, &memcg->id.ref);
}
-static void mem_cgroup_id_put_many(struct mem_cgroup *memcg, unsigned int n)
+void mem_cgroup_id_put_many(struct mem_cgroup *memcg, unsigned int n)
{
if (refcount_sub_and_test(n, &memcg->id.ref)) {
mem_cgroup_id_remove(memcg);
diff --git a/mm/vswap.c b/mm/vswap.c
index fa37165cb10d0..82092502130a6 100644
--- a/mm/vswap.c
+++ b/mm/vswap.c
@@ -482,18 +482,18 @@ static void vswap_cluster_free(struct vswap_cluster *cluster)
kvfree_rcu(cluster, rcu);
}
-static inline void release_vswap_slot(struct vswap_cluster *cluster,
- unsigned long index)
+static inline void release_vswap_slot_nr(struct vswap_cluster *cluster,
+ unsigned long index, int nr)
{
unsigned long slot_index = VSWAP_IDX_WITHIN_CLUSTER_VAL(index);
lockdep_assert_held(&cluster->lock);
- cluster->count--;
+ cluster->count -= nr;
- bitmap_clear(cluster->bitmap, slot_index, 1);
+ bitmap_clear(cluster->bitmap, slot_index, nr);
/* we only free uncached empty clusters */
- if (refcount_dec_and_test(&cluster->refcnt))
+ if (refcount_sub_and_test(nr, &cluster->refcnt))
vswap_cluster_free(cluster);
else if (cluster->full && cluster_is_alloc_candidate(cluster)) {
cluster->full = false;
@@ -506,7 +506,7 @@ static inline void release_vswap_slot(struct vswap_cluster *cluster,
}
}
- atomic_dec(&vswap_used);
+ atomic_sub(nr, &vswap_used);
}
/*
@@ -528,23 +528,33 @@ void vswap_rmap_set(struct swap_cluster_info *ci, swp_slot_t slot,
}
/*
- * Caller needs to handle races with other operations themselves.
+ * release_backing - release the backend storage for a given range of virtual
+ * swap slots.
*
- * Specifically, this function is safe to be called in contexts where the swap
- * entry has been added to the swap cache and the associated folio is locked.
- * We cannot race with other accessors, and the swap entry is guaranteed to be
- * valid the whole time (since swap cache implies one refcount).
+ * Entered with the cluster locked, but might drop the lock in between.
+ * This is because several operations, such as releasing physical swap slots
+ * (i.e swap_slot_free_nr()) require the cluster to be unlocked to avoid
+ * deadlocks.
*
- * We cannot assume that the backends will be of the same type,
- * contiguous, etc. We might have a large folio coalesced from subpages with
- * mixed backend, which is only rectified when it is reclaimed.
+ * This is safe, because:
+ *
+ * 1. Callers ensure no concurrent modification of the swap entry's internal
+ * state can occur. This is guaranteed by one of the following:
+ * - For vswap_free_nr() callers: the swap entry's refcnt (swap count and
+ * swapcache pin) is down to 0.
+ * - For vswap_store_folio(), swap_zeromap_folio_set(), and zswap_entry_store()
+ * callers: the folio is locked and in the swap cache.
+ *
+ * 2. The swap entry still holds a refcnt to the cluster, keeping the cluster
+ * itself valid.
+ *
+ * We will exit the function with the cluster re-locked.
*/
-static void release_backing(swp_entry_t entry, int nr)
+static void release_backing(struct vswap_cluster *cluster, swp_entry_t entry,
+ int nr)
{
- struct vswap_cluster *cluster = NULL;
struct swp_desc *desc;
unsigned long flush_nr, phys_swap_start = 0, phys_swap_end = 0;
- unsigned long phys_swap_released = 0;
unsigned int phys_swap_type = 0;
bool need_flushing_phys_swap = false;
swp_slot_t flush_slot;
@@ -552,9 +562,8 @@ static void release_backing(swp_entry_t entry, int nr)
VM_WARN_ON(!entry.val);
- rcu_read_lock();
for (i = 0; i < nr; i++) {
- desc = vswap_iter(&cluster, entry.val + i);
+ desc = __vswap_iter(cluster, entry.val + i);
VM_WARN_ON(!desc);
/*
@@ -574,7 +583,6 @@ static void release_backing(swp_entry_t entry, int nr)
if (desc->type == VSWAP_ZSWAP && desc->zswap_entry) {
zswap_entry_free(desc->zswap_entry);
} else if (desc->type == VSWAP_SWAPFILE) {
- phys_swap_released++;
if (!phys_swap_start) {
/* start a new contiguous range of phys swap */
phys_swap_start = swp_slot_offset(desc->slot);
@@ -590,56 +598,49 @@ static void release_backing(swp_entry_t entry, int nr)
if (need_flushing_phys_swap) {
spin_unlock(&cluster->lock);
- cluster = NULL;
swap_slot_free_nr(flush_slot, flush_nr);
+ mem_cgroup_uncharge_swap(entry, flush_nr);
+ spin_lock(&cluster->lock);
need_flushing_phys_swap = false;
}
}
- if (cluster)
- spin_unlock(&cluster->lock);
- rcu_read_unlock();
/* Flush any remaining physical swap range */
if (phys_swap_start) {
flush_slot = swp_slot(phys_swap_type, phys_swap_start);
flush_nr = phys_swap_end - phys_swap_start;
+ spin_unlock(&cluster->lock);
swap_slot_free_nr(flush_slot, flush_nr);
+ mem_cgroup_uncharge_swap(entry, flush_nr);
+ spin_lock(&cluster->lock);
}
-
- if (phys_swap_released)
- mem_cgroup_uncharge_swap(entry, phys_swap_released);
}
+static void __vswap_swap_cgroup_clear(struct vswap_cluster *cluster,
+ swp_entry_t entry, unsigned int nr_ents);
+
/*
- * Entered with the cluster locked, but might unlock the cluster.
- * This is because several operations, such as releasing physical swap slots
- * (i.e swap_slot_free_nr()) require the cluster to be unlocked to avoid
- * deadlocks.
- *
- * This is safe, because:
- *
- * 1. The swap entry to be freed has refcnt (swap count and swapcache pin)
- * down to 0, so no one can change its internal state
- *
- * 2. The swap entry to be freed still holds a refcnt to the cluster, keeping
- * the cluster itself valid.
- *
- * We will exit the function with the cluster re-locked.
+ * Entered with the cluster locked. We will exit the function with the cluster
+ * still locked.
*/
-static void vswap_free(struct vswap_cluster *cluster, struct swp_desc *desc,
- swp_entry_t entry)
+static void vswap_free_nr(struct vswap_cluster *cluster, swp_entry_t entry,
+ int nr)
{
- /* Clear shadow if present */
- if (xa_is_value(desc->shadow))
- desc->shadow = NULL;
- spin_unlock(&cluster->lock);
+ struct swp_desc *desc;
+ int i;
- release_backing(entry, 1);
- mem_cgroup_clear_swap(entry, 1);
+ for (i = 0; i < nr; i++) {
+ desc = __vswap_iter(cluster, entry.val + i);
+ /* Clear shadow if present */
+ if (xa_is_value(desc->shadow))
+ desc->shadow = NULL;
+ }
- /* erase forward mapping and release the virtual slot for reallocation */
- spin_lock(&cluster->lock);
- release_vswap_slot(cluster, entry.val);
+ release_backing(cluster, entry, nr);
+ __vswap_swap_cgroup_clear(cluster, entry, nr);
+
+ /* erase forward mapping and release the virtual slots for reallocation */
+ release_vswap_slot_nr(cluster, entry.val, nr);
}
/**
@@ -818,18 +819,32 @@ static bool vswap_free_nr_any_cache_only(swp_entry_t entry, int nr)
struct vswap_cluster *cluster = NULL;
struct swp_desc *desc;
bool ret = false;
- int i;
+ swp_entry_t free_start;
+ int i, free_nr = 0;
+ free_start.val = 0;
rcu_read_lock();
for (i = 0; i < nr; i++) {
+ /* flush pending free batch at cluster boundary */
+ if (free_nr && !VSWAP_IDX_WITHIN_CLUSTER_VAL(entry.val)) {
+ vswap_free_nr(cluster, free_start, free_nr);
+ free_nr = 0;
+ }
desc = vswap_iter(&cluster, entry.val);
VM_WARN_ON(!desc);
ret |= (desc->swap_count == 1 && desc->in_swapcache);
desc->swap_count--;
- if (!desc->swap_count && !desc->in_swapcache)
- vswap_free(cluster, desc, entry);
+ if (!desc->swap_count && !desc->in_swapcache) {
+ if (!free_nr++)
+ free_start = entry;
+ } else if (free_nr) {
+ vswap_free_nr(cluster, free_start, free_nr);
+ free_nr = 0;
+ }
entry.val++;
}
+ if (free_nr)
+ vswap_free_nr(cluster, free_start, free_nr);
if (cluster)
spin_unlock(&cluster->lock);
rcu_read_unlock();
@@ -952,19 +967,33 @@ void swapcache_clear(swp_entry_t entry, int nr)
{
struct vswap_cluster *cluster = NULL;
struct swp_desc *desc;
- int i;
+ swp_entry_t free_start;
+ int i, free_nr = 0;
if (!nr)
return;
+ free_start.val = 0;
rcu_read_lock();
for (i = 0; i < nr; i++) {
+ /* flush pending free batch at cluster boundary */
+ if (free_nr && !VSWAP_IDX_WITHIN_CLUSTER_VAL(entry.val)) {
+ vswap_free_nr(cluster, free_start, free_nr);
+ free_nr = 0;
+ }
desc = vswap_iter(&cluster, entry.val);
desc->in_swapcache = false;
- if (!desc->swap_count)
- vswap_free(cluster, desc, entry);
+ if (!desc->swap_count) {
+ if (!free_nr++)
+ free_start = entry;
+ } else if (free_nr) {
+ vswap_free_nr(cluster, free_start, free_nr);
+ free_nr = 0;
+ }
entry.val++;
}
+ if (free_nr)
+ vswap_free_nr(cluster, free_start, free_nr);
if (cluster)
spin_unlock(&cluster->lock);
rcu_read_unlock();
@@ -1105,11 +1134,13 @@ void vswap_store_folio(swp_entry_t entry, struct folio *folio)
VM_BUG_ON(!folio_test_locked(folio));
VM_BUG_ON(folio->swap.val != entry.val);
- release_backing(entry, nr);
-
rcu_read_lock();
+ desc = vswap_iter(&cluster, entry.val);
+ VM_WARN_ON(!desc);
+ release_backing(cluster, entry, nr);
+
for (i = 0; i < nr; i++) {
- desc = vswap_iter(&cluster, entry.val + i);
+ desc = __vswap_iter(cluster, entry.val + i);
VM_WARN_ON(!desc);
desc->type = VSWAP_FOLIO;
desc->swap_cache = folio;
@@ -1134,11 +1165,13 @@ void swap_zeromap_folio_set(struct folio *folio)
VM_BUG_ON(!folio_test_locked(folio));
VM_BUG_ON(!entry.val);
- release_backing(entry, nr);
-
rcu_read_lock();
+ desc = vswap_iter(&cluster, entry.val);
+ VM_WARN_ON(!desc);
+ release_backing(cluster, entry, nr);
+
for (i = 0; i < nr; i++) {
- desc = vswap_iter(&cluster, entry.val + i);
+ desc = __vswap_iter(cluster, entry.val + i);
VM_WARN_ON(!desc);
desc->type = VSWAP_ZERO;
}
@@ -1772,11 +1805,10 @@ void zswap_entry_store(swp_entry_t swpentry, struct zswap_entry *entry)
struct vswap_cluster *cluster = NULL;
struct swp_desc *desc;
- release_backing(swpentry, 1);
-
rcu_read_lock();
desc = vswap_iter(&cluster, swpentry.val);
VM_WARN_ON(!desc);
+ release_backing(cluster, swpentry, 1);
desc->zswap_entry = entry;
desc->type = VSWAP_ZSWAP;
spin_unlock(&cluster->lock);
@@ -1850,6 +1882,22 @@ static unsigned short __vswap_cgroup_record(struct vswap_cluster *cluster,
return oldid;
}
+/*
+ * Clear swap cgroup for a range of swap entries.
+ * Entered with the cluster locked. Caller must be under rcu_read_lock().
+ */
+static void __vswap_swap_cgroup_clear(struct vswap_cluster *cluster,
+ swp_entry_t entry, unsigned int nr_ents)
+{
+ unsigned short id;
+ struct mem_cgroup *memcg;
+
+ id = __vswap_cgroup_record(cluster, entry, 0, nr_ents);
+ memcg = mem_cgroup_from_id(id);
+ if (memcg)
+ mem_cgroup_id_put_many(memcg, nr_ents);
+}
+
static unsigned short vswap_cgroup_record(swp_entry_t entry,
unsigned short memcgid, unsigned int nr_ents)
{
@@ -1955,6 +2003,11 @@ unsigned short lookup_swap_cgroup_id(swp_entry_t entry)
rcu_read_unlock();
return ret;
}
+#else /* !CONFIG_MEMCG */
+static void __vswap_swap_cgroup_clear(struct vswap_cluster *cluster,
+ swp_entry_t entry, unsigned int nr_ents)
+{
+}
#endif /* CONFIG_MEMCG */
int vswap_init(void)
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v14 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Joel Fernandes @ 2026-03-20 19:50 UTC (permalink / raw)
To: Gary Guo, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellström,
Helge Deller, Alex Gaynor, Boqun Feng, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Alexandre Courbot,
Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, alexeyi, Eliot Courtney, joel,
linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH7MPTIK5OMK.3GHQAE07J5OO@garyguo.net>
On 3/20/2026 9:04 AM, Gary Guo wrote:
> On Fri Mar 20, 2026 at 4:57 AM GMT, Joel Fernandes wrote:
>> Add safe Rust abstractions over the Linux kernel's GPU buddy
>> allocator for physical memory management. The GPU buddy allocator
>> implements a binary buddy system useful for GPU physical memory
>> allocation. nova-core will use it for physical memory allocation.
>>
>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>> MAINTAINERS | 6 +
>> rust/bindings/bindings_helper.h | 11 +
>> rust/helpers/gpu.c | 23 ++
>> rust/helpers/helpers.c | 1 +
>> rust/kernel/gpu.rs | 6 +
>> rust/kernel/gpu/buddy.rs | 613 ++++++++++++++++++++++++++++++++
>> rust/kernel/lib.rs | 2 +
>> 7 files changed, 662 insertions(+)
>> create mode 100644 rust/helpers/gpu.c
>> create mode 100644 rust/kernel/gpu.rs
>> create mode 100644 rust/kernel/gpu/buddy.rs
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e847099efcc2..cd9505d3be60 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8531,7 +8531,10 @@ T: git https://gitlab.freedesktop.org/drm/rust/kernel.git
>> F: drivers/gpu/drm/nova/
>> F: drivers/gpu/drm/tyr/
>> F: drivers/gpu/nova-core/
>> +F: rust/helpers/gpu.c
>> F: rust/kernel/drm/
>> +F: rust/kernel/gpu.rs
>> +F: rust/kernel/gpu/
>>
>> DRM DRIVERS FOR ALLWINNER A10
>> M: Chen-Yu Tsai <wens@kernel.org>
>> @@ -8952,6 +8955,9 @@ F: drivers/gpu/drm/drm_buddy.c
>> F: drivers/gpu/tests/gpu_buddy_test.c
>> F: include/drm/drm_buddy.h
>> F: include/linux/gpu_buddy.h
>> +F: rust/helpers/gpu.c
>> +F: rust/kernel/gpu.rs
>> +F: rust/kernel/gpu/
>>
>> DRM AUTOMATED TESTING
>> M: Helen Koike <helen.fornazier@gmail.com>
>> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>> index 083cc44aa952..dbb765a9fdbd 100644
>> --- a/rust/bindings/bindings_helper.h
>> +++ b/rust/bindings/bindings_helper.h
>> @@ -29,6 +29,7 @@
>> #include <linux/hrtimer_types.h>
>>
>> #include <linux/acpi.h>
>> +#include <linux/gpu_buddy.h>
>> #include <drm/drm_device.h>
>> #include <drm/drm_drv.h>
>> #include <drm/drm_file.h>
>> @@ -146,6 +147,16 @@ const vm_flags_t RUST_CONST_HELPER_VM_MIXEDMAP = VM_MIXEDMAP;
>> const vm_flags_t RUST_CONST_HELPER_VM_HUGEPAGE = VM_HUGEPAGE;
>> const vm_flags_t RUST_CONST_HELPER_VM_NOHUGEPAGE = VM_NOHUGEPAGE;
>>
>> +#if IS_ENABLED(CONFIG_GPU_BUDDY)
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_RANGE_ALLOCATION = GPU_BUDDY_RANGE_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TOPDOWN_ALLOCATION = GPU_BUDDY_TOPDOWN_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CONTIGUOUS_ALLOCATION =
>> + GPU_BUDDY_CONTIGUOUS_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEAR_ALLOCATION = GPU_BUDDY_CLEAR_ALLOCATION;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_CLEARED = GPU_BUDDY_CLEARED;
>> +const unsigned long RUST_CONST_HELPER_GPU_BUDDY_TRIM_DISABLE = GPU_BUDDY_TRIM_DISABLE;
>> +#endif
>> +
>> #if IS_ENABLED(CONFIG_ANDROID_BINDER_IPC_RUST)
>> #include "../../drivers/android/binder/rust_binder.h"
>> #include "../../drivers/android/binder/rust_binder_events.h"
>> diff --git a/rust/helpers/gpu.c b/rust/helpers/gpu.c
>> new file mode 100644
>> index 000000000000..38b1a4e6bef8
>> --- /dev/null
>> +++ b/rust/helpers/gpu.c
>> @@ -0,0 +1,23 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/gpu_buddy.h>
>> +
>> +#ifdef CONFIG_GPU_BUDDY
>> +
>> +__rust_helper u64 rust_helper_gpu_buddy_block_offset(const struct gpu_buddy_block *block)
>> +{
>> + return gpu_buddy_block_offset(block);
>> +}
>> +
>> +__rust_helper unsigned int rust_helper_gpu_buddy_block_order(struct gpu_buddy_block *block)
>> +{
>> + return gpu_buddy_block_order(block);
>> +}
>> +
>> +__rust_helper u64 rust_helper_gpu_buddy_block_size(struct gpu_buddy *mm,
>> + struct gpu_buddy_block *block)
>> +{
>> + return gpu_buddy_block_size(mm, block);
>> +}
>
> From Sashiko:
> https://sashiko.dev/#/patchset/20260320045711.43494-1-joelagnelf%40nvidia.com
>
> Does the Rust wrapper use this helper? It looks like AllocatedBlock::size()
> manually duplicates the bitwise logic (chunk_size << order) rather than
> calling this helper, which could create a divergence risk if the underlying C
> allocator implementation changes.
>
> Many other review comments there seem to be false positive, but it might worth
> confirming.
If Danilo is applying the patch, can we please remove the helper on apply? I
think in this case we do not need to call the helper. I don't think there is a
divergence risk here.
thanks,
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH v2 08/10] gpu: nova-core: convert falcon registers to kernel register macro
From: John Hubbard @ 2026-03-20 19:52 UTC (permalink / raw)
To: Joel Fernandes, Alexandre Courbot, Danilo Krummrich, Alice Ryhl,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: Alistair Popple, Timur Tabi, Zhi Wang, Eliot Courtney, dri-devel,
linux-kernel, linux-riscv, linux-doc, rust-for-linux
In-Reply-To: <8f9da1e2-fb86-4653-b702-333fc920af58@nvidia.com>
On 3/20/26 10:38 AM, Joel Fernandes wrote:
> Hi Alex,
>
> On 3/20/2026 8:19 AM, Alexandre Courbot wrote:
>> /// Reset the controller, select the falcon core, and wait for memory scrubbing to complete.
>> @@ -616,9 +462,10 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
>> self.hal.select_core(self, bar)?;
>> self.hal.reset_wait_mem_scrubbing(bar)?;
>>
>> - regs::NV_PFALCON_FALCON_RM::default()
>> - .set_value(bar.read(regs::NV_PMC_BOOT_0).into())
>> - .write(bar, &E::ID);
>> + bar.write(
>> + WithBase::of::<E>(),
>> + regs::NV_PFALCON_FALCON_RM::from(bar.read(regs::NV_PMC_BOOT_0).into_raw()),
>> + );
>>
>
> Overall, I think the series is good improvement but I still feel this part is a
> step back in readability, and we should come up with something better. I don't
> think there's any chance anyone can memorize this syntax.
I must reluctantly (because I know this conversation has gone very
long, across so many versions) agree. That .write() statement is just
brutal, and we will be relying on AI in order to even figure it out,
I fear.
I'd hoped that there were other, simpler forms to express this,
is that not the case here?
thanks,
--
John Hubbard
>
> What about using a macro to hide the boilerplate?
^ permalink raw reply
* Re: [PATCH v3 04/16] mm: add vm_ops->mapped hook
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 19:57 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Andrew Morton, Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Jann Horn, Pedro Falcato,
linux-kernel, linux-doc, linux-hyperv, linux-mtd, linux-staging,
linux-scsi, target-devel, linux-afs, linux-fsdevel, linux-mm,
Ryan Roberts
In-Reply-To: <c9068fbb-a23c-4342-8638-3b11897a57cb@kernel.org>
On Fri, Mar 20, 2026 at 07:26:37PM +0100, Vlastimil Babka (SUSE) wrote:
> On 3/19/26 19:23, Lorenzo Stoakes (Oracle) wrote:
> > Previously, when a driver needed to do something like establish a
> > reference count, it could do so in the mmap hook in the knowledge that the
> > mapping would succeed.
> >
> > With the introduction of f_op->mmap_prepare this is no longer the case, as
> > it is invoked prior to actually establishing the mapping.
> >
> > mmap_prepare is not appropriate for this kind of thing as it is called
> > before any merge might take place, and after which an error might occur
> > meaning resources could be leaked.
> >
> > To take this into account, introduce a new vm_ops->mapped callback which
> > is invoked when the VMA is first mapped (though notably - not when it is
> > merged - which is correct and mirrors existing mmap/open/close behaviour).
> >
> > We do better that vm_ops->open() here, as this callback can return an
> > error, at which point the VMA will be unmapped.
> >
> > Note that vm_ops->mapped() is invoked after any mmap action is complete
> > (such as I/O remapping).
> >
> > We intentionally do not expose the VMA at this point, exposing only the
> > fields that could be used, and an output parameter in case the operation
> > needs to update the vma->vm_private_data field.
> >
> > In order to deal with stacked filesystems which invoke inner filesystem's
> > mmap() invocations, add __compat_vma_mapped() and invoke it on vfs_mmap()
> > (via compat_vma_mmap()) to ensure that the mapped callback is handled when
> > an mmap() caller invokes a nested filesystem's mmap_prepare() callback.
> >
> > We can now also remove call_action_complete() and invoke
> > mmap_action_complete() directly, as we separate out the rmap lock logic.
> >
> > The rmap lock logic, which was added in order to keep hugetlb working (!)
> > to allow for the rmap lock to be held longer, needs to be propagated to the
> > error paths on mmap complete and mapped hook error paths.
> >
> > This is because do_munmap() might otherwise deadlock with the rmap being
> > held, so instead we unlock at the point of unmap.
>
> Hmm but that was also true prior to this series? So is this a bugfix? Should
> it be a stable hotfix done outside of the series before the refactoring?
>
> > This is fine as any reliance on the rmap being held is irrelevant on error.
> >
> > While we're here, refactor mmap_action_finish() to avoid a big if (err)
> > branch.
> >
> > We also abstract unmapping of a VMA on mmap action completion into its own
> > helper function, unmap_vma_locked().
> >
> > Update the mmap_prepare documentation to describe the mapped hook and make
> > it clear what its intended use is.
> >
> > Additionally, update VMA userland test headers to reflect the change.
> >
> > Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
> > ---
> > Documentation/filesystems/mmap_prepare.rst | 15 +++
> > include/linux/fs.h | 9 +-
> > include/linux/mm.h | 20 +++-
> > mm/internal.h | 8 ++
> > mm/util.c | 129 ++++++++++++++-------
> > mm/vma.c | 35 +++---
> > tools/testing/vma/include/dup.h | 27 ++++-
> > tools/testing/vma/include/stubs.h | 3 +-
> > 8 files changed, 186 insertions(+), 60 deletions(-)
> >
> > diff --git a/Documentation/filesystems/mmap_prepare.rst b/Documentation/filesystems/mmap_prepare.rst
> > index 65a1f094e469..20db474915da 100644
> > --- a/Documentation/filesystems/mmap_prepare.rst
> > +++ b/Documentation/filesystems/mmap_prepare.rst
> > @@ -25,6 +25,21 @@ That is - no resources should be allocated nor state updated to reflect that a
> > mapping has been established, as the mapping may either be merged, or fail to be
> > mapped after the callback is complete.
> >
> > +Mapped callback
> > +---------------
> > +
> > +If resources need to be allocated per-mapping, or state such as a reference
> > +count needs to be manipulated, this should be done using the ``vm_ops->mapped``
> > +hook, which itself should be set by the >mmap_prepare hook.
> > +
> > +This callback is only invoked if a new mapping has been established and was not
> > +merged with any other, and is invoked at a point where no error may occur before
> > +the mapping is established.
> > +
> > +You may return an error to the callback itself, which will cause the mapping to
> > +become unmapped and an error returned to the mmap() caller. This is useful if
> > +resources need to be allocated, and that allocation might fail.
> > +
> > How To Use
> > ==========
> >
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index a2628a12bd2b..c390f5c667e3 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2059,13 +2059,20 @@ static inline bool can_mmap_file(struct file *file)
> > }
> >
> > int compat_vma_mmap(struct file *file, struct vm_area_struct *vma);
> > +int __vma_check_mmap_hook(struct vm_area_struct *vma);
> >
> > static inline int vfs_mmap(struct file *file, struct vm_area_struct *vma)
> > {
> > + int err;
> > +
> > if (file->f_op->mmap_prepare)
> > return compat_vma_mmap(file, vma);
> >
> > - return file->f_op->mmap(file, vma);
> > + err = file->f_op->mmap(file, vma);
> > + if (err)
> > + return err;
> > +
> > + return __vma_check_mmap_hook(vma);
> > }
> >
> > static inline int vfs_mmap_prepare(struct file *file, struct vm_area_desc *desc)
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index da94edb287cd..68dee1101313 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -777,6 +777,23 @@ struct vm_operations_struct {
> > * Context: User context. May sleep. Caller holds mmap_lock.
> > */
> > void (*close)(struct vm_area_struct *vma);
> > + /**
> > + * @mapped: Called when the VMA is first mapped in the MM. Not called if
> > + * the new VMA is merged with an adjacent VMA.
> > + *
> > + * The @vm_private_data field is an output field allowing the user to
> > + * modify vma->vm_private_data as necessary.
> > + *
> > + * ONLY valid if set from f_op->mmap_prepare. Will result in an error if
> > + * set from f_op->mmap.
> > + *
> > + * Returns %0 on success, or an error otherwise. On error, the VMA will
> > + * be unmapped.
> > + *
> > + * Context: User context. May sleep. Caller holds mmap_lock.
> > + */
> > + int (*mapped)(unsigned long start, unsigned long end, pgoff_t pgoff,
> > + const struct file *file, void **vm_private_data);
> > /* Called any time before splitting to check if it's allowed */
> > int (*may_split)(struct vm_area_struct *vma, unsigned long addr);
> > int (*mremap)(struct vm_area_struct *vma);
> > @@ -4327,7 +4344,8 @@ static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,
> >
> > int mmap_action_prepare(struct vm_area_desc *desc);
> > int mmap_action_complete(struct vm_area_struct *vma,
> > - struct mmap_action *action);
> > + struct mmap_action *action,
> > + bool rmap_lock_held);
> >
> > /* Look up the first VMA which exactly match the interval vm_start ... vm_end */
> > static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,
> > diff --git a/mm/internal.h b/mm/internal.h
> > index 0256ca44115a..e0f554178143 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -202,6 +202,14 @@ static inline void vma_close(struct vm_area_struct *vma)
> > /* unmap_vmas is in mm/memory.c */
> > void unmap_vmas(struct mmu_gather *tlb, struct unmap_desc *unmap);
> >
> > +static inline void unmap_vma_locked(struct vm_area_struct *vma)
> > +{
> > + const size_t len = vma_pages(vma) << PAGE_SHIFT;
> > +
> > + mmap_assert_write_locked(vma->vm_mm);
> > + do_munmap(vma->vm_mm, vma->vm_start, len, NULL);
> > +}
> > +
> > #ifdef CONFIG_MMU
> >
> > static inline void get_anon_vma(struct anon_vma *anon_vma)
> > diff --git a/mm/util.c b/mm/util.c
> > index 73c97a748d8e..fc1bd8a8f3ea 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -1163,6 +1163,54 @@ void flush_dcache_folio(struct folio *folio)
> > EXPORT_SYMBOL(flush_dcache_folio);
> > #endif
> >
> > +static int __compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
> > +{
> > + struct vm_area_desc desc = {
> > + .mm = vma->vm_mm,
> > + .file = file,
> > + .start = vma->vm_start,
> > + .end = vma->vm_end,
> > +
> > + .pgoff = vma->vm_pgoff,
> > + .vm_file = vma->vm_file,
> > + .vma_flags = vma->flags,
> > + .page_prot = vma->vm_page_prot,
> > +
> > + .action.type = MMAP_NOTHING, /* Default */
> > + };
> > + int err;
> > +
> > + err = vfs_mmap_prepare(file, &desc);
> > + if (err)
> > + return err;
> > +
> > + err = mmap_action_prepare(&desc);
> > + if (err)
> > + return err;
> > +
> > + set_vma_from_desc(vma, &desc);
> > + return mmap_action_complete(vma, &desc.action, /*rmap_lock_held=*/false);
>
> Patch 1 removed this function and this one reinstates it with some
> modifications. Could patch 1 only remove the export and otherwise both do
> only the necessary modifications?
No, I'd rather do it this way because the function is now doing something
different and for different reasons.
Let's not just churn respins for no reason. If it's not breaking anything
let's just leave it as is please.
>
> > +}
> > +
> > +static int __compat_vma_mapped(struct file *file, struct vm_area_struct *vma)
> > +{
> > + const struct vm_operations_struct *vm_ops = vma->vm_ops;
> > + void *vm_private_data = vma->vm_private_data;
> > + int err;
> > +
> > + if (!vm_ops || !vm_ops->mapped)
> > + return 0;
> > +
> > + err = vm_ops->mapped(vma->vm_start, vma->vm_end, vma->vm_pgoff, file,
> > + &vm_private_data);
> > + if (err)
> > + unmap_vma_locked(vma);
> > + else if (vm_private_data != vma->vm_private_data)
> > + vma->vm_private_data = vm_private_data;
> > +
> > + return err;
> > +}
> > +
> > /**
> > * compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an
> > * existing VMA and execute any requested actions.
> > @@ -1191,34 +1239,26 @@ EXPORT_SYMBOL(flush_dcache_folio);
> > */
> > int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
> > {
> > - struct vm_area_desc desc = {
> > - .mm = vma->vm_mm,
> > - .file = file,
> > - .start = vma->vm_start,
> > - .end = vma->vm_end,
> > -
> > - .pgoff = vma->vm_pgoff,
> > - .vm_file = vma->vm_file,
> > - .vma_flags = vma->flags,
> > - .page_prot = vma->vm_page_prot,
> > -
> > - .action.type = MMAP_NOTHING, /* Default */
> > - };
> > int err;
> >
> > - err = vfs_mmap_prepare(file, &desc);
> > + err = __compat_vma_mmap(file, vma);
> > if (err)
> > return err;
> >
> > - err = mmap_action_prepare(&desc);
> > - if (err)
> > - return err;
> > -
> > - set_vma_from_desc(vma, &desc);
> > - return mmap_action_complete(vma, &desc.action);
> > + return __compat_vma_mapped(file, vma);
> > }
> > EXPORT_SYMBOL(compat_vma_mmap);
> >
> > +int __vma_check_mmap_hook(struct vm_area_struct *vma)
> > +{
> > + /* vm_ops->mapped is not valid if mmap() is specified. */
> > + if (vma->vm_ops && WARN_ON_ONCE(vma->vm_ops->mapped))
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(__vma_check_mmap_hook);
> > +
> > static void set_ps_flags(struct page_snapshot *ps, const struct folio *folio,
> > const struct page *page)
> > {
> > @@ -1308,32 +1348,31 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
> > }
> >
> > static int mmap_action_finish(struct vm_area_struct *vma,
> > - struct mmap_action *action, int err)
> > + struct mmap_action *action, int err,
> > + bool rmap_lock_held)
> > {
> > + if (rmap_lock_held)
> > + i_mmap_unlock_write(vma->vm_file->f_mapping);
>
> Should this be moved below the if (!err) ?
Yup sigh, the success hook needs this hack to be held.
This is all because hugetlb does stupid things.
The one case in which it uses generic hooks rather than stupid 'if
(hugetlb) { ... }' branches, and it does stuff it shouldn't (of course! :)
>
> Otherwise I think we unlock prematurely, and can even try to unlock twice -
> here and in call_mapped_hook(). And we want to unlock only if we're about to
> munmap, right?
No, we want to unlock at the end of the operation regardless. But this can
unlock twice because I'm not threading this crap through enough clearly.
Let me rethink this I guess. I hate hugetlb.
>
> > +
> > + if (!err) {
> > + if (action->success_hook)
> > + return action->success_hook(vma);
> > + return 0;
> > + }
> > +
> > /*
> > * If an error occurs, unmap the VMA altogether and return an error. We
> > * only clear the newly allocated VMA, since this function is only
> > * invoked if we do NOT merge, so we only clean up the VMA we created.
> > */
> > - if (err) {
> > - const size_t len = vma_pages(vma) << PAGE_SHIFT;
> > -
> > - do_munmap(current->mm, vma->vm_start, len, NULL);
> > -
> > - if (action->error_hook) {
> > - /* We may want to filter the error. */
> > - err = action->error_hook(err);
> > -
> > - /* The caller should not clear the error. */
> > - VM_WARN_ON_ONCE(!err);
> > - }
> > - return err;
> > + unmap_vma_locked(vma);
> > + if (action->error_hook) {
> > + /* We may want to filter the error. */
> > + err = action->error_hook(err);
> > + /* The caller should not clear the error. */
> > + VM_WARN_ON_ONCE(!err);
> > }
> > -
> > - if (action->success_hook)
> > - return action->success_hook(vma);
> > -
> > - return 0;
> > + return err;
> > }
> >
> > #ifdef CONFIG_MMU
> > @@ -1364,13 +1403,15 @@ EXPORT_SYMBOL(mmap_action_prepare);
> > * mmap_action_complete - Execute VMA descriptor action.
> > * @vma: The VMA to perform the action upon.
> > * @action: The action to perform.
> > + * @rmap_lock_held: Is the file rmap lock held?
> > *
> > * Similar to mmap_action_prepare().
> > *
> > * Return: 0 on success, or error, at which point the VMA will be unmapped.
> > */
> > int mmap_action_complete(struct vm_area_struct *vma,
> > - struct mmap_action *action)
> > + struct mmap_action *action,
> > + bool rmap_lock_held)
> >
> > {
> > int err = 0;
> > @@ -1388,7 +1429,8 @@ int mmap_action_complete(struct vm_area_struct *vma,
> > break;
> > }
> >
> > - return mmap_action_finish(vma, action, err);
> > + return mmap_action_finish(vma, action, err,
> > + rmap_lock_held);
> > }
> > EXPORT_SYMBOL(mmap_action_complete);
> > #else
> > @@ -1408,7 +1450,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)
> > EXPORT_SYMBOL(mmap_action_prepare);
> >
> > int mmap_action_complete(struct vm_area_struct *vma,
> > - struct mmap_action *action)
> > + struct mmap_action *action,
> > + bool rmap_lock_held)
> > {
> > int err = 0;
> >
> > @@ -1423,7 +1466,7 @@ int mmap_action_complete(struct vm_area_struct *vma,
> > break;
> > }
> >
> > - return mmap_action_finish(vma, action, err);
> > + return mmap_action_finish(vma, action, err, rmap_lock_held);
> > }
> > EXPORT_SYMBOL(mmap_action_complete);
> > #endif
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 2a86c7575000..a27d1278ea6d 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -2731,21 +2731,28 @@ static bool can_set_ksm_flags_early(struct mmap_state *map)
> > return false;
> > }
> >
> > -static int call_action_complete(struct mmap_state *map,
> > - struct mmap_action *action,
> > - struct vm_area_struct *vma)
> > +static int call_mapped_hook(struct mmap_state *map,
> > + struct vm_area_struct *vma)
> > {
> > - int ret;
> > -
> > - ret = mmap_action_complete(vma, action);
> > + const struct vm_operations_struct *vm_ops = vma->vm_ops;
> > + void *vm_private_data = vma->vm_private_data;
> > + int err;
> >
> > - /* If we held the file rmap we need to release it. */
> > - if (map->hold_file_rmap_lock) {
> > - struct file *file = vma->vm_file;
> > + if (!vm_ops || !vm_ops->mapped)
> > + return 0;
> > + err = vm_ops->mapped(vma->vm_start, vma->vm_end, vma->vm_pgoff,
> > + vma->vm_file, &vm_private_data);
> > + if (err) {
> > + if (map->hold_file_rmap_lock)
> > + i_mmap_unlock_write(vma->vm_file->f_mapping);
> >
> > - i_mmap_unlock_write(file->f_mapping);
> > + unmap_vma_locked(vma);
> > + return err;
> > }
> > - return ret;
> > + /* Update private data if changed. */
> > + if (vm_private_data != vma->vm_private_data)
> > + vma->vm_private_data = vm_private_data;
> > + return 0;
> > }
> >
> > static unsigned long __mmap_region(struct file *file, unsigned long addr,
> > @@ -2799,8 +2806,10 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
> > __mmap_complete(&map, vma);
> >
> > if (have_mmap_prepare && allocated_new) {
> > - error = call_action_complete(&map, &desc.action, vma);
> > -
> > + error = mmap_action_complete(vma, &desc.action,
> > + map.hold_file_rmap_lock);
> > + if (!error)
> > + error = call_mapped_hook(&map, vma);
>
> And if neither of those above end up doing i_mmap_unlock_write(), we should
> do it here? I think currently the misplaced unlock in mmap_action_finish()
I don't think it's misplaced, I think it's a much better place for it than
here, and otherwise we have yet more hacks upon hacks threaded throughout
the code.
We don't actually need the hugetlb hack to hold the lock over .mapped but I
guess we should do that anyway just for the sake of consistency.
But obviously call_mapped_hook() is trying to unlock anyway.
> masks the lack of it here, otherwise bots would already notice. Loss of
And AI... it would require hitting a .mapped error I guess.
> locking coverage (due to premature unlock) or the risk of double unlock is
> probably harder to trigger. Or maybe a later patch in the series happens to
> fix the issues, so it's just a bisection hazard here. Or I'm completely wrong.
Nope I messed it up, will fix.
>
> > if (error)
> > return error;
> > }
> > diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
> > index 8ae525ed1738..aa34966cbc62 100644
> > --- a/tools/testing/vma/include/dup.h
> > +++ b/tools/testing/vma/include/dup.h
> > @@ -643,6 +643,23 @@ struct vm_operations_struct {
> > * Context: User context. May sleep. Caller holds mmap_lock.
> > */
> > void (*close)(struct vm_area_struct *vma);
> > + /**
> > + * @mapped: Called when the VMA is first mapped in the MM. Not called if
> > + * the new VMA is merged with an adjacent VMA.
> > + *
> > + * The @vm_private_data field is an output field allowing the user to
> > + * modify vma->vm_private_data as necessary.
> > + *
> > + * ONLY valid if set from f_op->mmap_prepare. Will result in an error if
> > + * set from f_op->mmap.
> > + *
> > + * Returns %0 on success, or an error otherwise. On error, the VMA will
> > + * be unmapped.
> > + *
> > + * Context: User context. May sleep. Caller holds mmap_lock.
> > + */
> > + int (*mapped)(unsigned long start, unsigned long end, pgoff_t pgoff,
> > + const struct file *file, void **vm_private_data);
> > /* Called any time before splitting to check if it's allowed */
> > int (*may_split)(struct vm_area_struct *vma, unsigned long addr);
> > int (*mremap)(struct vm_area_struct *vma);
> > @@ -1281,7 +1298,7 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,
> > return err;
> >
> > set_vma_from_desc(vma, &desc);
> > - return mmap_action_complete(vma, &desc.action);
> > + return mmap_action_complete(vma, &desc.action, /*rmap_lock_held=*/false);
> > }
> >
> > static inline int compat_vma_mmap(struct file *file,
> > @@ -1500,3 +1517,11 @@ static inline pgprot_t vma_get_page_prot(vma_flags_t vma_flags)
> >
> > return vm_get_page_prot(vm_flags);
> > }
> > +
> > +static inline void unmap_vma_locked(struct vm_area_struct *vma)
> > +{
> > + const size_t len = vma_pages(vma) << PAGE_SHIFT;
> > +
> > + mmap_assert_write_locked(vma->vm_mm);
> > + do_munmap(vma->vm_mm, vma->vm_start, len, NULL);
> > +}
> > diff --git a/tools/testing/vma/include/stubs.h b/tools/testing/vma/include/stubs.h
> > index a30b8bc84955..d1c3d4ddb5e9 100644
> > --- a/tools/testing/vma/include/stubs.h
> > +++ b/tools/testing/vma/include/stubs.h
> > @@ -87,7 +87,8 @@ static inline int mmap_action_prepare(struct vm_area_desc *desc)
> > }
> >
> > static inline int mmap_action_complete(struct vm_area_struct *vma,
> > - struct mmap_action *action)
> > + struct mmap_action *action,
> > + bool rmap_lock_held)
> > {
> > return 0;
> > }
>
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH v2 08/10] gpu: nova-core: convert falcon registers to kernel register macro
From: Danilo Krummrich @ 2026-03-20 20:07 UTC (permalink / raw)
To: John Hubbard
Cc: Joel Fernandes, Alexandre Courbot, Alice Ryhl, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, dri-devel, linux-kernel, linux-riscv, linux-doc,
rust-for-linux
In-Reply-To: <ba3c5689-5e34-4097-8823-c37a48a4f358@nvidia.com>
On 3/20/26 8:52 PM, John Hubbard wrote:
> On 3/20/26 10:38 AM, Joel Fernandes wrote:
>> Hi Alex,
>>
>> On 3/20/2026 8:19 AM, Alexandre Courbot wrote:
>>> /// Reset the controller, select the falcon core, and wait for memory scrubbing to complete.
>>> @@ -616,9 +462,10 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
>>> self.hal.select_core(self, bar)?;
>>> self.hal.reset_wait_mem_scrubbing(bar)?;
>>>
>>> - regs::NV_PFALCON_FALCON_RM::default()
>>> - .set_value(bar.read(regs::NV_PMC_BOOT_0).into())
>>> - .write(bar, &E::ID);
>>> + bar.write(
>>> + WithBase::of::<E>(),
>>> + regs::NV_PFALCON_FALCON_RM::from(bar.read(regs::NV_PMC_BOOT_0).into_raw()),
>>> + );
>>>
>>
>> Overall, I think the series is good improvement but I still feel this part is a
>> step back in readability, and we should come up with something better. I don't
>> think there's any chance anyone can memorize this syntax.
>
> I must reluctantly (because I know this conversation has gone very
> long, across so many versions) agree. That .write() statement is just
> brutal, and we will be relying on AI in order to even figure it out,
> I fear.
>
> I'd hoped that there were other, simpler forms to express this,
> is that not the case here?
I mean, it is barely different from the above, except for the
WithBase::of::<E>() argument instead of &E::ID.
It becomes much more readable if you break it down (which is what we should
probably do):
let boot0 = bar.read(regs::NV_PMC_BOOT_0).into_raw();
bar.write(WithBase::of::<E>(), regs::NV_PFALCON_FALCON_RM::from(boot0));
For other cases where we read a register and write the same register we will
have a way in the future that can omit WithBase::of::<E>() entirely, see also my
reply in [1].
I want to expand write_reg() to also be able to handle variable offset
registers, e.g.:
let reg = bar.read_reg(regs::NV_PFALCON_FALCON_RM::of::<E>());
// modify reg
bar.write_reg(reg);
[1] https://lore.kernel.org/all/DH0XBLXZD81K.22SWIZ1ZAOW1@kernel.org/
^ permalink raw reply
* Re: [PATCH v2 08/10] gpu: nova-core: convert falcon registers to kernel register macro
From: Gary Guo @ 2026-03-20 20:17 UTC (permalink / raw)
To: John Hubbard, Joel Fernandes, Alexandre Courbot, Danilo Krummrich,
Alice Ryhl, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross
Cc: Alistair Popple, Timur Tabi, Zhi Wang, Eliot Courtney, dri-devel,
linux-kernel, linux-riscv, linux-doc, rust-for-linux
In-Reply-To: <ba3c5689-5e34-4097-8823-c37a48a4f358@nvidia.com>
On Fri Mar 20, 2026 at 7:52 PM GMT, John Hubbard wrote:
> On 3/20/26 10:38 AM, Joel Fernandes wrote:
>> Hi Alex,
>>
>> On 3/20/2026 8:19 AM, Alexandre Courbot wrote:
>>> /// Reset the controller, select the falcon core, and wait for memory scrubbing to complete.
>>> @@ -616,9 +462,10 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
>>> self.hal.select_core(self, bar)?;
>>> self.hal.reset_wait_mem_scrubbing(bar)?;
>>>
>>> - regs::NV_PFALCON_FALCON_RM::default()
>>> - .set_value(bar.read(regs::NV_PMC_BOOT_0).into())
>>> - .write(bar, &E::ID);
>>> + bar.write(
>>> + WithBase::of::<E>(),
>>> + regs::NV_PFALCON_FALCON_RM::from(bar.read(regs::NV_PMC_BOOT_0).into_raw()),
>>> + );
>>>
>>
>> Overall, I think the series is good improvement but I still feel this part is a
>> step back in readability, and we should come up with something better. I don't
>> think there's any chance anyone can memorize this syntax.
>
> I must reluctantly (because I know this conversation has gone very
> long, across so many versions) agree. That .write() statement is just
> brutal, and we will be relying on AI in order to even figure it out,
> I fear.
>
> I'd hoped that there were other, simpler forms to express this,
> is that not the case here?
It's on my radar to integrate `register!` and I/O projection (but of course, I
need to ship I/O projection first) to solve the relative register issue.
The idea is that instead of having an operation with all of `bar`, `E` and
`NV_PFALCON_FALCON_RM`, you first use projection (with a surface syntax that I
haven't considered yet) to go from `bar` to `View<'_, .., .E>` and then your
relative register becomes just a normal register on it.
Best,
Gary
>
> thanks,
^ permalink raw reply
* Re: [PATCH v2 08/10] gpu: nova-core: convert falcon registers to kernel register macro
From: Gary Guo @ 2026-03-20 20:23 UTC (permalink / raw)
To: Danilo Krummrich, John Hubbard
Cc: Joel Fernandes, Alexandre Courbot, Alice Ryhl, David Airlie,
Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, dri-devel, linux-kernel, linux-riscv, linux-doc,
rust-for-linux
In-Reply-To: <354b3e67-9bfb-4538-a865-695ebec6b433@kernel.org>
On Fri Mar 20, 2026 at 8:07 PM GMT, Danilo Krummrich wrote:
> On 3/20/26 8:52 PM, John Hubbard wrote:
>> On 3/20/26 10:38 AM, Joel Fernandes wrote:
>>> Hi Alex,
>>>
>>> On 3/20/2026 8:19 AM, Alexandre Courbot wrote:
>>>> /// Reset the controller, select the falcon core, and wait for memory scrubbing to complete.
>>>> @@ -616,9 +462,10 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
>>>> self.hal.select_core(self, bar)?;
>>>> self.hal.reset_wait_mem_scrubbing(bar)?;
>>>>
>>>> - regs::NV_PFALCON_FALCON_RM::default()
>>>> - .set_value(bar.read(regs::NV_PMC_BOOT_0).into())
>>>> - .write(bar, &E::ID);
>>>> + bar.write(
>>>> + WithBase::of::<E>(),
>>>> + regs::NV_PFALCON_FALCON_RM::from(bar.read(regs::NV_PMC_BOOT_0).into_raw()),
>>>> + );
>>>>
>>>
>>> Overall, I think the series is good improvement but I still feel this part is a
>>> step back in readability, and we should come up with something better. I don't
>>> think there's any chance anyone can memorize this syntax.
>>
>> I must reluctantly (because I know this conversation has gone very
>> long, across so many versions) agree. That .write() statement is just
>> brutal, and we will be relying on AI in order to even figure it out,
>> I fear.
>>
>> I'd hoped that there were other, simpler forms to express this,
>> is that not the case here?
>
> I mean, it is barely different from the above, except for the
> WithBase::of::<E>() argument instead of &E::ID.
>
> It becomes much more readable if you break it down (which is what we should
> probably do):
>
> let boot0 = bar.read(regs::NV_PMC_BOOT_0).into_raw();
>
> bar.write(WithBase::of::<E>(), regs::NV_PFALCON_FALCON_RM::from(boot0));
>
> For other cases where we read a register and write the same register we will
> have a way in the future that can omit WithBase::of::<E>() entirely, see also my
> reply in [1].
>
> I want to expand write_reg() to also be able to handle variable offset
> registers, e.g.:
>
> let reg = bar.read_reg(regs::NV_PFALCON_FALCON_RM::of::<E>());
>
> // modify reg
>
> bar.write_reg(reg);
If we add API that allows you to do projection based on a register, then you
could do:
let reg: View<'_, _, _> = bar.project(regs::NV_PFALCON_FALCON_RM::of::<E>());
let v = reg.read_val();
reg.write_val(v.with_foo());
I'm planning to introduce `read_val`/`write_val` on `io::View<'_, IO, integer primitives>`
but with some traits it should be doable for register types, too.
(I think `read_val`, `write_val` makes sense here because they're views of a
simple value. I cannot use `read`/`write` here because they're used by register
mechanism already).
Best,
Gary
^ permalink raw reply
* Re: [PATCH v3 04/16] mm: add vm_ops->mapped hook
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 20:43 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Andrew Morton, Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Jann Horn, Pedro Falcato,
linux-kernel, linux-doc, linux-hyperv, linux-mtd, linux-staging,
linux-scsi, target-devel, linux-afs, linux-fsdevel, linux-mm,
Ryan Roberts
In-Reply-To: <c9068fbb-a23c-4342-8638-3b11897a57cb@kernel.org>
On Fri, Mar 20, 2026 at 07:26:37PM +0100, Vlastimil Babka (SUSE) wrote:
> On 3/19/26 19:23, Lorenzo Stoakes (Oracle) wrote:
> > Previously, when a driver needed to do something like establish a
> > reference count, it could do so in the mmap hook in the knowledge that the
> > mapping would succeed.
> >
> > With the introduction of f_op->mmap_prepare this is no longer the case, as
> > it is invoked prior to actually establishing the mapping.
> >
> > mmap_prepare is not appropriate for this kind of thing as it is called
> > before any merge might take place, and after which an error might occur
> > meaning resources could be leaked.
> >
> > To take this into account, introduce a new vm_ops->mapped callback which
> > is invoked when the VMA is first mapped (though notably - not when it is
> > merged - which is correct and mirrors existing mmap/open/close behaviour).
> >
> > We do better that vm_ops->open() here, as this callback can return an
> > error, at which point the VMA will be unmapped.
> >
> > Note that vm_ops->mapped() is invoked after any mmap action is complete
> > (such as I/O remapping).
> >
> > We intentionally do not expose the VMA at this point, exposing only the
> > fields that could be used, and an output parameter in case the operation
> > needs to update the vma->vm_private_data field.
> >
> > In order to deal with stacked filesystems which invoke inner filesystem's
> > mmap() invocations, add __compat_vma_mapped() and invoke it on vfs_mmap()
> > (via compat_vma_mmap()) to ensure that the mapped callback is handled when
> > an mmap() caller invokes a nested filesystem's mmap_prepare() callback.
> >
> > We can now also remove call_action_complete() and invoke
> > mmap_action_complete() directly, as we separate out the rmap lock logic.
> >
> > The rmap lock logic, which was added in order to keep hugetlb working (!)
> > to allow for the rmap lock to be held longer, needs to be propagated to the
> > error paths on mmap complete and mapped hook error paths.
> >
> > This is because do_munmap() might otherwise deadlock with the rmap being
> > held, so instead we unlock at the point of unmap.
>
> Hmm but that was also true prior to this series? So is this a bugfix? Should
> it be a stable hotfix done outside of the series before the refactoring?
Yup, will send a hotfix.
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH v3 15/16] mm: add mmap_action_map_kernel_pages[_full]()
From: Nathan Chancellor @ 2026-03-20 21:08 UTC (permalink / raw)
To: Lorenzo Stoakes (Oracle)
Cc: Andrew Morton, Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <54ff3670662e10a66ce0c1a13c0ae93b99a5f201.1773944114.git.ljs@kernel.org>
Hi Lorenzo,
On Thu, Mar 19, 2026 at 06:23:39PM +0000, Lorenzo Stoakes (Oracle) wrote:
> A user can invoke mmap_action_map_kernel_pages() to specify that the
> mapping should map kernel pages starting from desc->start of a specified
> number of pages specified in an array.
>
> In order to implement this, adjust mmap_action_prepare() to be able to
> return an error code, as it makes sense to assert that the specified
> parameters are valid as quickly as possible as well as updating the VMA
> flags to include VMA_MIXEDMAP_BIT as necessary.
>
> This provides an mmap_prepare equivalent of vm_insert_pages(). We
> additionally update the existing vm_insert_pages() code to use
> range_in_vma() and add a new range_in_vma_desc() helper function for the
> mmap_prepare case, sharing the code between the two in range_is_subset().
>
> We add both mmap_action_map_kernel_pages() and
> mmap_action_map_kernel_pages_full() to allow for both partial and full VMA
> mappings.
>
> We update the documentation to reflect the new features.
>
> Finally, we update the VMA tests accordingly to reflect the changes.
>
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
...
> diff --git a/mm/util.c b/mm/util.c
> index 8cf59267a9ac..682d0d24e1c6 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -1446,6 +1446,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)
> return io_remap_pfn_range_prepare(desc);
> case MMAP_SIMPLE_IO_REMAP:
> return simple_ioremap_prepare(desc);
> + case MMAP_MAP_KERNEL_PAGES:
> + return map_kernel_pages_prepare(desc);
> }
>
> WARN_ON_ONCE(1);
> @@ -1476,6 +1478,9 @@ int mmap_action_complete(struct vm_area_struct *vma,
> case MMAP_REMAP_PFN:
> err = remap_pfn_range_complete(vma, action);
> break;
> + case MMAP_MAP_KERNEL_PAGES:
> + err = map_kernel_pages_complete(vma, action);
> + break;
> case MMAP_IO_REMAP_PFN:
> case MMAP_SIMPLE_IO_REMAP:
> /* Should have been delegated. */
> @@ -1497,6 +1502,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)
> case MMAP_REMAP_PFN:
> case MMAP_IO_REMAP_PFN:
> case MMAP_SIMPLE_IO_REMAP:
> + case MMAP_MAP_KERNEL_PAGES:
> WARN_ON_ONCE(1); /* nommu cannot handle these. */
> break;
> }
Not sure if it has been reported/addressed yet but it looks like
mmap_action_complete() was missed here, as pointed out by clang:
$ make -skj"$(nproc)" ARCH=arm LLVM=1 mrproper allnoconfig mm/util.o
mm/util.c:1520:10: warning: enumeration value 'MMAP_MAP_KERNEL_PAGES' not handled in switch [-Wswitch]
1520 | switch (action->type) {
| ^~~~~~~~~~~~
I assume
diff --git a/mm/util.c b/mm/util.c
index 682d0d24e1c6..c41c119a5a74 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1523,6 +1523,7 @@ int mmap_action_complete(struct vm_area_struct *vma,
case MMAP_REMAP_PFN:
case MMAP_IO_REMAP_PFN:
case MMAP_SIMPLE_IO_REMAP:
+ case MMAP_MAP_KERNEL_PAGES:
WARN_ON_ONCE(1); /* nommu cannot handle this. */
err = -EINVAL;
--
should be the fix?
Cheers,
Nathan
^ permalink raw reply related
* Re: [PATCH v3 15/16] mm: add mmap_action_map_kernel_pages[_full]()
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:14 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Andrew Morton, Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <20260320210812.GA3988975@ax162>
On Fri, Mar 20, 2026 at 02:08:12PM -0700, Nathan Chancellor wrote:
> Hi Lorenzo,
>
> On Thu, Mar 19, 2026 at 06:23:39PM +0000, Lorenzo Stoakes (Oracle) wrote:
> > A user can invoke mmap_action_map_kernel_pages() to specify that the
> > mapping should map kernel pages starting from desc->start of a specified
> > number of pages specified in an array.
> >
> > In order to implement this, adjust mmap_action_prepare() to be able to
> > return an error code, as it makes sense to assert that the specified
> > parameters are valid as quickly as possible as well as updating the VMA
> > flags to include VMA_MIXEDMAP_BIT as necessary.
> >
> > This provides an mmap_prepare equivalent of vm_insert_pages(). We
> > additionally update the existing vm_insert_pages() code to use
> > range_in_vma() and add a new range_in_vma_desc() helper function for the
> > mmap_prepare case, sharing the code between the two in range_is_subset().
> >
> > We add both mmap_action_map_kernel_pages() and
> > mmap_action_map_kernel_pages_full() to allow for both partial and full VMA
> > mappings.
> >
> > We update the documentation to reflect the new features.
> >
> > Finally, we update the VMA tests accordingly to reflect the changes.
> >
> > Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> > Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
> ...
> > diff --git a/mm/util.c b/mm/util.c
> > index 8cf59267a9ac..682d0d24e1c6 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -1446,6 +1446,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)
> > return io_remap_pfn_range_prepare(desc);
> > case MMAP_SIMPLE_IO_REMAP:
> > return simple_ioremap_prepare(desc);
> > + case MMAP_MAP_KERNEL_PAGES:
> > + return map_kernel_pages_prepare(desc);
> > }
> >
> > WARN_ON_ONCE(1);
> > @@ -1476,6 +1478,9 @@ int mmap_action_complete(struct vm_area_struct *vma,
> > case MMAP_REMAP_PFN:
> > err = remap_pfn_range_complete(vma, action);
> > break;
> > + case MMAP_MAP_KERNEL_PAGES:
> > + err = map_kernel_pages_complete(vma, action);
> > + break;
> > case MMAP_IO_REMAP_PFN:
> > case MMAP_SIMPLE_IO_REMAP:
> > /* Should have been delegated. */
> > @@ -1497,6 +1502,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)
> > case MMAP_REMAP_PFN:
> > case MMAP_IO_REMAP_PFN:
> > case MMAP_SIMPLE_IO_REMAP:
> > + case MMAP_MAP_KERNEL_PAGES:
> > WARN_ON_ONCE(1); /* nommu cannot handle these. */
> > break;
> > }
>
> Not sure if it has been reported/addressed yet but it looks like
> mmap_action_complete() was missed here, as pointed out by clang:
>
> $ make -skj"$(nproc)" ARCH=arm LLVM=1 mrproper allnoconfig mm/util.o
> mm/util.c:1520:10: warning: enumeration value 'MMAP_MAP_KERNEL_PAGES' not handled in switch [-Wswitch]
> 1520 | switch (action->type) {
> | ^~~~~~~~~~~~
>
> I assume
>
> diff --git a/mm/util.c b/mm/util.c
> index 682d0d24e1c6..c41c119a5a74 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -1523,6 +1523,7 @@ int mmap_action_complete(struct vm_area_struct *vma,
> case MMAP_REMAP_PFN:
> case MMAP_IO_REMAP_PFN:
> case MMAP_SIMPLE_IO_REMAP:
> + case MMAP_MAP_KERNEL_PAGES:
> WARN_ON_ONCE(1); /* nommu cannot handle this. */
>
> err = -EINVAL;
> --
>
> should be the fix?
>
> Cheers,
> Nathan
Thanks, will fix, working on a respin now anyway :)
Cheers, Lorenzo
^ permalink raw reply
* [PATCH v4 00/21] mm: expand mmap_prepare functionality and usage
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
This series expands the mmap_prepare functionality, which is intended to
replace the deprecated f_op->mmap hook which has been the source of bugs
and security issues for some time.
This series starts with some cleanup of existing mmap_prepare logic, then
adds documentation for the mmap_prepare call to make it easier for
filesystem and driver writers to understand how it works.
It then importantly adds a vm_ops->mapped hook, a key feature that was
missing from mmap_prepare previously - this is invoked when a driver which
specifies mmap_prepare has successfully been mapped but not merged with
another VMA.
mmap_prepare is invoked prior to a merge being attempted, so you cannot
manipulate state such as reference counts as if it were a new mapping.
The vm_ops->mapped hook allows a driver to perform tasks required at this
stage, and provides symmetry against subsequent vm_ops->open,close calls.
The series uses this to correct the afs implementation which wrongly
manipulated reference count at mmap_prepare time.
It then adds an mmap_prepare equivalent of vm_iomap_memory() -
mmap_action_simple_ioremap(), then uses this to update a number of drivers.
It then splits out the mmap_prepare compatibility layer (which allows for
invocation of mmap_prepare hooks in an mmap() hook) in such a way as to
allow for more incremental implementation of mmap_prepare hooks.
It then uses this to extend mmap_prepare usage in drivers.
Finally it adds an mmap_prepare equivalent of vm_map_pages(), which lays
the foundation for future work which will extend mmap_prepare to DMA
coherent mappings.
v4:
* Added partial revert of AFS as per Vlasta. Labelled as hotfix.
* Updated subsequent afs patch to apply against this version of AFS.
* Reverted rmap_lock_held changes to util.c, mm.h mmap_action_complete()
etc. as per Vlasta.
* Added hotfix to fix issue with rmap lock held over munmap() as per
Vlasta. Labelled as hotfix.
* Force-disable the rmap lock hold feature in the compatbility layer
because being run under the mmap hook eliminates the need for it.
* Removed superfluous map->hold_file_rmap_lock field.
* Moved handling of rmap lock and unmapping to mmap_action_complete().
* Removed unmap_vma_locked() as previous added patches render it
unnecessary.
* Removed __compat_vma_mapped() from compatibility layer and
call_vma_mapped() from VMA layer and made it part of mmap_action_finish()
for all callers.
* Propagated changes to VMA tests.
* Updated mmap_action_map_kernel_pages[_full]() patch to add missing
mmap_complete() noop switch enum value as per Nathan.
* Fixed a doc issue in the mmap_prepare docs - reference
vma_desc_test_flags() rather than _any().
* Rearranged logic so the vm_ops->mapped hook is called before the success
hook, but this should have no impact.
v3:
* Propagated tags (thanks Suren, Richard!)
* Updated 12/16 to correctly clear the vm_area_desc data structure in
set_desc_from_vma() as per Joshua Hahn (thanks! :)
* Fixed type in 12/16 as per Suren (cheers!)
* Fixed up 6/16 to use mmap_action_ioremap_full() in simple_ioremap_prepare() as
suggested by Suren.
* Also fixed up 6/16 to call io_remap_pfn_range_prepare() direct rather than
mmap_action_prepare() as per Suren.
* Also fixed up 6/16 to pass vm_len rather than vm_[start, end] to
__simple_ioremap_prep() as per Suren (thanks for all the above! :)
* Fixed issue in rmap lock being held - we were referencing a vma->vm_file after
the VMA was unmapped, so UAF. Avoid that. Also do_munmap() relies on rmap lock
NOT being held or may deadlock, so extend functionality to ensure we drop it
when it is held on error paths.
* Updated 'area' -> 'vma' variable in 3/16 in VMA test dup.h.
* Fixed up reference to __compat_vma_mmap() in 12/16 commit message.
* Updated 1/16 to no longer duplicatively apply io_remap_pfn_range_pfn().
* Updated 1/16 to delegate I/O remap complete to remap complete logic.
* Fixed various typos in 12/16.
* Fixed stale comment typos in 13/16.
* Fixed commit msg and comment typos in 14/16.
* Removed accidental sneak peak to future functionality in 15/16 commit message
:).
* Fixed up field names to be identical in VMA tests + mm_types.h in 6/16,
15/16.
https://lore.kernel.org/all/cover.1773944114.git.ljs@kernel.org/
v2:
* Rebased on
https://lore.kernel.org/all/cover.1773665966.git.ljs@kernel.org/ to make
Andrew's life easier :)
* Folded all interim fixes into series (thanks Randy for many doc fixes!))
* As per Suren, removed a comment about allocations too small to fail.
* As per Randy, fixed up typo in documentation for vm_area_desc.
* Fixed mmap_action_prepare() not returning if invalid action->type
specified, as updated from Andrew's interim fix (thanks!) and also
reported by kernel test bot.
* Updated mmap_action_prepare() and specific prepare functions to only
pass vm_area_desc parameter as per Suren.
* Fixed up whitespace as per Suren.
* Updated vm_op->open comment in vm_operations_struct to reference forking
as per Suren.
* Added a commit to check that input range is within VMA on remap as per
Suren (this also covers I/O remap and all other cases already asserted).
* Updated AFS to not incorrectly reference count on mmap prepare as per
Usama.
* Also updated various static AFS functions to be consistent with each
other.
* Updated AFS commit message to reflect mmap_prepare being before any VMA
merging as per Suren.
* Updated __compat_vma_mapped() to check for NULL vm_ops as per Usama.
* Updated __compat_vma_mapped() to not reference an unmapped VMA's fields
as per Usama.
* Updated __vma_check_mmap_hook() to check for NULL vm_ops as per Usama.
* Dropped comment about preferring mmap_prepare as seems overly confusing,
as per Suren.
* Updated the mmap lock assert in unmap_vma_locked() to a write lock assert
as per Suren.
* Copied vm_ops->open comment over to VMA tests in appropriate patch as per
Suren.
* Updated mmap_prepare documentation to reflect the fact that no resources
should be allocated upon mmap_prepare.
* Updated mmap_prepare documentation to reference the vm_ops->mapped
callback.
* Fixed stray markdown '## How to use' in documentation.
* Fixed bug reported by kernel test bot re: overlooked
vma_desc_test_flags() -> vma_desc_test() in MTD driver for nommu.
https://lore.kernel.org/linux-mm/cover.1773695307.git.ljs@kernel.org/
v1:
https://lore.kernel.org/linux-mm/cover.1773346620.git.ljs@kernel.org/
Lorenzo Stoakes (Oracle) (21):
mm: various small mmap_prepare cleanups
mm: add documentation for the mmap_prepare file operation callback
mm: document vm_operations_struct->open the same as close()
mm: avoid deadlock when holding rmap on mmap_prepare error
mm: switch the rmap lock held option off in compat layer
mm/vma: remove superfluous map->hold_file_rmap_lock
mm: have mmap_action_complete() handle the rmap lock and unmap
mm: add vm_ops->mapped hook
fs: afs: revert mmap_prepare() change
fs: afs: restore mmap_prepare implementation
mm: add mmap_action_simple_ioremap()
misc: open-dice: replace deprecated mmap hook with mmap_prepare
hpet: replace deprecated mmap hook with mmap_prepare
mtdchar: replace deprecated mmap hook with mmap_prepare, clean up
stm: replace deprecated mmap hook with mmap_prepare
staging: vme_user: replace deprecated mmap hook with mmap_prepare
mm: allow handling of stacked mmap_prepare hooks in more drivers
drivers: hv: vmbus: replace deprecated mmap hook with mmap_prepare
uio: replace deprecated mmap hook with mmap_prepare in uio_info
mm: add mmap_action_map_kernel_pages[_full]()
mm: on remap assert that input range within the proposed VMA
Documentation/driver-api/vme.rst | 2 +-
Documentation/filesystems/index.rst | 1 +
Documentation/filesystems/mmap_prepare.rst | 168 ++++++++++++++
drivers/char/hpet.c | 12 +-
drivers/hv/hyperv_vmbus.h | 4 +-
drivers/hv/vmbus_drv.c | 31 ++-
drivers/hwtracing/stm/core.c | 31 ++-
drivers/misc/open-dice.c | 19 +-
drivers/mtd/mtdchar.c | 21 +-
drivers/staging/vme_user/vme.c | 20 +-
drivers/staging/vme_user/vme.h | 2 +-
drivers/staging/vme_user/vme_user.c | 51 +++--
drivers/target/target_core_user.c | 26 ++-
drivers/uio/uio.c | 10 +-
drivers/uio/uio_hv_generic.c | 11 +-
fs/afs/file.c | 36 ++-
include/linux/fs.h | 14 +-
include/linux/hyperv.h | 4 +-
include/linux/mm.h | 158 ++++++++++++-
include/linux/mm_types.h | 17 +-
include/linux/uio_driver.h | 4 +-
mm/internal.h | 46 +++-
mm/memory.c | 175 ++++++++++----
mm/util.c | 251 ++++++++++++++-------
mm/vma.c | 48 ++--
mm/vma.h | 2 +-
tools/testing/vma/include/dup.h | 134 +++++++----
tools/testing/vma/include/stubs.h | 8 +-
28 files changed, 956 insertions(+), 350 deletions(-)
create mode 100644 Documentation/filesystems/mmap_prepare.rst
--
2.53.0
^ permalink raw reply
* [PATCH v4 01/21] mm: various small mmap_prepare cleanups
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Rather than passing arbitrary fields, pass a vm_area_desc pointer to mmap
prepare functions to mmap prepare, and an action and vma pointer to mmap
complete in order to put all the action-specific logic in the function
actually doing the work.
Additionally, allow mmap prepare functions to return an error so we can
error out as soon as possible if there is something logically incorrect in
the input.
Update remap_pfn_range_prepare() to properly check the input range for the
CoW case.
Also remove io_remap_pfn_range_complete(), as we can simply set up the
fields correctly in io_remap_pfn_range_prepare() and use
remap_pfn_range_complete() for this.
While we're here, make remap_pfn_range_prepare_vma() a little neater, and
pass mmap_action directly to call_action_complete().
Then, update compat_vma_mmap() to perform its logic directly, as
__compat_vma_map() is not used by anything so we don't need to export it.
Also update compat_vma_mmap() to use vfs_mmap_prepare() rather than
calling the mmap_prepare op directly.
Finally, update the VMA userland tests to reflect the changes.
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
include/linux/fs.h | 2 -
include/linux/mm.h | 7 +-
mm/internal.h | 32 ++++----
mm/memory.c | 45 +++++++----
mm/util.c | 121 +++++++++++++-----------------
mm/vma.c | 24 +++---
tools/testing/vma/include/dup.h | 7 +-
tools/testing/vma/include/stubs.h | 8 +-
8 files changed, 126 insertions(+), 120 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8b3dd145b25e..a2628a12bd2b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2058,8 +2058,6 @@ static inline bool can_mmap_file(struct file *file)
return true;
}
-int __compat_vma_mmap(const struct file_operations *f_op,
- struct file *file, struct vm_area_struct *vma);
int compat_vma_mmap(struct file *file, struct vm_area_struct *vma);
static inline int vfs_mmap(struct file *file, struct vm_area_struct *vma)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 42cc40aa63d9..1e63b3a44a47 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4320,10 +4320,9 @@ static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,
mmap_action_ioremap(desc, desc->start, start_pfn, vma_desc_size(desc));
}
-void mmap_action_prepare(struct mmap_action *action,
- struct vm_area_desc *desc);
-int mmap_action_complete(struct mmap_action *action,
- struct vm_area_struct *vma);
+int mmap_action_prepare(struct vm_area_desc *desc);
+int mmap_action_complete(struct vm_area_struct *vma,
+ struct mmap_action *action);
/* Look up the first VMA which exactly match the interval vm_start ... vm_end */
static inline struct vm_area_struct *find_exact_vma(struct mm_struct *mm,
diff --git a/mm/internal.h b/mm/internal.h
index 708d240b4198..0256ca44115a 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1793,26 +1793,28 @@ int walk_page_range_debug(struct mm_struct *mm, unsigned long start,
void dup_mm_exe_file(struct mm_struct *mm, struct mm_struct *oldmm);
int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm);
-void remap_pfn_range_prepare(struct vm_area_desc *desc, unsigned long pfn);
-int remap_pfn_range_complete(struct vm_area_struct *vma, unsigned long addr,
- unsigned long pfn, unsigned long size, pgprot_t pgprot);
+int remap_pfn_range_prepare(struct vm_area_desc *desc);
+int remap_pfn_range_complete(struct vm_area_struct *vma,
+ struct mmap_action *action);
-static inline void io_remap_pfn_range_prepare(struct vm_area_desc *desc,
- unsigned long orig_pfn, unsigned long size)
+static inline int io_remap_pfn_range_prepare(struct vm_area_desc *desc)
{
+ struct mmap_action *action = &desc->action;
+ const unsigned long orig_pfn = action->remap.start_pfn;
+ const pgprot_t orig_pgprot = action->remap.pgprot;
+ const unsigned long size = action->remap.size;
const unsigned long pfn = io_remap_pfn_range_pfn(orig_pfn, size);
+ int err;
- return remap_pfn_range_prepare(desc, pfn);
-}
+ action->remap.start_pfn = pfn;
+ action->remap.pgprot = pgprot_decrypted(orig_pgprot);
+ err = remap_pfn_range_prepare(desc);
+ if (err)
+ return err;
-static inline int io_remap_pfn_range_complete(struct vm_area_struct *vma,
- unsigned long addr, unsigned long orig_pfn, unsigned long size,
- pgprot_t orig_prot)
-{
- const unsigned long pfn = io_remap_pfn_range_pfn(orig_pfn, size);
- const pgprot_t prot = pgprot_decrypted(orig_prot);
-
- return remap_pfn_range_complete(vma, addr, pfn, size, prot);
+ /* Remap does the actual work. */
+ action->type = MMAP_REMAP_PFN;
+ return 0;
}
#ifdef CONFIG_MMU_NOTIFIER
diff --git a/mm/memory.c b/mm/memory.c
index 219b9bf6cae0..9dec67a18116 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3099,26 +3099,34 @@ static int do_remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
}
#endif
-void remap_pfn_range_prepare(struct vm_area_desc *desc, unsigned long pfn)
+int remap_pfn_range_prepare(struct vm_area_desc *desc)
{
- /*
- * We set addr=VMA start, end=VMA end here, so this won't fail, but we
- * check it again on complete and will fail there if specified addr is
- * invalid.
- */
- get_remap_pgoff(vma_desc_is_cow_mapping(desc), desc->start, desc->end,
- desc->start, desc->end, pfn, &desc->pgoff);
+ const struct mmap_action *action = &desc->action;
+ const unsigned long start = action->remap.start;
+ const unsigned long end = start + action->remap.size;
+ const unsigned long pfn = action->remap.start_pfn;
+ const bool is_cow = vma_desc_is_cow_mapping(desc);
+ int err;
+
+ err = get_remap_pgoff(is_cow, start, end, desc->start, desc->end, pfn,
+ &desc->pgoff);
+ if (err)
+ return err;
+
vma_desc_set_flags_mask(desc, VMA_REMAP_FLAGS);
+ return 0;
}
-static int remap_pfn_range_prepare_vma(struct vm_area_struct *vma, unsigned long addr,
- unsigned long pfn, unsigned long size)
+static int remap_pfn_range_prepare_vma(struct vm_area_struct *vma,
+ unsigned long addr, unsigned long pfn,
+ unsigned long size)
{
- unsigned long end = addr + PAGE_ALIGN(size);
+ const unsigned long end = addr + PAGE_ALIGN(size);
+ const bool is_cow = is_cow_mapping(vma->vm_flags);
int err;
- err = get_remap_pgoff(is_cow_mapping(vma->vm_flags), addr, end,
- vma->vm_start, vma->vm_end, pfn, &vma->vm_pgoff);
+ err = get_remap_pgoff(is_cow, addr, end, vma->vm_start, vma->vm_end,
+ pfn, &vma->vm_pgoff);
if (err)
return err;
@@ -3151,10 +3159,15 @@ int remap_pfn_range(struct vm_area_struct *vma, unsigned long addr,
}
EXPORT_SYMBOL(remap_pfn_range);
-int remap_pfn_range_complete(struct vm_area_struct *vma, unsigned long addr,
- unsigned long pfn, unsigned long size, pgprot_t prot)
+int remap_pfn_range_complete(struct vm_area_struct *vma,
+ struct mmap_action *action)
{
- return do_remap_pfn_range(vma, addr, pfn, size, prot);
+ const unsigned long start = action->remap.start;
+ const unsigned long pfn = action->remap.start_pfn;
+ const unsigned long size = action->remap.size;
+ const pgprot_t prot = action->remap.pgprot;
+
+ return do_remap_pfn_range(vma, start, pfn, size, prot);
}
/**
diff --git a/mm/util.c b/mm/util.c
index ce7ae80047cf..73c97a748d8e 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1163,43 +1163,6 @@ void flush_dcache_folio(struct folio *folio)
EXPORT_SYMBOL(flush_dcache_folio);
#endif
-/**
- * __compat_vma_mmap() - See description for compat_vma_mmap()
- * for details. This is the same operation, only with a specific file operations
- * struct which may or may not be the same as vma->vm_file->f_op.
- * @f_op: The file operations whose .mmap_prepare() hook is specified.
- * @file: The file which backs or will back the mapping.
- * @vma: The VMA to apply the .mmap_prepare() hook to.
- * Returns: 0 on success or error.
- */
-int __compat_vma_mmap(const struct file_operations *f_op,
- struct file *file, struct vm_area_struct *vma)
-{
- struct vm_area_desc desc = {
- .mm = vma->vm_mm,
- .file = file,
- .start = vma->vm_start,
- .end = vma->vm_end,
-
- .pgoff = vma->vm_pgoff,
- .vm_file = vma->vm_file,
- .vma_flags = vma->flags,
- .page_prot = vma->vm_page_prot,
-
- .action.type = MMAP_NOTHING, /* Default */
- };
- int err;
-
- err = f_op->mmap_prepare(&desc);
- if (err)
- return err;
-
- mmap_action_prepare(&desc.action, &desc);
- set_vma_from_desc(vma, &desc);
- return mmap_action_complete(&desc.action, vma);
-}
-EXPORT_SYMBOL(__compat_vma_mmap);
-
/**
* compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an
* existing VMA and execute any requested actions.
@@ -1228,7 +1191,31 @@ EXPORT_SYMBOL(__compat_vma_mmap);
*/
int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
{
- return __compat_vma_mmap(file->f_op, file, vma);
+ struct vm_area_desc desc = {
+ .mm = vma->vm_mm,
+ .file = file,
+ .start = vma->vm_start,
+ .end = vma->vm_end,
+
+ .pgoff = vma->vm_pgoff,
+ .vm_file = vma->vm_file,
+ .vma_flags = vma->flags,
+ .page_prot = vma->vm_page_prot,
+
+ .action.type = MMAP_NOTHING, /* Default */
+ };
+ int err;
+
+ err = vfs_mmap_prepare(file, &desc);
+ if (err)
+ return err;
+
+ err = mmap_action_prepare(&desc);
+ if (err)
+ return err;
+
+ set_vma_from_desc(vma, &desc);
+ return mmap_action_complete(vma, &desc.action);
}
EXPORT_SYMBOL(compat_vma_mmap);
@@ -1320,8 +1307,8 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
}
}
-static int mmap_action_finish(struct mmap_action *action,
- const struct vm_area_struct *vma, int err)
+static int mmap_action_finish(struct vm_area_struct *vma,
+ struct mmap_action *action, int err)
{
/*
* If an error occurs, unmap the VMA altogether and return an error. We
@@ -1353,37 +1340,38 @@ static int mmap_action_finish(struct mmap_action *action,
/**
* mmap_action_prepare - Perform preparatory setup for an VMA descriptor
* action which need to be performed.
- * @desc: The VMA descriptor to prepare for @action.
- * @action: The action to perform.
+ * @desc: The VMA descriptor to prepare for its @desc->action.
+ *
+ * Returns: %0 on success, otherwise error.
*/
-void mmap_action_prepare(struct mmap_action *action,
- struct vm_area_desc *desc)
+int mmap_action_prepare(struct vm_area_desc *desc)
{
- switch (action->type) {
+ switch (desc->action.type) {
case MMAP_NOTHING:
- break;
+ return 0;
case MMAP_REMAP_PFN:
- remap_pfn_range_prepare(desc, action->remap.start_pfn);
- break;
+ return remap_pfn_range_prepare(desc);
case MMAP_IO_REMAP_PFN:
- io_remap_pfn_range_prepare(desc, action->remap.start_pfn,
- action->remap.size);
- break;
+ return io_remap_pfn_range_prepare(desc);
}
+
+ WARN_ON_ONCE(1);
+ return -EINVAL;
}
EXPORT_SYMBOL(mmap_action_prepare);
/**
* mmap_action_complete - Execute VMA descriptor action.
- * @action: The action to perform.
* @vma: The VMA to perform the action upon.
+ * @action: The action to perform.
*
* Similar to mmap_action_prepare().
*
* Return: 0 on success, or error, at which point the VMA will be unmapped.
*/
-int mmap_action_complete(struct mmap_action *action,
- struct vm_area_struct *vma)
+int mmap_action_complete(struct vm_area_struct *vma,
+ struct mmap_action *action)
+
{
int err = 0;
@@ -1391,25 +1379,22 @@ int mmap_action_complete(struct mmap_action *action,
case MMAP_NOTHING:
break;
case MMAP_REMAP_PFN:
- err = remap_pfn_range_complete(vma, action->remap.start,
- action->remap.start_pfn, action->remap.size,
- action->remap.pgprot);
+ err = remap_pfn_range_complete(vma, action);
break;
case MMAP_IO_REMAP_PFN:
- err = io_remap_pfn_range_complete(vma, action->remap.start,
- action->remap.start_pfn, action->remap.size,
- action->remap.pgprot);
+ /* Should have been delegated. */
+ WARN_ON_ONCE(1);
+ err = -EINVAL;
break;
}
- return mmap_action_finish(action, vma, err);
+ return mmap_action_finish(vma, action, err);
}
EXPORT_SYMBOL(mmap_action_complete);
#else
-void mmap_action_prepare(struct mmap_action *action,
- struct vm_area_desc *desc)
+int mmap_action_prepare(struct vm_area_desc *desc)
{
- switch (action->type) {
+ switch (desc->action.type) {
case MMAP_NOTHING:
break;
case MMAP_REMAP_PFN:
@@ -1417,11 +1402,13 @@ void mmap_action_prepare(struct mmap_action *action,
WARN_ON_ONCE(1); /* nommu cannot handle these. */
break;
}
+
+ return 0;
}
EXPORT_SYMBOL(mmap_action_prepare);
-int mmap_action_complete(struct mmap_action *action,
- struct vm_area_struct *vma)
+int mmap_action_complete(struct vm_area_struct *vma,
+ struct mmap_action *action)
{
int err = 0;
@@ -1436,7 +1423,7 @@ int mmap_action_complete(struct mmap_action *action,
break;
}
- return mmap_action_finish(action, vma, err);
+ return mmap_action_finish(vma, action, err);
}
EXPORT_SYMBOL(mmap_action_complete);
#endif
diff --git a/mm/vma.c b/mm/vma.c
index 8cccaeb8ccbb..ee91f2b76acf 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2641,15 +2641,18 @@ static void __mmap_complete(struct mmap_state *map, struct vm_area_struct *vma)
vma_set_page_prot(vma);
}
-static void call_action_prepare(struct mmap_state *map,
- struct vm_area_desc *desc)
+static int call_action_prepare(struct mmap_state *map,
+ struct vm_area_desc *desc)
{
- struct mmap_action *action = &desc->action;
+ int err;
- mmap_action_prepare(action, desc);
+ err = mmap_action_prepare(desc);
+ if (err)
+ return err;
- if (action->hide_from_rmap_until_complete)
+ if (desc->action.hide_from_rmap_until_complete)
map->hold_file_rmap_lock = true;
+ return 0;
}
/*
@@ -2673,7 +2676,9 @@ static int call_mmap_prepare(struct mmap_state *map,
if (err)
return err;
- call_action_prepare(map, desc);
+ err = call_action_prepare(map, desc);
+ if (err)
+ return err;
/* Update fields permitted to be changed. */
map->pgoff = desc->pgoff;
@@ -2728,13 +2733,12 @@ static bool can_set_ksm_flags_early(struct mmap_state *map)
}
static int call_action_complete(struct mmap_state *map,
- struct vm_area_desc *desc,
+ struct mmap_action *action,
struct vm_area_struct *vma)
{
- struct mmap_action *action = &desc->action;
int ret;
- ret = mmap_action_complete(action, vma);
+ ret = mmap_action_complete(vma, action);
/* If we held the file rmap we need to release it. */
if (map->hold_file_rmap_lock) {
@@ -2796,7 +2800,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
__mmap_complete(&map, vma);
if (have_mmap_prepare && allocated_new) {
- error = call_action_complete(&map, &desc, vma);
+ error = call_action_complete(&map, &desc.action, vma);
if (error)
return error;
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index b69eefba4cf7..477a5be65dd2 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1273,9 +1273,12 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,
if (err)
return err;
- mmap_action_prepare(&desc.action, &desc);
+ err = mmap_action_prepare(&desc);
+ if (err)
+ return err;
+
set_vma_from_desc(vma, &desc);
- return mmap_action_complete(&desc.action, vma);
+ return mmap_action_complete(vma, &desc.action);
}
static inline int compat_vma_mmap(struct file *file,
diff --git a/tools/testing/vma/include/stubs.h b/tools/testing/vma/include/stubs.h
index 5afb0afe2d48..a30b8bc84955 100644
--- a/tools/testing/vma/include/stubs.h
+++ b/tools/testing/vma/include/stubs.h
@@ -81,13 +81,13 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma)
{
}
-static inline void mmap_action_prepare(struct mmap_action *action,
- struct vm_area_desc *desc)
+static inline int mmap_action_prepare(struct vm_area_desc *desc)
{
+ return 0;
}
-static inline int mmap_action_complete(struct mmap_action *action,
- struct vm_area_struct *vma)
+static inline int mmap_action_complete(struct vm_area_struct *vma,
+ struct mmap_action *action)
{
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v4 02/21] mm: add documentation for the mmap_prepare file operation callback
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
This documentation makes it easier for a driver/file system implementer to
correctly use this callback.
It covers the fundamentals, whilst intentionally leaving the less lovely
possible actions one might take undocumented (for instance - the
success_hook, error_hook fields in mmap_action).
The document also covers the new VMA flags implementation which is the
only one which will work correctly with mmap_prepare.
Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
Documentation/filesystems/index.rst | 1 +
Documentation/filesystems/mmap_prepare.rst | 142 +++++++++++++++++++++
2 files changed, 143 insertions(+)
create mode 100644 Documentation/filesystems/mmap_prepare.rst
diff --git a/Documentation/filesystems/index.rst b/Documentation/filesystems/index.rst
index f4873197587d..6cbc3e0292ae 100644
--- a/Documentation/filesystems/index.rst
+++ b/Documentation/filesystems/index.rst
@@ -29,6 +29,7 @@ algorithms work.
fiemap
files
locks
+ mmap_prepare
multigrain-ts
mount_api
quota
diff --git a/Documentation/filesystems/mmap_prepare.rst b/Documentation/filesystems/mmap_prepare.rst
new file mode 100644
index 000000000000..ae484d371861
--- /dev/null
+++ b/Documentation/filesystems/mmap_prepare.rst
@@ -0,0 +1,142 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================
+mmap_prepare callback HOWTO
+===========================
+
+Introduction
+============
+
+The ``struct file->f_op->mmap()`` callback has been deprecated as it is both a
+stability and security risk, and doesn't always permit the merging of adjacent
+mappings resulting in unnecessary memory fragmentation.
+
+It has been replaced with the ``file->f_op->mmap_prepare()`` callback which
+solves these problems.
+
+This hook is called right at the beginning of setting up the mapping, and
+importantly it is invoked *before* any merging of adjacent mappings has taken
+place.
+
+If an error arises upon mapping, it might arise after this callback has been
+invoked, therefore it should be treated as effectively stateless.
+
+That is - no resources should be allocated nor state updated to reflect that a
+mapping has been established, as the mapping may either be merged, or fail to be
+mapped after the callback is complete.
+
+How To Use
+==========
+
+In your driver's struct file_operations struct, specify an ``mmap_prepare``
+callback rather than an ``mmap`` one, e.g. for ext4:
+
+.. code-block:: C
+
+ const struct file_operations ext4_file_operations = {
+ ...
+ .mmap_prepare = ext4_file_mmap_prepare,
+ };
+
+This has a signature of ``int (*mmap_prepare)(struct vm_area_desc *)``.
+
+Examining the struct vm_area_desc type:
+
+.. code-block:: C
+
+ struct vm_area_desc {
+ /* Immutable state. */
+ const struct mm_struct *const mm;
+ struct file *const file; /* May vary from vm_file in stacked callers. */
+ unsigned long start;
+ unsigned long end;
+
+ /* Mutable fields. Populated with initial state. */
+ pgoff_t pgoff;
+ struct file *vm_file;
+ vma_flags_t vma_flags;
+ pgprot_t page_prot;
+
+ /* Write-only fields. */
+ const struct vm_operations_struct *vm_ops;
+ void *private_data;
+
+ /* Take further action? */
+ struct mmap_action action;
+ };
+
+This is straightforward - you have all the fields you need to set up the
+mapping, and you can update the mutable and writable fields, for instance:
+
+.. code-block:: C
+
+ static int ext4_file_mmap_prepare(struct vm_area_desc *desc)
+ {
+ int ret;
+ struct file *file = desc->file;
+ struct inode *inode = file->f_mapping->host;
+
+ ...
+
+ file_accessed(file);
+ if (IS_DAX(file_inode(file))) {
+ desc->vm_ops = &ext4_dax_vm_ops;
+ vma_desc_set_flags(desc, VMA_HUGEPAGE_BIT);
+ } else {
+ desc->vm_ops = &ext4_file_vm_ops;
+ }
+ return 0;
+ }
+
+Importantly, you no longer have to dance around with reference counts or locks
+when updating these fields - **you can simply go ahead and change them**.
+
+Everything is taken care of by the mapping code.
+
+VMA Flags
+---------
+
+Along with ``mmap_prepare``, VMA flags have undergone an overhaul. Where before
+you would invoke one of vm_flags_init(), vm_flags_reset(), vm_flags_set(),
+vm_flags_clear(), and vm_flags_mod() to modify flags (and to have the
+locking done correctly for you, this is no longer necessary.
+
+Also, the legacy approach of specifying VMA flags via ``VM_READ``, ``VM_WRITE``,
+etc. - i.e. using a ``-VM_xxx``- macro has changed too.
+
+When implementing mmap_prepare(), reference flags by their bit number, defined
+as a ``VMA_xxx_BIT`` macro, e.g. ``VMA_READ_BIT``, ``VMA_WRITE_BIT`` etc.,
+and use one of (where ``desc`` is a pointer to struct vm_area_desc):
+
+* ``vma_desc_test_any(desc, ...)`` - Specify a comma-separated list of flags
+ you wish to test for (whether _any_ are set), e.g. - ``vma_desc_test_any(
+ desc, VMA_WRITE_BIT, VMA_MAYWRITE_BIT)`` - returns ``true`` if either are set,
+ otherwise ``false``.
+* ``vma_desc_set_flags(desc, ...)`` - Update the VMA descriptor flags to set
+ additional flags specified by a comma-separated list,
+ e.g. - ``vma_desc_set_flags(desc, VMA_PFNMAP_BIT, VMA_IO_BIT)``.
+* ``vma_desc_clear_flags(desc, ...)`` - Update the VMA descriptor flags to clear
+ flags specified by a comma-separated list, e.g. - ``vma_desc_clear_flags(
+ desc, VMA_WRITE_BIT, VMA_MAYWRITE_BIT)``.
+
+Actions
+=======
+
+You can now very easily have actions be performed upon a mapping once set up by
+utilising simple helper functions invoked upon the struct vm_area_desc
+pointer. These are:
+
+* mmap_action_remap() - Remaps a range consisting only of PFNs for a specific
+ range starting a virtual address and PFN number of a set size.
+
+* mmap_action_remap_full() - Same as mmap_action_remap(), only remaps the
+ entire mapping from ``start_pfn`` onward.
+
+* mmap_action_ioremap() - Same as mmap_action_remap(), only performs an I/O
+ remap.
+
+* mmap_action_ioremap_full() - Same as mmap_action_ioremap(), only remaps
+ the entire mapping from ``start_pfn`` onward.
+
+**NOTE:** The ``action`` field should never normally be manipulated directly,
+rather you ought to use one of these helpers.
--
2.53.0
^ permalink raw reply related
* [PATCH v4 03/21] mm: document vm_operations_struct->open the same as close()
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Describe when the operation is invoked and the context in which it is
invoked, matching the description already added for vm_op->close().
While we're here, update all outdated references to an 'area' field for
VMAs to the more consistent 'vma'.
Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
include/linux/mm.h | 15 ++++++++++-----
tools/testing/vma/include/dup.h | 15 ++++++++++-----
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1e63b3a44a47..da94edb287cd 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -766,15 +766,20 @@ struct vm_uffd_ops;
* to the functions called when a no-page or a wp-page exception occurs.
*/
struct vm_operations_struct {
- void (*open)(struct vm_area_struct * area);
+ /**
+ * @open: Called when a VMA is remapped, split or forked. Not called
+ * upon first mapping a VMA.
+ * Context: User context. May sleep. Caller holds mmap_lock.
+ */
+ void (*open)(struct vm_area_struct *vma);
/**
* @close: Called when the VMA is being removed from the MM.
* Context: User context. May sleep. Caller holds mmap_lock.
*/
- void (*close)(struct vm_area_struct * area);
+ void (*close)(struct vm_area_struct *vma);
/* Called any time before splitting to check if it's allowed */
- int (*may_split)(struct vm_area_struct *area, unsigned long addr);
- int (*mremap)(struct vm_area_struct *area);
+ int (*may_split)(struct vm_area_struct *vma, unsigned long addr);
+ int (*mremap)(struct vm_area_struct *vma);
/*
* Called by mprotect() to make driver-specific permission
* checks before mprotect() is finalised. The VMA must not
@@ -786,7 +791,7 @@ struct vm_operations_struct {
vm_fault_t (*huge_fault)(struct vm_fault *vmf, unsigned int order);
vm_fault_t (*map_pages)(struct vm_fault *vmf,
pgoff_t start_pgoff, pgoff_t end_pgoff);
- unsigned long (*pagesize)(struct vm_area_struct * area);
+ unsigned long (*pagesize)(struct vm_area_struct *vma);
/* notification that a previously read-only page is about to become
* writable, if an error is returned it will cause a SIGBUS */
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 477a5be65dd2..26c6c3255a94 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -632,15 +632,20 @@ struct vm_area_struct {
} __randomize_layout;
struct vm_operations_struct {
- void (*open)(struct vm_area_struct * area);
+ /**
+ * @open: Called when a VMA is remapped, split or forked. Not called
+ * upon first mapping a VMA.
+ * Context: User context. May sleep. Caller holds mmap_lock.
+ */
+ void (*open)(struct vm_area_struct *vma);
/**
* @close: Called when the VMA is being removed from the MM.
* Context: User context. May sleep. Caller holds mmap_lock.
*/
- void (*close)(struct vm_area_struct * area);
+ void (*close)(struct vm_area_struct *vma);
/* Called any time before splitting to check if it's allowed */
- int (*may_split)(struct vm_area_struct *area, unsigned long addr);
- int (*mremap)(struct vm_area_struct *area);
+ int (*may_split)(struct vm_area_struct *vma, unsigned long addr);
+ int (*mremap)(struct vm_area_struct *vma);
/*
* Called by mprotect() to make driver-specific permission
* checks before mprotect() is finalised. The VMA must not
@@ -652,7 +657,7 @@ struct vm_operations_struct {
vm_fault_t (*huge_fault)(struct vm_fault *vmf, unsigned int order);
vm_fault_t (*map_pages)(struct vm_fault *vmf,
pgoff_t start_pgoff, pgoff_t end_pgoff);
- unsigned long (*pagesize)(struct vm_area_struct * area);
+ unsigned long (*pagesize)(struct vm_area_struct *vma);
/* notification that a previously read-only page is about to become
* writable, if an error is returned it will cause a SIGBUS */
--
2.53.0
^ permalink raw reply related
* [PATCH v4 04/21] mm: avoid deadlock when holding rmap on mmap_prepare error
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Commit ac0a3fc9c07d ("mm: add ability to take further action in
vm_area_desc") added the ability for drivers to instruct mm to take actions
after the .mmap_prepare callback is complete.
To make life simpler and safer, this is done before the VMA/mmap write lock
is dropped but when the VMA is completely established.
So on error, we simply munmap() the VMA.
As part of this implementation, unfortunately a horrible hack had to be
implemented to support some questionable behaviour hugetlb relies upon -
that is that the file rmap lock is held until the operation is complete.
The implementation, for convenience, did this in mmap_action_finish() so
both the VMA and mmap_prepare compatibility layer paths would have this
correctly handled.
However, it turns out there is a mistake here - the rmap lock cannot be
held on munmap, as free_pgtables() -> unlink_file_vma_batch_add() ->
unlink_file_vma_batch_process() takes the file rmap lock.
We therefore currently have a deadlock issue that might arise.
Resolve this by leaving it to callers to handle the unmap.
The compatibility layer does not support this rmap behaviour, so we simply
have it unmap on error after calling mmap_action_complete().
In the VMA implementation, we only perform the unmap after the rmap lock is
dropped.
This resolves the issue by ensuring the rmap lock is always dropped when
the unmap occurs.
Fixes: ac0a3fc9c07d ("mm: add ability to take further action in vm_area_desc")
Cc: <stable@vger.kernel.org>
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
mm/util.c | 12 +++++++-----
mm/vma.c | 13 ++++++++++---
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/mm/util.c b/mm/util.c
index 73c97a748d8e..a2cfa0d77c35 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1215,7 +1215,13 @@ int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
return err;
set_vma_from_desc(vma, &desc);
- return mmap_action_complete(vma, &desc.action);
+ err = mmap_action_complete(vma, &desc.action);
+ if (err) {
+ const size_t len = vma_pages(vma) << PAGE_SHIFT;
+
+ do_munmap(current->mm, vma->vm_start, len, NULL);
+ }
+ return err;
}
EXPORT_SYMBOL(compat_vma_mmap);
@@ -1316,10 +1322,6 @@ static int mmap_action_finish(struct vm_area_struct *vma,
* invoked if we do NOT merge, so we only clean up the VMA we created.
*/
if (err) {
- const size_t len = vma_pages(vma) << PAGE_SHIFT;
-
- do_munmap(current->mm, vma->vm_start, len, NULL);
-
if (action->error_hook) {
/* We may want to filter the error. */
err = action->error_hook(err);
diff --git a/mm/vma.c b/mm/vma.c
index ee91f2b76acf..3fc5fe4f1a7c 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2736,9 +2736,9 @@ static int call_action_complete(struct mmap_state *map,
struct mmap_action *action,
struct vm_area_struct *vma)
{
- int ret;
+ int err;
- ret = mmap_action_complete(vma, action);
+ err = mmap_action_complete(vma, action);
/* If we held the file rmap we need to release it. */
if (map->hold_file_rmap_lock) {
@@ -2746,7 +2746,14 @@ static int call_action_complete(struct mmap_state *map,
i_mmap_unlock_write(file->f_mapping);
}
- return ret;
+
+ if (err) {
+ const size_t len = vma_pages(vma) << PAGE_SHIFT;
+
+ do_munmap(current->mm, vma->vm_start, len, NULL);
+ }
+
+ return err;
}
static unsigned long __mmap_region(struct file *file, unsigned long addr,
--
2.53.0
^ permalink raw reply related
* [PATCH v4 05/21] mm: switch the rmap lock held option off in compat layer
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
In the mmap_prepare compatibility layer, we don't need to hold the rmap
lock, as we are being called from an .mmap handler.
The .mmap_prepare hook, when invoked in the VMA logic, is called prior to
the VMA being instantiated, but the completion hook is called after the VMA
is linked into the maple tree, meaning rmap walkers can reach it.
The mmap hook does not link the VMA into the tree, so this cannot happen.
Therefore it's safe to simply disable this in the mmap_prepare
compatibility layer.
Also update VMA tests code to reflect current compatibility layer state.
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
mm/util.c | 6 ++++-
tools/testing/vma/include/dup.h | 42 +++++++++++++++++----------------
2 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/mm/util.c b/mm/util.c
index a2cfa0d77c35..182f0f5cc400 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1204,6 +1204,7 @@ int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
.action.type = MMAP_NOTHING, /* Default */
};
+ struct mmap_action *action = &desc.action;
int err;
err = vfs_mmap_prepare(file, &desc);
@@ -1214,8 +1215,11 @@ int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
if (err)
return err;
+ /* being invoked from .mmmap means we don't have to enforce this. */
+ action->hide_from_rmap_until_complete = false;
+
set_vma_from_desc(vma, &desc);
- err = mmap_action_complete(vma, &desc.action);
+ err = mmap_action_complete(vma, action);
if (err) {
const size_t len = vma_pages(vma) << PAGE_SHIFT;
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 26c6c3255a94..c62d3998e922 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1256,8 +1256,17 @@ static inline void vma_set_anonymous(struct vm_area_struct *vma)
static inline void set_vma_from_desc(struct vm_area_struct *vma,
struct vm_area_desc *desc);
-static inline int __compat_vma_mmap(const struct file_operations *f_op,
- struct file *file, struct vm_area_struct *vma)
+static inline int vfs_mmap_prepare(struct file *file, struct vm_area_desc *desc)
+{
+ return file->f_op->mmap_prepare(desc);
+}
+
+static inline unsigned long vma_pages(struct vm_area_struct *vma)
+{
+ return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+}
+
+static inline int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
{
struct vm_area_desc desc = {
.mm = vma->vm_mm,
@@ -1272,9 +1281,10 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,
.action.type = MMAP_NOTHING, /* Default */
};
+ struct mmap_action *action = &desc.action;
int err;
- err = f_op->mmap_prepare(&desc);
+ err = vfs_mmap_prepare(file, &desc);
if (err)
return err;
@@ -1282,28 +1292,25 @@ static inline int __compat_vma_mmap(const struct file_operations *f_op,
if (err)
return err;
+ /* being invoked from .mmmap means we don't have to enforce this. */
+ action->hide_from_rmap_until_complete = false;
+
set_vma_from_desc(vma, &desc);
- return mmap_action_complete(vma, &desc.action);
-}
+ err = mmap_action_complete(vma, action);
+ if (err) {
+ const size_t len = vma_pages(vma) << PAGE_SHIFT;
-static inline int compat_vma_mmap(struct file *file,
- struct vm_area_struct *vma)
-{
- return __compat_vma_mmap(file->f_op, file, vma);
+ do_munmap(current->mm, vma->vm_start, len, NULL);
+ }
+ return err;
}
-
static inline void vma_iter_init(struct vma_iterator *vmi,
struct mm_struct *mm, unsigned long addr)
{
mas_init(&vmi->mas, &mm->mm_mt, addr);
}
-static inline unsigned long vma_pages(struct vm_area_struct *vma)
-{
- return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
-}
-
static inline void mmap_assert_locked(struct mm_struct *);
static inline struct vm_area_struct *find_vma_intersection(struct mm_struct *mm,
unsigned long start_addr,
@@ -1473,11 +1480,6 @@ static inline int vfs_mmap(struct file *file, struct vm_area_struct *vma)
return file->f_op->mmap(file, vma);
}
-static inline int vfs_mmap_prepare(struct file *file, struct vm_area_desc *desc)
-{
- return file->f_op->mmap_prepare(desc);
-}
-
static inline void vma_set_file(struct vm_area_struct *vma, struct file *file)
{
/* Changing an anonymous vma with this is illegal */
--
2.53.0
^ permalink raw reply related
* [PATCH v4 06/21] mm/vma: remove superfluous map->hold_file_rmap_lock
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
We don't need to reference this field, it's confusing as it duplicates
mmap_action->hide_from_rmap_until_complete, so thread the mmap_action
through to __mmap_new_vma() instead and use the same field consistently.
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
mm/vma.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/mm/vma.c b/mm/vma.c
index 3fc5fe4f1a7c..6105f54cea61 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -38,8 +38,6 @@ struct mmap_state {
/* Determine if we can check KSM flags early in mmap() logic. */
bool check_ksm_early :1;
- /* If we map new, hold the file rmap lock on mapping. */
- bool hold_file_rmap_lock :1;
/* If .mmap_prepare changed the file, we don't need to pin. */
bool file_doesnt_need_get :1;
};
@@ -2531,10 +2529,12 @@ static int __mmap_new_file_vma(struct mmap_state *map,
*
* @map: Mapping state.
* @vmap: Output pointer for the new VMA.
+ * @action: Any mmap_prepare action that is still to complete.
*
* Returns: Zero on success, or an error.
*/
-static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap)
+static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
+ struct mmap_action *action)
{
struct vma_iterator *vmi = map->vmi;
int error = 0;
@@ -2583,7 +2583,7 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap)
vma_start_write(vma);
vma_iter_store_new(vmi, vma);
map->mm->map_count++;
- vma_link_file(vma, map->hold_file_rmap_lock);
+ vma_link_file(vma, action->hide_from_rmap_until_complete);
/*
* vma_merge_new_range() calls khugepaged_enter_vma() too, the below
@@ -2650,8 +2650,6 @@ static int call_action_prepare(struct mmap_state *map,
if (err)
return err;
- if (desc->action.hide_from_rmap_until_complete)
- map->hold_file_rmap_lock = true;
return 0;
}
@@ -2741,7 +2739,7 @@ static int call_action_complete(struct mmap_state *map,
err = mmap_action_complete(vma, action);
/* If we held the file rmap we need to release it. */
- if (map->hold_file_rmap_lock) {
+ if (action->hide_from_rmap_until_complete) {
struct file *file = vma->vm_file;
i_mmap_unlock_write(file->f_mapping);
@@ -2795,7 +2793,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
/* ...but if we can't, allocate a new VMA. */
if (!vma) {
- error = __mmap_new_vma(&map, &vma);
+ error = __mmap_new_vma(&map, &vma, &desc.action);
if (error)
goto unacct_error;
allocated_new = true;
--
2.53.0
^ permalink raw reply related
* [PATCH v4 07/21] mm: have mmap_action_complete() handle the rmap lock and unmap
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Rather than have the callers handle this both the rmap lock release and
unmapping the VMA on error, handle it within the mmap_action_complete()
logic where it makes sense to, being careful not to unlock twice.
This simplifies the logic and makes it harder to make mistake with this,
while retaining correct behaviour with regard to avoiding deadlocks.
Also replace the call_action_complete() function with a direct invocation
of mmap_action_complete() as the abstraction is no longer required.
Also update the VMA tests to reflect this change.
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
mm/internal.h | 19 +++++++++++++++
mm/util.c | 41 +++++++++++++++------------------
mm/vma.c | 26 +--------------------
tools/testing/vma/include/dup.h | 8 +------
4 files changed, 40 insertions(+), 54 deletions(-)
diff --git a/mm/internal.h b/mm/internal.h
index 0256ca44115a..760fbff9c430 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1817,6 +1817,25 @@ static inline int io_remap_pfn_range_prepare(struct vm_area_desc *desc)
return 0;
}
+/*
+ * When we succeed an mmap action or just before we unmap a VMA on error, we
+ * need to ensure any rmap lock held is released. On unmap it's required to
+ * avoid a deadlock.
+ */
+static inline void maybe_rmap_unlock_action(struct vm_area_struct *vma,
+ struct mmap_action *action)
+{
+ struct file *file;
+
+ if (!action->hide_from_rmap_until_complete)
+ return;
+
+ VM_WARN_ON_ONCE(vma_is_anonymous(vma));
+ file = vma->vm_file;
+ i_mmap_unlock_write(file->f_mapping);
+ action->hide_from_rmap_until_complete = false;
+}
+
#ifdef CONFIG_MMU_NOTIFIER
static inline int clear_flush_young_ptes_notify(struct vm_area_struct *vma,
unsigned long addr, pte_t *ptep, unsigned int nr)
diff --git a/mm/util.c b/mm/util.c
index 182f0f5cc400..a187f3ab4437 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1219,13 +1219,7 @@ int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
action->hide_from_rmap_until_complete = false;
set_vma_from_desc(vma, &desc);
- err = mmap_action_complete(vma, action);
- if (err) {
- const size_t len = vma_pages(vma) << PAGE_SHIFT;
-
- do_munmap(current->mm, vma->vm_start, len, NULL);
- }
- return err;
+ return mmap_action_complete(vma, action);
}
EXPORT_SYMBOL(compat_vma_mmap);
@@ -1320,26 +1314,30 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
static int mmap_action_finish(struct vm_area_struct *vma,
struct mmap_action *action, int err)
{
+ size_t len;
+
+ if (!err && action->success_hook)
+ err = action->success_hook(vma);
+
+ /* do_munmap() might take rmap lock, so release if held. */
+ maybe_rmap_unlock_action(vma, action);
+ if (!err)
+ return 0;
+
/*
* If an error occurs, unmap the VMA altogether and return an error. We
* only clear the newly allocated VMA, since this function is only
* invoked if we do NOT merge, so we only clean up the VMA we created.
*/
- if (err) {
- if (action->error_hook) {
- /* We may want to filter the error. */
- err = action->error_hook(err);
-
- /* The caller should not clear the error. */
- VM_WARN_ON_ONCE(!err);
- }
- return err;
+ len = vma_pages(vma) << PAGE_SHIFT;
+ do_munmap(current->mm, vma->vm_start, len, NULL);
+ if (action->error_hook) {
+ /* We may want to filter the error. */
+ err = action->error_hook(err);
+ /* The caller should not clear the error. */
+ VM_WARN_ON_ONCE(!err);
}
-
- if (action->success_hook)
- return action->success_hook(vma);
-
- return 0;
+ return err;
}
#ifdef CONFIG_MMU
@@ -1377,7 +1375,6 @@ EXPORT_SYMBOL(mmap_action_prepare);
*/
int mmap_action_complete(struct vm_area_struct *vma,
struct mmap_action *action)
-
{
int err = 0;
diff --git a/mm/vma.c b/mm/vma.c
index 6105f54cea61..86f947c90442 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2730,30 +2730,6 @@ static bool can_set_ksm_flags_early(struct mmap_state *map)
return false;
}
-static int call_action_complete(struct mmap_state *map,
- struct mmap_action *action,
- struct vm_area_struct *vma)
-{
- int err;
-
- err = mmap_action_complete(vma, action);
-
- /* If we held the file rmap we need to release it. */
- if (action->hide_from_rmap_until_complete) {
- struct file *file = vma->vm_file;
-
- i_mmap_unlock_write(file->f_mapping);
- }
-
- if (err) {
- const size_t len = vma_pages(vma) << PAGE_SHIFT;
-
- do_munmap(current->mm, vma->vm_start, len, NULL);
- }
-
- return err;
-}
-
static unsigned long __mmap_region(struct file *file, unsigned long addr,
unsigned long len, vma_flags_t vma_flags,
unsigned long pgoff, struct list_head *uf)
@@ -2805,7 +2781,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
__mmap_complete(&map, vma);
if (have_mmap_prepare && allocated_new) {
- error = call_action_complete(&map, &desc.action, vma);
+ error = mmap_action_complete(vma, &desc.action);
if (error)
return error;
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index c62d3998e922..bf8cffe1ea58 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1296,13 +1296,7 @@ static inline int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
action->hide_from_rmap_until_complete = false;
set_vma_from_desc(vma, &desc);
- err = mmap_action_complete(vma, action);
- if (err) {
- const size_t len = vma_pages(vma) << PAGE_SHIFT;
-
- do_munmap(current->mm, vma->vm_start, len, NULL);
- }
- return err;
+ return mmap_action_complete(vma, action);
}
static inline void vma_iter_init(struct vma_iterator *vmi,
--
2.53.0
^ permalink raw reply related
* [PATCH v4 08/21] mm: add vm_ops->mapped hook
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Previously, when a driver needed to do something like establish a
reference count, it could do so in the mmap hook in the knowledge that the
mapping would succeed.
With the introduction of f_op->mmap_prepare this is no longer the case, as
it is invoked prior to actually establishing the mapping.
mmap_prepare is not appropriate for this kind of thing as it is called
before any merge might take place, and after which an error might occur
meaning resources could be leaked.
To take this into account, introduce a new vm_ops->mapped callback which
is invoked when the VMA is first mapped (though notably - not when it is
merged - which is correct and mirrors existing mmap/open/close behaviour).
We do better that vm_ops->open() here, as this callback can return an
error, at which point the VMA will be unmapped.
Note that vm_ops->mapped() is invoked after any mmap action is complete
(such as I/O remapping).
We intentionally do not expose the VMA at this point, exposing only the
fields that could be used, and an output parameter in case the operation
needs to update the vma->vm_private_data field.
In order to deal with stacked filesystems which invoke inner filesystem's
mmap() invocations, add __compat_vma_mapped() and invoke it on vfs_mmap()
(via compat_vma_mmap()) to ensure that the mapped callback is handled when
an mmap() caller invokes a nested filesystem's mmap_prepare() callback.
Update the mmap_prepare documentation to describe the mapped hook and make
it clear what its intended use is.
The vm_ops->mapped() call is handled by the mmap complete logic to ensure
the same code paths are handled by both the compatibility and VMA layers.
Additionally, update VMA userland test headers to reflect the change.
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
Documentation/filesystems/mmap_prepare.rst | 15 ++++
include/linux/fs.h | 9 ++-
include/linux/mm.h | 17 ++++
mm/util.c | 90 +++++++++++++++-------
mm/vma.c | 1 -
tools/testing/vma/include/dup.h | 17 ++++
6 files changed, 120 insertions(+), 29 deletions(-)
diff --git a/Documentation/filesystems/mmap_prepare.rst b/Documentation/filesystems/mmap_prepare.rst
index ae484d371861..f14b35ee11d5 100644
--- a/Documentation/filesystems/mmap_prepare.rst
+++ b/Documentation/filesystems/mmap_prepare.rst
@@ -25,6 +25,21 @@ That is - no resources should be allocated nor state updated to reflect that a
mapping has been established, as the mapping may either be merged, or fail to be
mapped after the callback is complete.
+Mapped callback
+---------------
+
+If resources need to be allocated per-mapping, or state such as a reference
+count needs to be manipulated, this should be done using the ``vm_ops->mapped``
+hook, which itself should be set by the >mmap_prepare hook.
+
+This callback is only invoked if a new mapping has been established and was not
+merged with any other, and is invoked at a point where no error may occur before
+the mapping is established.
+
+You may return an error to the callback itself, which will cause the mapping to
+become unmapped and an error returned to the mmap() caller. This is useful if
+resources need to be allocated, and that allocation might fail.
+
How To Use
==========
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a2628a12bd2b..c390f5c667e3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2059,13 +2059,20 @@ static inline bool can_mmap_file(struct file *file)
}
int compat_vma_mmap(struct file *file, struct vm_area_struct *vma);
+int __vma_check_mmap_hook(struct vm_area_struct *vma);
static inline int vfs_mmap(struct file *file, struct vm_area_struct *vma)
{
+ int err;
+
if (file->f_op->mmap_prepare)
return compat_vma_mmap(file, vma);
- return file->f_op->mmap(file, vma);
+ err = file->f_op->mmap(file, vma);
+ if (err)
+ return err;
+
+ return __vma_check_mmap_hook(vma);
}
static inline int vfs_mmap_prepare(struct file *file, struct vm_area_desc *desc)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index da94edb287cd..ad1b8c3c0cfd 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -777,6 +777,23 @@ struct vm_operations_struct {
* Context: User context. May sleep. Caller holds mmap_lock.
*/
void (*close)(struct vm_area_struct *vma);
+ /**
+ * @mapped: Called when the VMA is first mapped in the MM. Not called if
+ * the new VMA is merged with an adjacent VMA.
+ *
+ * The @vm_private_data field is an output field allowing the user to
+ * modify vma->vm_private_data as necessary.
+ *
+ * ONLY valid if set from f_op->mmap_prepare. Will result in an error if
+ * set from f_op->mmap.
+ *
+ * Returns %0 on success, or an error otherwise. On error, the VMA will
+ * be unmapped.
+ *
+ * Context: User context. May sleep. Caller holds mmap_lock.
+ */
+ int (*mapped)(unsigned long start, unsigned long end, pgoff_t pgoff,
+ const struct file *file, void **vm_private_data);
/* Called any time before splitting to check if it's allowed */
int (*may_split)(struct vm_area_struct *vma, unsigned long addr);
int (*mremap)(struct vm_area_struct *vma);
diff --git a/mm/util.c b/mm/util.c
index a187f3ab4437..df95ae41e09b 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1163,33 +1163,7 @@ void flush_dcache_folio(struct folio *folio)
EXPORT_SYMBOL(flush_dcache_folio);
#endif
-/**
- * compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an
- * existing VMA and execute any requested actions.
- * @file: The file which possesss an f_op->mmap_prepare() hook.
- * @vma: The VMA to apply the .mmap_prepare() hook to.
- *
- * Ordinarily, .mmap_prepare() is invoked directly upon mmap(). However, certain
- * stacked filesystems invoke a nested mmap hook of an underlying file.
- *
- * Until all filesystems are converted to use .mmap_prepare(), we must be
- * conservative and continue to invoke these stacked filesystems using the
- * deprecated .mmap() hook.
- *
- * However we have a problem if the underlying file system possesses an
- * .mmap_prepare() hook, as we are in a different context when we invoke the
- * .mmap() hook, already having a VMA to deal with.
- *
- * compat_vma_mmap() is a compatibility function that takes VMA state,
- * establishes a struct vm_area_desc descriptor, passes to the underlying
- * .mmap_prepare() hook and applies any changes performed by it.
- *
- * Once the conversion of filesystems is complete this function will no longer
- * be required and will be removed.
- *
- * Returns: 0 on success or error.
- */
-int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
+static int __compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
{
struct vm_area_desc desc = {
.mm = vma->vm_mm,
@@ -1221,8 +1195,49 @@ int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
set_vma_from_desc(vma, &desc);
return mmap_action_complete(vma, action);
}
+
+/**
+ * compat_vma_mmap() - Apply the file's .mmap_prepare() hook to an
+ * existing VMA and execute any requested actions.
+ * @file: The file which possesss an f_op->mmap_prepare() hook.
+ * @vma: The VMA to apply the .mmap_prepare() hook to.
+ *
+ * Ordinarily, .mmap_prepare() is invoked directly upon mmap(). However, certain
+ * stacked filesystems invoke a nested mmap hook of an underlying file.
+ *
+ * Until all filesystems are converted to use .mmap_prepare(), we must be
+ * conservative and continue to invoke these stacked filesystems using the
+ * deprecated .mmap() hook.
+ *
+ * However we have a problem if the underlying file system possesses an
+ * .mmap_prepare() hook, as we are in a different context when we invoke the
+ * .mmap() hook, already having a VMA to deal with.
+ *
+ * compat_vma_mmap() is a compatibility function that takes VMA state,
+ * establishes a struct vm_area_desc descriptor, passes to the underlying
+ * .mmap_prepare() hook and applies any changes performed by it.
+ *
+ * Once the conversion of filesystems is complete this function will no longer
+ * be required and will be removed.
+ *
+ * Returns: 0 on success or error.
+ */
+int compat_vma_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ return __compat_vma_mmap(file, vma);
+}
EXPORT_SYMBOL(compat_vma_mmap);
+int __vma_check_mmap_hook(struct vm_area_struct *vma)
+{
+ /* vm_ops->mapped is not valid if mmap() is specified. */
+ if (vma->vm_ops && WARN_ON_ONCE(vma->vm_ops->mapped))
+ return -EINVAL;
+
+ return 0;
+}
+EXPORT_SYMBOL(__vma_check_mmap_hook);
+
static void set_ps_flags(struct page_snapshot *ps, const struct folio *folio,
const struct page *page)
{
@@ -1311,11 +1326,32 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
}
}
+static int call_vma_mapped(struct vm_area_struct *vma)
+{
+ const struct vm_operations_struct *vm_ops = vma->vm_ops;
+ void *vm_private_data = vma->vm_private_data;
+ int err;
+
+ if (!vm_ops || !vm_ops->mapped)
+ return 0;
+
+ err = vm_ops->mapped(vma->vm_start, vma->vm_end, vma->vm_pgoff,
+ vma->vm_file, &vm_private_data);
+ if (err)
+ return err;
+
+ if (vm_private_data != vma->vm_private_data)
+ vma->vm_private_data = vm_private_data;
+ return 0;
+}
+
static int mmap_action_finish(struct vm_area_struct *vma,
struct mmap_action *action, int err)
{
size_t len;
+ if (!err)
+ err = call_vma_mapped(vma);
if (!err && action->success_hook)
err = action->success_hook(vma);
diff --git a/mm/vma.c b/mm/vma.c
index 86f947c90442..2d631db272e6 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2782,7 +2782,6 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
if (have_mmap_prepare && allocated_new) {
error = mmap_action_complete(vma, &desc.action);
-
if (error)
return error;
}
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index bf8cffe1ea58..b4b12fc742c1 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -643,6 +643,23 @@ struct vm_operations_struct {
* Context: User context. May sleep. Caller holds mmap_lock.
*/
void (*close)(struct vm_area_struct *vma);
+ /**
+ * @mapped: Called when the VMA is first mapped in the MM. Not called if
+ * the new VMA is merged with an adjacent VMA.
+ *
+ * The @vm_private_data field is an output field allowing the user to
+ * modify vma->vm_private_data as necessary.
+ *
+ * ONLY valid if set from f_op->mmap_prepare. Will result in an error if
+ * set from f_op->mmap.
+ *
+ * Returns %0 on success, or an error otherwise. On error, the VMA will
+ * be unmapped.
+ *
+ * Context: User context. May sleep. Caller holds mmap_lock.
+ */
+ int (*mapped)(unsigned long start, unsigned long end, pgoff_t pgoff,
+ const struct file *file, void **vm_private_data);
/* Called any time before splitting to check if it's allowed */
int (*may_split)(struct vm_area_struct *vma, unsigned long addr);
int (*mremap)(struct vm_area_struct *vma);
--
2.53.0
^ permalink raw reply related
* [PATCH v4 09/21] fs: afs: revert mmap_prepare() change
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Partially reverts commit 9d5403b1036c ("fs: convert most other
generic_file_*mmap() users to .mmap_prepare()").
This is because the .mmap invocation establishes a refcount, but
.mmap_prepare is called at a point where a merge or an allocation failure
might happen after the call, which would leak the refcount increment.
Functionality is being added to permit the use of .mmap_prepare in this
case, but in the interim, we need to fix this.
Fixes: 9d5403b1036c ("fs: convert most other generic_file_*mmap() users to .mmap_prepare()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
fs/afs/file.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/afs/file.c b/fs/afs/file.c
index f609366fd2ac..74d04af51ff4 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -19,7 +19,7 @@
#include <trace/events/netfs.h>
#include "internal.h"
-static int afs_file_mmap_prepare(struct vm_area_desc *desc);
+static int afs_file_mmap(struct file *file, struct vm_area_struct *vma);
static ssize_t afs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter);
static ssize_t afs_file_splice_read(struct file *in, loff_t *ppos,
@@ -35,7 +35,7 @@ const struct file_operations afs_file_operations = {
.llseek = generic_file_llseek,
.read_iter = afs_file_read_iter,
.write_iter = netfs_file_write_iter,
- .mmap_prepare = afs_file_mmap_prepare,
+ .mmap = afs_file_mmap,
.splice_read = afs_file_splice_read,
.splice_write = iter_file_splice_write,
.fsync = afs_fsync,
@@ -492,16 +492,16 @@ static void afs_drop_open_mmap(struct afs_vnode *vnode)
/*
* Handle setting up a memory mapping on an AFS file.
*/
-static int afs_file_mmap_prepare(struct vm_area_desc *desc)
+static int afs_file_mmap(struct file *file, struct vm_area_struct *vma)
{
- struct afs_vnode *vnode = AFS_FS_I(file_inode(desc->file));
+ struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
int ret;
afs_add_open_mmap(vnode);
- ret = generic_file_mmap_prepare(desc);
+ ret = generic_file_mmap(file, vma);
if (ret == 0)
- desc->vm_ops = &afs_vm_ops;
+ vma->vm_ops = &afs_vm_ops;
else
afs_drop_open_mmap(vnode);
return ret;
--
2.53.0
^ permalink raw reply related
* [PATCH v4 10/21] fs: afs: restore mmap_prepare implementation
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Commit 9d5403b1036c ("fs: convert most other generic_file_*mmap() users to
.mmap_prepare()") updated AFS to use the mmap_prepare callback in favour
of the deprecated mmap callback.
However, it did not account for the fact that mmap_prepare is called
pre-merge, and may then be merged, nor that mmap_prepare can fail to map
due to an out of memory error.
This change was therefore since reverted.
Both of those are cases in which we should not be incrementing a reference
count.
With the newly added vm_ops->mapped callback available, we can simply
defer this operation to that callback which is only invoked once the
mapping is successfully in place (but not yet visible to userspace as the
mmap and VMA write locks are held).
This allows us to once again reimplement the .mmap_prepare implementation
for this file system.
Therefore add afs_mapped() to implement this callback for AFS, and remove
the code doing so in afs_mmap_prepare().
Also update afs_vm_open(), afs_vm_close() and afs_vm_map_pages() to be
consistent in how the vnode is accessed.
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
fs/afs/file.c | 42 +++++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/fs/afs/file.c b/fs/afs/file.c
index 74d04af51ff4..85696ac984cc 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -19,7 +19,7 @@
#include <trace/events/netfs.h>
#include "internal.h"
-static int afs_file_mmap(struct file *file, struct vm_area_struct *vma);
+static int afs_file_mmap_prepare(struct vm_area_desc *desc);
static ssize_t afs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter);
static ssize_t afs_file_splice_read(struct file *in, loff_t *ppos,
@@ -28,6 +28,8 @@ static ssize_t afs_file_splice_read(struct file *in, loff_t *ppos,
static void afs_vm_open(struct vm_area_struct *area);
static void afs_vm_close(struct vm_area_struct *area);
static vm_fault_t afs_vm_map_pages(struct vm_fault *vmf, pgoff_t start_pgoff, pgoff_t end_pgoff);
+static int afs_mapped(unsigned long start, unsigned long end, pgoff_t pgoff,
+ const struct file *file, void **vm_private_data);
const struct file_operations afs_file_operations = {
.open = afs_open,
@@ -35,7 +37,7 @@ const struct file_operations afs_file_operations = {
.llseek = generic_file_llseek,
.read_iter = afs_file_read_iter,
.write_iter = netfs_file_write_iter,
- .mmap = afs_file_mmap,
+ .mmap_prepare = afs_file_mmap_prepare,
.splice_read = afs_file_splice_read,
.splice_write = iter_file_splice_write,
.fsync = afs_fsync,
@@ -61,6 +63,7 @@ const struct address_space_operations afs_file_aops = {
};
static const struct vm_operations_struct afs_vm_ops = {
+ .mapped = afs_mapped,
.open = afs_vm_open,
.close = afs_vm_close,
.fault = filemap_fault,
@@ -492,34 +495,47 @@ static void afs_drop_open_mmap(struct afs_vnode *vnode)
/*
* Handle setting up a memory mapping on an AFS file.
*/
-static int afs_file_mmap(struct file *file, struct vm_area_struct *vma)
+static int afs_file_mmap_prepare(struct vm_area_desc *desc)
{
- struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
int ret;
- afs_add_open_mmap(vnode);
+ ret = generic_file_mmap_prepare(desc);
+ if (ret)
+ return ret;
- ret = generic_file_mmap(file, vma);
- if (ret == 0)
- vma->vm_ops = &afs_vm_ops;
- else
- afs_drop_open_mmap(vnode);
+ desc->vm_ops = &afs_vm_ops;
return ret;
}
+static int afs_mapped(unsigned long start, unsigned long end, pgoff_t pgoff,
+ const struct file *file, void **vm_private_data)
+{
+ struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
+
+ afs_add_open_mmap(vnode);
+ return 0;
+}
+
static void afs_vm_open(struct vm_area_struct *vma)
{
- afs_add_open_mmap(AFS_FS_I(file_inode(vma->vm_file)));
+ struct file *file = vma->vm_file;
+ struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
+
+ afs_add_open_mmap(vnode);
}
static void afs_vm_close(struct vm_area_struct *vma)
{
- afs_drop_open_mmap(AFS_FS_I(file_inode(vma->vm_file)));
+ struct file *file = vma->vm_file;
+ struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
+
+ afs_drop_open_mmap(vnode);
}
static vm_fault_t afs_vm_map_pages(struct vm_fault *vmf, pgoff_t start_pgoff, pgoff_t end_pgoff)
{
- struct afs_vnode *vnode = AFS_FS_I(file_inode(vmf->vma->vm_file));
+ struct file *file = vmf->vma->vm_file;
+ struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
if (afs_check_validity(vnode))
return filemap_map_pages(vmf, start_pgoff, end_pgoff);
--
2.53.0
^ permalink raw reply related
* [PATCH v4 11/21] mm: add mmap_action_simple_ioremap()
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
Currently drivers use vm_iomap_memory() as a simple helper function for
I/O remapping memory over a range starting at a specified physical address
over a specified length.
In order to utilise this from mmap_prepare, separate out the core logic
into __simple_ioremap_prep(), update vm_iomap_memory() to use it, and add
simple_ioremap_prepare() to do the same with a VMA descriptor object.
We also add MMAP_SIMPLE_IO_REMAP and relevant fields to the struct
mmap_action type to permit this operation also.
We use mmap_action_ioremap() to set up the actual I/O remap operation once
we have checked and figured out the parameters, which makes
simple_ioremap_prepare() easy to implement.
We then add mmap_action_simple_ioremap() to allow drivers to make use of
this mode.
We update the mmap_prepare documentation to describe this mode. Finally,
we update the VMA tests to reflect this change.
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
Documentation/filesystems/mmap_prepare.rst | 3 +
include/linux/mm.h | 24 +++++-
include/linux/mm_types.h | 6 +-
mm/internal.h | 1 +
mm/memory.c | 85 +++++++++++++++-------
mm/util.c | 5 ++
tools/testing/vma/include/dup.h | 6 +-
7 files changed, 102 insertions(+), 28 deletions(-)
diff --git a/Documentation/filesystems/mmap_prepare.rst b/Documentation/filesystems/mmap_prepare.rst
index f14b35ee11d5..14bb057be564 100644
--- a/Documentation/filesystems/mmap_prepare.rst
+++ b/Documentation/filesystems/mmap_prepare.rst
@@ -153,5 +153,8 @@ pointer. These are:
* mmap_action_ioremap_full() - Same as mmap_action_ioremap(), only remaps
the entire mapping from ``start_pfn`` onward.
+* mmap_action_simple_ioremap() - Sets up an I/O remap from a specified
+ physical address and over a specified length.
+
**NOTE:** The ``action`` field should never normally be manipulated directly,
rather you ought to use one of these helpers.
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ad1b8c3c0cfd..df8fa6e6402b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4337,11 +4337,33 @@ static inline void mmap_action_ioremap(struct vm_area_desc *desc,
* @start_pfn: The first PFN in the range to remap.
*/
static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,
- unsigned long start_pfn)
+ unsigned long start_pfn)
{
mmap_action_ioremap(desc, desc->start, start_pfn, vma_desc_size(desc));
}
+/**
+ * mmap_action_simple_ioremap - helper for mmap_prepare hook to specify that the
+ * physical range in [start_phys_addr, start_phys_addr + size) should be I/O
+ * remapped.
+ * @desc: The VMA descriptor for the VMA requiring remap.
+ * @start_phys_addr: Start of the physical memory to be mapped.
+ * @size: Size of the area to map.
+ *
+ * NOTE: Some drivers might want to tweak desc->page_prot for purposes of
+ * write-combine or similar.
+ */
+static inline void mmap_action_simple_ioremap(struct vm_area_desc *desc,
+ phys_addr_t start_phys_addr,
+ unsigned long size)
+{
+ struct mmap_action *action = &desc->action;
+
+ action->simple_ioremap.start_phys_addr = start_phys_addr;
+ action->simple_ioremap.size = size;
+ action->type = MMAP_SIMPLE_IO_REMAP;
+}
+
int mmap_action_prepare(struct vm_area_desc *desc);
int mmap_action_complete(struct vm_area_struct *vma,
struct mmap_action *action);
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 16d31045e26e..174286f9ecf0 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -814,6 +814,7 @@ enum mmap_action_type {
MMAP_NOTHING, /* Mapping is complete, no further action. */
MMAP_REMAP_PFN, /* Remap PFN range. */
MMAP_IO_REMAP_PFN, /* I/O remap PFN range. */
+ MMAP_SIMPLE_IO_REMAP, /* I/O remap with guardrails. */
};
/*
@@ -822,13 +823,16 @@ enum mmap_action_type {
*/
struct mmap_action {
union {
- /* Remap range. */
struct {
unsigned long start;
unsigned long start_pfn;
unsigned long size;
pgprot_t pgprot;
} remap;
+ struct {
+ phys_addr_t start_phys_addr;
+ unsigned long size;
+ } simple_ioremap;
};
enum mmap_action_type type;
diff --git a/mm/internal.h b/mm/internal.h
index 760fbff9c430..46cc1a695a0e 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1796,6 +1796,7 @@ int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm);
int remap_pfn_range_prepare(struct vm_area_desc *desc);
int remap_pfn_range_complete(struct vm_area_struct *vma,
struct mmap_action *action);
+int simple_ioremap_prepare(struct vm_area_desc *desc);
static inline int io_remap_pfn_range_prepare(struct vm_area_desc *desc)
{
diff --git a/mm/memory.c b/mm/memory.c
index 9dec67a18116..b3bcc21af20a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3170,6 +3170,58 @@ int remap_pfn_range_complete(struct vm_area_struct *vma,
return do_remap_pfn_range(vma, start, pfn, size, prot);
}
+static int __simple_ioremap_prep(unsigned long vm_len, pgoff_t vm_pgoff,
+ phys_addr_t start_phys, unsigned long size,
+ unsigned long *pfnp)
+{
+ unsigned long pfn, pages;
+
+ /* Check that the physical memory area passed in looks valid */
+ if (start_phys + size < start_phys)
+ return -EINVAL;
+ /*
+ * You *really* shouldn't map things that aren't page-aligned,
+ * but we've historically allowed it because IO memory might
+ * just have smaller alignment.
+ */
+ size += start_phys & ~PAGE_MASK;
+ pfn = start_phys >> PAGE_SHIFT;
+ pages = (size + ~PAGE_MASK) >> PAGE_SHIFT;
+ if (pfn + pages < pfn)
+ return -EINVAL;
+
+ /* We start the mapping 'vm_pgoff' pages into the area */
+ if (vm_pgoff > pages)
+ return -EINVAL;
+ pfn += vm_pgoff;
+ pages -= vm_pgoff;
+
+ /* Can we fit all of the mapping? */
+ if ((vm_len >> PAGE_SHIFT) > pages)
+ return -EINVAL;
+
+ *pfnp = pfn;
+ return 0;
+}
+
+int simple_ioremap_prepare(struct vm_area_desc *desc)
+{
+ struct mmap_action *action = &desc->action;
+ const phys_addr_t start = action->simple_ioremap.start_phys_addr;
+ const unsigned long size = action->simple_ioremap.size;
+ unsigned long pfn;
+ int err;
+
+ err = __simple_ioremap_prep(vma_desc_size(desc), desc->pgoff,
+ start, size, &pfn);
+ if (err)
+ return err;
+
+ /* The I/O remap logic does the heavy lifting. */
+ mmap_action_ioremap_full(desc, pfn);
+ return io_remap_pfn_range_prepare(desc);
+}
+
/**
* vm_iomap_memory - remap memory to userspace
* @vma: user vma to map to
@@ -3187,32 +3239,15 @@ int remap_pfn_range_complete(struct vm_area_struct *vma,
*/
int vm_iomap_memory(struct vm_area_struct *vma, phys_addr_t start, unsigned long len)
{
- unsigned long vm_len, pfn, pages;
-
- /* Check that the physical memory area passed in looks valid */
- if (start + len < start)
- return -EINVAL;
- /*
- * You *really* shouldn't map things that aren't page-aligned,
- * but we've historically allowed it because IO memory might
- * just have smaller alignment.
- */
- len += start & ~PAGE_MASK;
- pfn = start >> PAGE_SHIFT;
- pages = (len + ~PAGE_MASK) >> PAGE_SHIFT;
- if (pfn + pages < pfn)
- return -EINVAL;
-
- /* We start the mapping 'vm_pgoff' pages into the area */
- if (vma->vm_pgoff > pages)
- return -EINVAL;
- pfn += vma->vm_pgoff;
- pages -= vma->vm_pgoff;
+ const unsigned long vm_start = vma->vm_start;
+ const unsigned long vm_end = vma->vm_end;
+ const unsigned long vm_len = vm_end - vm_start;
+ unsigned long pfn;
+ int err;
- /* Can we fit all of the mapping? */
- vm_len = vma->vm_end - vma->vm_start;
- if (vm_len >> PAGE_SHIFT > pages)
- return -EINVAL;
+ err = __simple_ioremap_prep(vm_len, vma->vm_pgoff, start, len, &pfn);
+ if (err)
+ return err;
/* Ok, let it rip */
return io_remap_pfn_range(vma, vma->vm_start, pfn, vm_len, vma->vm_page_prot);
diff --git a/mm/util.c b/mm/util.c
index df95ae41e09b..a19f062b84dc 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -1393,6 +1393,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)
return remap_pfn_range_prepare(desc);
case MMAP_IO_REMAP_PFN:
return io_remap_pfn_range_prepare(desc);
+ case MMAP_SIMPLE_IO_REMAP:
+ return simple_ioremap_prepare(desc);
}
WARN_ON_ONCE(1);
@@ -1421,6 +1423,7 @@ int mmap_action_complete(struct vm_area_struct *vma,
err = remap_pfn_range_complete(vma, action);
break;
case MMAP_IO_REMAP_PFN:
+ case MMAP_SIMPLE_IO_REMAP:
/* Should have been delegated. */
WARN_ON_ONCE(1);
err = -EINVAL;
@@ -1438,6 +1441,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)
break;
case MMAP_REMAP_PFN:
case MMAP_IO_REMAP_PFN:
+ case MMAP_SIMPLE_IO_REMAP:
WARN_ON_ONCE(1); /* nommu cannot handle these. */
break;
}
@@ -1456,6 +1460,7 @@ int mmap_action_complete(struct vm_area_struct *vma,
break;
case MMAP_REMAP_PFN:
case MMAP_IO_REMAP_PFN:
+ case MMAP_SIMPLE_IO_REMAP:
WARN_ON_ONCE(1); /* nommu cannot handle this. */
err = -EINVAL;
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index b4b12fc742c1..a0c2379bd42b 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -453,6 +453,7 @@ enum mmap_action_type {
MMAP_NOTHING, /* Mapping is complete, no further action. */
MMAP_REMAP_PFN, /* Remap PFN range. */
MMAP_IO_REMAP_PFN, /* I/O remap PFN range. */
+ MMAP_SIMPLE_IO_REMAP, /* I/O remap with guardrails. */
};
/*
@@ -461,13 +462,16 @@ enum mmap_action_type {
*/
struct mmap_action {
union {
- /* Remap range. */
struct {
unsigned long start;
unsigned long start_pfn;
unsigned long size;
pgprot_t pgprot;
} remap;
+ struct {
+ phys_addr_t start_phys_addr;
+ unsigned long size;
+ } simple_ioremap;
};
enum mmap_action_type type;
--
2.53.0
^ permalink raw reply related
* [PATCH v4 12/21] misc: open-dice: replace deprecated mmap hook with mmap_prepare
From: Lorenzo Stoakes (Oracle) @ 2026-03-20 22:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Clemens Ladisch, Arnd Bergmann,
Greg Kroah-Hartman, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Bodo Stroesser, Martin K . Petersen,
David Howells, Marc Dionne, Alexander Viro, Christian Brauner,
Jan Kara, David Hildenbrand, Liam R . Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jann Horn,
Pedro Falcato, linux-kernel, linux-doc, linux-hyperv, linux-stm32,
linux-arm-kernel, linux-mtd, linux-staging, linux-scsi,
target-devel, linux-afs, linux-fsdevel, linux-mm, Ryan Roberts
In-Reply-To: <cover.1774045440.git.ljs@kernel.org>
The f_op->mmap interface is deprecated, so update driver to use its
successor, mmap_prepare.
The driver previously used vm_iomap_memory(), so this change replaces it
with its mmap_prepare equivalent, mmap_action_simple_ioremap().
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
---
drivers/misc/open-dice.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/open-dice.c b/drivers/misc/open-dice.c
index 24c29e0f00ef..45060fb4ea27 100644
--- a/drivers/misc/open-dice.c
+++ b/drivers/misc/open-dice.c
@@ -86,29 +86,32 @@ static ssize_t open_dice_write(struct file *filp, const char __user *ptr,
/*
* Creates a mapping of the reserved memory region in user address space.
*/
-static int open_dice_mmap(struct file *filp, struct vm_area_struct *vma)
+static int open_dice_mmap_prepare(struct vm_area_desc *desc)
{
+ struct file *filp = desc->file;
struct open_dice_drvdata *drvdata = to_open_dice_drvdata(filp);
- if (vma->vm_flags & VM_MAYSHARE) {
+ if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) {
/* Do not allow userspace to modify the underlying data. */
- if (vma->vm_flags & VM_WRITE)
+ if (vma_desc_test(desc, VMA_WRITE_BIT))
return -EPERM;
/* Ensure userspace cannot acquire VM_WRITE later. */
- vm_flags_clear(vma, VM_MAYWRITE);
+ vma_desc_clear_flags(desc, VMA_MAYWRITE_BIT);
}
/* Create write-combine mapping so all clients observe a wipe. */
- vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
- vm_flags_set(vma, VM_DONTCOPY | VM_DONTDUMP);
- return vm_iomap_memory(vma, drvdata->rmem->base, drvdata->rmem->size);
+ desc->page_prot = pgprot_writecombine(desc->page_prot);
+ vma_desc_set_flags(desc, VMA_DONTCOPY_BIT, VMA_DONTDUMP_BIT);
+ mmap_action_simple_ioremap(desc, drvdata->rmem->base,
+ drvdata->rmem->size);
+ return 0;
}
static const struct file_operations open_dice_fops = {
.owner = THIS_MODULE,
.read = open_dice_read,
.write = open_dice_write,
- .mmap = open_dice_mmap,
+ .mmap_prepare = open_dice_mmap_prepare,
};
static int __init open_dice_probe(struct platform_device *pdev)
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox