Re: [PATCH RFC 08/13] mm/huge_memory: move anon_vma and filemap management into split helpers
"Zi Yan" <[email protected]>
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Fri Aug 7, 2026 at 5:17 PM EDT, Kairui Song via B4 Relay wrote: > From: Kairui Song <[email protected]> > > Only anon split needs vma info, and only file split needs the filemap > handling. Move the related code into separate helpers so they are > genuinely more self-contained. > > Signed-off-by: Kairui Song <[email protected]> > --- > mm/huge_memory.c | 177 +++++++++++++++++++++++++------------------------------ > 1 file changed, 79 insertions(+), 98 deletions(-) > <snip> > static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new_order, > - struct page *split_at, struct xa_state *xas, > - struct address_space *mapping, bool do_lru, > + struct page *split_at, bool do_lru, > struct list_head *list, enum split_type split_type) > { > + struct address_space *mapping = folio->mapping; > + XA_STATE(xas, &mapping->i_pages, folio->index); mapping should be always non-NULL now. mapping->i_pages no longer exists for anon code path. I like it. > struct folio *end_folio = folio_next(folio); > struct folio *new_folio, *next; > int nr_shmem_dropped = 0; > + unsigned int min_order; > struct lruvec *lruvec; > pgoff_t end = 0; > - int ret; > + gfp_t gfp; > + int ret = 0; > + > + min_order = mapping_min_folio_order(mapping); > + if (new_order < min_order) > + return -EINVAL; > + > + gfp = current_gfp_context(mapping_gfp_mask(mapping) & GFP_RECLAIM_MASK); > + if (!filemap_release_folio(folio, gfp)) > + return -EBUSY; > + > + mapping_set_update(&xas, mapping); > + > + if (split_type == SPLIT_TYPE_UNIFORM) { > + int old_order = folio_order(folio); > + > + xas_set_order(&xas, folio->index, new_order); > + xas_split_alloc(&xas, folio, old_order, gfp); > + if (xas_error(&xas)) { > + ret = xas_error(&xas); > + goto fail_free; > + } > + } > + > + i_mmap_lock_read(mapping); > + > + /* Racy check if we can split the page, before unmap_folio() */ > + if (folio_expected_ref_count(folio) != folio_ref_count(folio) - 1) { > + ret = -EAGAIN; > + goto fail_mmap_unlock; > + } > > /* > *__split_unmapped_folio() may need to trim off pages beyond > @@ -4060,13 +4118,13 @@ static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new > > unmap_folio(folio); > > - xas_lock_irq(xas); > + xas_lock_irq(&xas); > > /* > * Check if the folio is present in page cache. > * We assume all tail are present too, if folio is there. > */ > - if (xas_load(xas) != folio) { > + if (xas_load(&xas) != folio) { > ret = -EAGAIN; > goto fail; > } > @@ -4093,7 +4151,7 @@ static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new > if (do_lru) > lruvec = folio_lruvec_lock(folio); > > - ret = __split_unmapped_folio(folio, new_order, split_at, xas, > + ret = __split_unmapped_folio(folio, new_order, split_at, &xas, > mapping, split_type); > > /* > @@ -4145,9 +4203,19 @@ static int __folio_freeze_split_unmap_file(struct folio *folio, unsigned int new > lruvec_unlock(lruvec); > > fail: > - xas_unlock_irq(xas); > + xas_unlock_irq(&xas); > +fail_mmap_unlock: > if (nr_shmem_dropped) > shmem_uncharge(mapping->host, nr_shmem_dropped); > + /* > + * Drop the mapping while the inode is still pinned. @folio stays > + * locked and present in the page cache, so eviction cannot free > + * the inode yet, nothing past this point may touch the inode or > + * the mapping. > + */ > + i_mmap_unlock_read(mapping); > +fail_free: > + xas_destroy(&xas); > return ret; > } > > @@ -4176,12 +4244,9 @@ static int __folio_split(struct folio *folio, unsigned int new_order, > struct page *split_at, struct page *lock_at, > struct list_head *list, enum split_type split_type) > { > - XA_STATE(xas, &folio->mapping->i_pages, folio->index); > struct folio *end_folio = folio_next(folio); > bool is_anon = folio_test_anon(folio); > struct mem_cgroup *memcg, *old_memcg; > - struct address_space *mapping = NULL; > - struct anon_vma *anon_vma = NULL; > int old_order = folio_order(folio); > struct folio *new_folio, *next; > int ret; > @@ -4212,84 +4277,12 @@ static int __folio_split(struct folio *folio, unsigned int new_order, > memcg = get_mem_cgroup_from_folio(folio); > old_memcg = set_active_memcg(memcg); > > - if (is_anon) { > - /* > - * The caller does not necessarily hold an mmap_lock that would > - * prevent the anon_vma disappearing so we first we take a > - * reference to it and then lock the anon_vma for write. This > - * is similar to folio_lock_anon_vma_read except the write lock > - * is taken to serialise against parallel split or collapse > - * operations. > - */ > - anon_vma = folio_get_anon_vma(folio); > - if (!anon_vma) { > - ret = -EBUSY; > - goto out; > - } > - anon_vma_lock_write(anon_vma); > - mapping = NULL; > - } else { > - unsigned int min_order; > - gfp_t gfp; > - > - mapping = folio->mapping; > - min_order = mapping_min_folio_order(mapping); > - if (new_order < min_order) { > - ret = -EINVAL; > - goto out; > - } > - > - gfp = current_gfp_context(mapping_gfp_mask(mapping) & > - GFP_RECLAIM_MASK); > - > - if (!filemap_release_folio(folio, gfp)) { > - ret = -EBUSY; > - goto out; > - } > - > - mapping_set_update(&xas, mapping); > - > - if (split_type == SPLIT_TYPE_UNIFORM) { > - xas_set_order(&xas, folio->index, new_order); > - xas_split_alloc(&xas, folio, old_order, gfp); > - if (xas_error(&xas)) { > - ret = xas_error(&xas); > - goto out; > - } > - } > - > - anon_vma = NULL; > - i_mmap_lock_read(mapping); > - } > - > - /* > - * Racy check if we can split the page, before unmap_folio() will > - * split PMDs > - */ > - if (folio_expected_ref_count(folio) != folio_ref_count(folio) - 1) { > - ret = -EAGAIN; > - goto out_unlock; > - } > - > - if (!is_anon) { > - ret = __folio_freeze_split_unmap_file(folio, new_order, split_at, &xas, mapping, > - true, list, split_type); > - } else { > + if (is_anon) > ret = __folio_freeze_split_unmap_anon(folio, new_order, split_at, true, > true, list, split_type); > - } > - > - /* > - * Drop the mapping while the inode is still pinned. @folio stays > - * locked and present in the page cache until the loop below, so > - * eviction cannot free the inode yet; @lock_at is not enough, it may > - * be a tail beyond EOF that the split already dropped from the page > - * cache. Nothing past this point may touch the inode or the mapping. > - */ > - if (mapping) { > - i_mmap_unlock_read(mapping); > - mapping = NULL; > - } > + else > + ret = __folio_freeze_split_unmap_file(folio, new_order, split_at, > + true, list, split_type); > > /* > * Unlock all after-split folios except the one containing > @@ -4310,19 +4303,10 @@ static int __folio_split(struct folio *folio, unsigned int new_order, > free_folio_and_swap_cache(new_folio); > } > > -out_unlock: > - if (anon_vma) { > - anon_vma_unlock_write(anon_vma); > - put_anon_vma(anon_vma); > - } > - if (mapping) > - i_mmap_unlock_read(mapping); > -out: > /* restore to caller's old_memcg */ > set_active_memcg(old_memcg); > mem_cgroup_put(memcg); > out_no_memcg: > - xas_destroy(&xas); > if (is_pmd_order(old_order)) > count_vm_event(!ret ? THP_SPLIT_PAGE : THP_SPLIT_PAGE_FAILED); > count_mthp_stat(old_order, !ret ? MTHP_STAT_SPLIT : MTHP_STAT_SPLIT_FAILED); > @@ -4358,9 +4342,6 @@ int folio_split_unmapped(struct folio *folio, unsigned int new_order) > VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio); > VM_WARN_ON_ONCE_FOLIO(!folio_test_anon(folio), folio); > > - if (folio_expected_ref_count(folio) != folio_ref_count(folio) - 1) > - return -EAGAIN; > - > return __folio_freeze_split_unmap_anon(folio, new_order, &folio->page, false, > false, NULL, SPLIT_TYPE_UNIFORM); > } __folio_split() looks much cleaner. Thanks. Reviewed-by: Zi Yan <[email protected]> -- Best Regards, Yan, Zi