mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff
Subsystems (33)
Lorenzo Stoakes (ARM)
maintainerv5 cover mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff
In memory management we've managed to manufacture a great deal of confusion around the concept of anonymous memory. We have:
1. 'Pure anon' memory - anonymous VMAs whose folios are anonymous and swap-backed (thus for reclaim purposes, treated as anonymous). These are simple enough.
2. shmem - file-backed VMAs, file-backed folios (from rmap perspective) so present in the page cache and mapped by an address_space object, but whose folios are also swap-backed (thus treated as anonymous for reclaim purposes).
3. MAP_PRIVATE-mapped /dev/zero - a strange beast whose VMAs have vma->vm_file set, but which clears vma->vm_ops to satisfy vma_is_anonymous(), resulting in VMAs that were mmap()'d referencing a file, but are in every other sense anonymous, including the folios.
4. Other MAP_PRIVATE-file backed mappings - These possess file-backed VMAs and have file-backed folios until CoW'd, at which point those CoW'd folios are anonymous.
This series fixes issue 3.
In order for us to traverse VMAs using the reverse mapping, we require two fields - folio->mapping and folio->index. The first tells the rmap code where to look for VMAs, and the second tells it at which offset the folio starts within the referenced object.
For anonymous folios, folio->mapping points at an anon_vma object. For file-backed folios, it points at an address_space. And:
* For file-backed folios folio->index is simply the page offset of the start of the folio within the file.
* For anonymous folios belonging to pure anon mappings, folio->index is equal to the anonymous page offset of the folio.
* For anonymous folios belonging to file-backed mappings (i.e. CoW'd folios of a MAP_PRIVATE file-backed mapping), folio->index is equal to the file page offset.
This series establishes a new anonymous page offset property of VMAs to allow us to map anonymous folios at their anonymous page offset, consistent with pure anon.
The purpose of doing so is to lay the foundations for the scalable CoW work. This is necessary because scalable CoW looks in the maple tree for the VMA located at folio->index << PAGE_SHIFT, before falling back to looking up tracked remaps if necessary.
The MAP_PRIVATE file-backed case means that folio indices will very often conflict with one another and this remap tracking becomes substantially more contended, and of course the fast path can never be used.
This also makes it possible, in future, to unshare anonymously mapped folios with deep fork hierarchies on remap, eliminating the need for remap tracking in the vast majority of cases.
Similar to page offset of pure anonymous VMAs, we update the anonymous page offset of unfaulted file-backed VMAs on remap, but do not once CoW'd (i.e. vma->anon_vma is non-NULL).
Overall, there is little impact on mergeability, which remains exactly the same for pure anonymous and shared file-backed mappings, with the only impact being on MAP_PRIVATE-mapped file-backed mappings, which must now match on anonymous page offset as well as file page offset to be merged.
To fail to merge like this would require CoW'ing the mapping, then finding another VMA with identical file and compatible page offset to remap next to.
This is therefore very much an edge case that should have very little impact (and which scalable CoW may very well address in any case).
---
v5:
* Accumulated tags (thanks everybody!)
* Removed the final 4 patches to be handled later as there are nuances with the /dev/zero stuff we need to figure out, as discussed with David.
* Updated the cover letter to reflect this.
* Added comments to vma_flags_is_cow_mapping(), vma_[desc_]is_cow_mapping() as per Suren.
* Correct typo as per Suren.
* Reworded test comment in patch 16 from 'fault in' to 'trigger a CoW fault' as per David.
* Fix -> 75 char limit in patch 12's commit msg.
v4:
- Updated tags (thanks everyone!)
- Adjusted some prose as per David.
- Adjusted whitespace to 2 tabs in 2/15 as per David.
- Improved linear_page_index(), __linear_anon_page_index() to be more succinct in 2/15 as per David and updated commit message to reflect it.
- Added commit to provide vma_[flags_]is_cow_mapping() - nearly all callers are calling is_cow_mapping() as is_cow_mapping(vma->vm_flags) so just provide a helper to do this for them.
- In the same commit update all is_cow_mapping() callers and remove the now-unused function.
- Change linear_anon_page_index() to assert on !CoW mapping using vma_is_cow_mapping() in 2/15 as per David.
- Updated 4/15 to output index differently depending on whether the VMA is a CoW mapping or not or whether the file index differs from the anon index as per Gregory.
- Updated 6/15 to change the 'update page offset' logic in copy_vma() to not be gated on CoW or VMA_SHARED_BIT as discussed with David.
- Updated 6/15 to improve the 'faulted in anon vma' assert stuff. It was very unclear so rename the variable sensibly and update the comment.
- Added a separate commit to fix the mess that is the faulted_in_anon_vma and the VM_WARN_ON_ONCE_VMA() assert in copy_vma() - make it actually only update the vmap in cases where the VMA was replaced (backwards remap), update the checks to reflect this in a way that's actually understandable and improve the comment.
- Updated needs_adjacent_anon_gpoff() in 9/15 to use vma_flags_is_cow_mapping() as per David. Updated the comment to reflect it.
- Broke out changes to vma_address_end() into a separate patch as per David.
- Eliminated pgoff in vma_address_end() as it adds confusion - just use pgoff_end, which was what pgoff used to be (confusingly).
- Placed more variables in vma_address_end() as constants at the start of the function.
- Dropped KSM comment in 9/15 as per David.
- Dropped linear_folio_page_index() patch altogether as per David.
- Realised all off uffd is only anon so use linear_anon_page_index() throughout there and also the huge memory case for the same reason.
- Added a patch to make remove_migration_pmd() accept a folio instead of a page.
- Added a patch to calculate large folio index using PFN as suggested by David, eliminating the need for linear page index lookup at all.
- Added a patch to add self merge VMA userland tests.
https://patch.msgid.link/[email protected]
v3:
- Renamed virtual page offset to anonymous page offset across the series as per David and adjusted prose to reflect it.
- As part of the rename, eliminated vma_anon_pgoff_addr() and the conflicting vma_[start, end]_anon_pgoff() functions by passing anon pgoff to the merge logic and having that figure out which page offset to use, significantly simplifying things.
- Introduced needs_adjacent_anon_pgoff() to be really clear about what the merge logic is doing.
- Passing through the anon pgoff fixes the issue Sashiko raised with shared file-backed mappings having incorrect anon pgoff (inconsequential but a wrinkle nonetheless).
- Dropped unnecessary needs_rmap_locks change in 9/15 - this checks to see if rmap locks need to be taken due to the range being moved backwards. Since both file-backed and anon page offset are updated on such a move, it suffices to check only the former. Updated commit message to reflect it.
- Sashiko complained about a couple missing .is_anon_walk entries in pvmw's - page_mapped_in_vma() and migrate_vma_collect_huge_pmd() - however neither impact anything - the field is only meaningful for vma_address_end() if nr_pages > 1 and neither site is impacted. Moreover, neither site sets pgoff either. Update the commit message to explain this.
- Renamed is_anon_walk to pgoff_is_anon and document that pvmw->pgoff is only meaningful if pvmw->nr_pages > 1.
- Highlighted the user-visible changes to /dev/zero being true-anon in the relevant commit message as per Yang.
- Fixed a bug in the proc-self-map-files-00[1,2].c procfs selftests - the code was trying to MAP_PRIVATE 'an arbitrary file' then asserts that file-backed procfs entries exist, but happened to choose /dev/zero. Updated to the guaranteed-available /proc/self/exe, as reported by Mark.
- Fixed an issue with drivers that intentionally mark vma->vm_ops as NULL (using the legacy ->mmap callback). If they do this set dummy ops, which is what they meant. The mmap_prepare case is fine as nobody does this there and this will be fixed when all drivers are finally converted to mmap_prepare. As reported by Sashiko.
- Added some missed VMA selftest vma_start_[anon]_pgoff() conversions in the merge test commit as per Sashiko.
- Various small prose/comment fixups.
https://patch.msgid.link/[email protected]
v2:
- Removed incorrect assert on always-NULL folio from 7/15, as per syzbot.
- Updated 2/15 so linear_virt_page_index() checks for vma_is_anonymous() as well to be cautious about 'special' (VDSO, VVAR, etc.) VMAs accidentally being asserted when CONFIG_DEBUG_VM is set, as per Sashiko.
- Updated commit message of 9/15 to mention the subtle change in NUMA interleaving behaviour, as per Sashiko.
- Updated 10/15 to assert virtual page offset for adjacent VMAs for various VMA userland tests, as per Sashiko.
- Updated 13/15 to remove the !vma->vm_file check altogether after MAP_PRIVATE-/dev/zero is made pure anon in vma_start_virt_pgoff().
- Updated 12/15 to check that the /dev/zero device is a character device since it turns out that block and character devices have their own separate major/minor device number namespaces... :) as per Sashiko.
- Updated 12/15 to fix a bisection hazard where vma->vm_ops would be overwritten by vma_dummy_vm_ops for MAP_PRIVATE-/dev/zero, as per Sashiko.
- Updated 12/15 to fix another bisection hazard (...!) due to ordering of vma_set_anonymous(). Removed in 13/15.
- Added comments to __vm_virt_pgoff[lo, hi] fields referencing vma_start_virt_pgoff()'s comment to be clearer what these are as per Xu Xin in 1/15.
- Various typo fixes + cleanups in prose.
- Updated the cover letter to point out that the MAP_PRIVATE-/dev/zero issue is addressed in this series too.
https://patch.msgid.link/[email protected]
v1:
- Rebased onto mm-new.
- Dependent series heavily reviewed and looks highly likely to land, so un-RFC.
- Added explicit check for /dev/zero and removed ability for arbitrary mmap/mmap_prepare hooks to make themselves anonymous.
- Made MAP_PRIVATE-/dev/zero mappings truly anonymous.
- Added MAP_PRIVATE file-backed mapping merge test to selftests.
- Added a MAP_PRIVATE-/dev/zero VMA userland test to assert that the VMA really is made anonymous.
- Added MAP_PRIVATE-/dev/zero merge tests to selftests.
- Fixed missed virtual page index site in try_to_merge_with_ksm_page().
- Updated folio_within_range() to use virtual page offset for anon folio. This had no impact as it is only called for large folios at the moment (and MAP_PRIVATE-file backed mappings can't currently be backed by a large folio) but making the change now protects us for the future.
- Fixed typos etc.
https://patch.msgid.link/[email protected]
RFC:
https://patch.msgid.link/[email protected]
To: Andrew Morton <[email protected]>
To: David Hildenbrand <[email protected]>
To: "Liam R. Howlett" <[email protected]>
To: Vlastimil Babka <[email protected]>
To: Mike Rapoport <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Michal Hocko <[email protected]>
To: Jann Horn <[email protected]>
To: Pedro Falcato <[email protected]>
To: "Matthew Wilcox (Oracle)" <[email protected]>
To: Jan Kara <[email protected]>
To: Miaohe Lin <[email protected]>
To: Naoya Horiguchi <[email protected]>
To: Rik van Riel <[email protected]>
To: Harry Yoo <[email protected]>
To: Lance Yang <[email protected]>
To: Kees Cook <[email protected]>
To: Zi Yan <[email protected]>
To: Baolin Wang <[email protected]>
To: Nico Pache <[email protected]>
To: Ryan Roberts <[email protected]>
To: Dev Jain <[email protected]>
To: Barry Song <[email protected]>
To: Usama Arif <[email protected]>
To: Matthew Brost <[email protected]>
To: Joshua Hahn <[email protected]>
To: Rakie Kim <[email protected]>
To: Byungchul Park <[email protected]>
To: Gregory Price <[email protected]>
To: Ying Huang <[email protected]>
To: Alistair Popple <[email protected]>
To: Peter Xu <[email protected]>
To: Xu Xin <[email protected]>
To: Chengming Zhou <[email protected]>
To: Arnd Bergmann <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Christian Borntraeger <[email protected]>
To: Janosch Frank <[email protected]>
To: Claudio Imbrenda <[email protected]>
To: Alexander Gordeev <[email protected]>
To: Gerald Schaefer <[email protected]>
To: Heiko Carstens <[email protected]>
To: Vasily Gorbik <[email protected]>
To: Sven Schnelle <[email protected]>
To: Alex Deucher <[email protected]>
To: Christian König <[email protected]>
To: David Airlie <[email protected]>
To: Simona Vetter <[email protected]>
To: Maarten Lankhorst <[email protected]>
To: Maxime Ripard <[email protected]>
To: Thomas Zimmermann <[email protected]>
To: Boris Brezillon <[email protected]>
To: Steven Price <[email protected]>
To: Liviu Dudau <[email protected]>
To: Huang Rui <[email protected]>
To: Matthew Auld <[email protected]>
To: Thomas Hellström <[email protected]>
To: Rodrigo Vivi <[email protected]>
To: Masami Hiramatsu <[email protected]>
To: Oleg Nesterov <[email protected]>
To: Peter Zijlstra <[email protected]>
To: Ingo Molnar <[email protected]>
To: Arnaldo Carvalho de Melo <[email protected]>
To: Namhyung Kim <[email protected]>
To: Mark Rutland <[email protected]>
To: Alexander Shishkin <[email protected]>
To: Jiri Olsa <[email protected]>
To: Ian Rogers <[email protected]>
To: Adrian Hunter <[email protected]>
To: James Clark <[email protected]>
To: Jason Gunthorpe <[email protected]>
To: John Hubbard <[email protected]>
To: Muchun Song <[email protected]>
To: Oscar Salvador <[email protected]>
To: Chris Li <[email protected]>
To: Kairui Song <[email protected]>
To: Kemeng Shi <[email protected]>
To: Nhat Pham <[email protected]>
To: Baoquan He <[email protected]>
To: Youngjun Park <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
Lorenzo Stoakes (ARM) (16):
mm/vma: introduce VMA anon page offset field and add helpers
mm: provide vma_[flags_]is_cow_mapping() and remove is_cow_mapping()
mm: introduce linear_anon_page_index()
mm: abstract vma_address() and introduce vma_anon_address()
mm: update print_bad_page_map() to show anon index if appropriate
mm: introduce and use vma_filebacked_address()
mm/vma: fix self-merge check in copy_vma() tools/testing/vma: add tests for copy_vma() self-merge
mm: propagate VMA anonymous page offset on map, remap, split + merge mm/rmap: track whether the page VMA mapped pgoff is anonymous
mm: clean up vma_address_end()
mm/huge_memory: update remove_migration_pmd() to accept a folio mm/migrate: calculate large folio page index using PFN mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios tools/testing/vma: expand VMA merge tests to assert anon pgoff tools/testing/selftests/mm: test anonymous page offset merge behaviour
arch/s390/mm/gmap_helpers.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/panthor/panthor_gem.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_vm.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- fs/proc/task_mmu.c | 2 +- include/linux/mm.h | 132 ++++++++++++++++++++++++++++++-- include/linux/mm_types.h | 12 +++ include/linux/pagemap.h | 40 +++++++++- include/linux/rmap.h | 4 +- include/linux/swapops.h | 6 +- kernel/events/uprobes.c | 2 +- mm/gup.c | 2 +- mm/huge_memory.c | 26 +++---- mm/hugetlb.c | 2 +- mm/internal.h | 72 ++++++++++++----- mm/interval_tree.c | 4 +- mm/ksm.c | 6 +- mm/memory-failure.c | 4 +- mm/memory.c | 38 +++++---- mm/mempolicy.c | 2 +- mm/migrate.c | 21 +++-- mm/mremap.c | 6 +- mm/page_vma_mapped.c | 6 +- mm/rmap.c | 22 +++--- mm/userfaultfd.c | 4 +- mm/vma.c | 113 +++++++++++++++++++-------- mm/vma.h | 89 +++++++++++++-------- mm/vma_exec.c | 2 +- mm/vma_init.c | 1 + tools/testing/selftests/mm/merge.c | 57 ++++++++++++++ tools/testing/vma/include/dup.h | 62 ++++++++++++++- tools/testing/vma/shared.c | 3 +- tools/testing/vma/tests/merge.c | 49 +++++++++--- tools/testing/vma/tests/vma.c | 50 +++++++++++- tools/testing/vma/vma_internal.h | 1 + 37 files changed, 667 insertions(+), 187 deletions(-)
---
base-commit: 5093dba1014c1d7f7e247fd118f0fa8f22136046
change-id: 20260711-b4-scalable-cow-virt-pgoff-a0cc0eb14bc6
Cheers,
Signature · 1 line
Lorenzo Stoakes (ARM) <[email protected]>
Lorenzo Stoakes (ARM)
maintainerPATCH v5 1/16 mm/vma: introduce VMA anon page offset field and add helpers
Establish fields in vm_area_struct to store the anonymous page offset of VMAs.
Initially, the anonymous page offset of a VMA is vma->vm_start >> PAGE_SHIFT.
When a VMA is remapped to new_address its anonymous page offset is either updated to new_address >> PAGE_SHIFT if unfaulted or, if faulted, remains equal to the anonymous page offset it had when first faulted.
Currently, anonymous folios belonging to CoW'd MAP_PRIVATE-mapped file-backed VMAs are tracked by their file offsets. By adding anonymous offset as a property of VMAs, we can now track them by their anonymous page offset instead.
By tracking this, we provide the means by which to eliminate this inconsistency, and more importantly lay the foundations for future work for the scalable CoW anonymous rmap rework.
This patch simply adds the fields and some simple helpers. Subsequent patches will update mm code to make use of these fields correctly.
The fields chosen are packed in the VMA such that, for 64-bit kernel builds, no additional space is taken up.
The first field is present on cacheline 0 containing key VMA fields, and the second on cacheline 3, which contains file-backed reverse mapping fields.
Given the relative time spent accessing reverse mapping fields as well as updating them, there shouldn't be any performance impact here from false sharing.
Update the VMA userland tests to account for this change.
No callsites are updated yet, so no functional change intended.
--- include/linux/mm.h | 59 +++++++++++++++++++++++++++++++++++++++++ include/linux/mm_types.h | 12 +++++++++ mm/vma.h | 14 ++++++++++ mm/vma_init.c | 1 + tools/testing/vma/include/dup.h | 26 ++++++++++++++++++ 5 files changed, 112 insertions(+)
include/linux/mm.h +59 -0
@@ -4393,6 +4393,65 @@ static inline pgoff_t vma_last_pgoff(const struct vm_area_struct *vma)return vma_end_pgoff (vma) -1 ; } +/** +* vma_start_anon_pgoff() - Get the anonymous page offset of the start of @vma +* @vma: The VMA whose anonymous page offset is required. +* +* If unfaulted, then this is vma->vm_start >> PAGE_SHIFT, if faulted then the +* anonymous page offset at the time of first fault. +* +* If the VMA is anonymous, this returns the same value as vma_start_pgoff(). +* +* This value is used for tracking MAP_PRIVATE file-backed mappings by their +* anonymous page offset. +* +* Returns: The anonymous page offset of the start of @vma. +*/ +static inline pgoff_t vma_start_anon_pgoff (const struct vm_area_struct *vma) +{ +pgoff_t pgoff =0 ; + +#ifdef CONFIG_64BIT + pgoff += vma->__vm_anon_pgoff_hi; + pgoff <<=32 ; +#endif + pgoff += vma->__vm_anon_pgoff_lo; +return pgoff; +} + +/** +* vma_end_anon_pgoff() - Get the anonymous page offset of the exclusive end of +* @vma. +* @vma: The VMA whose end anonymous page offset is required. +* +* This returns the anonymous exclusive end page offset of @vma, which is useful +* for expressing page offset ranges. +* +* See the description of vma_start_anon_pgoff() for a description of VMA +* anonymous page offsets. +* +* Returns: The exclusive end anonymous page offset of @vma. +*/ +static inline pgoff_t vma_end_anon_pgoff (const struct vm_area_struct *vma) +{ +return vma_start_anon_pgoff (vma) +vma_pages (vma); +} + +/** +* vma_last_anon_pgoff() - Get the anonymous page offset of the last page in +* @vma. +* @vma: The VMA whose last anonymous page offset is required. +* +* See the description of vma_start_anon_pgoff() for a description of VMA +* anonymous page offsets. +* +* Returns: The last anonymous page offset of @vma. +*/ +static inline pgoff_t vma_last_anon_pgoff (const struct vm_area_struct *vma) +{ +return vma_end_anon_pgoff (vma) -1 ; +} +static inline unsigned long vma_desc_size (const struct vm_area_desc *desc) {return desc->end - desc->start;
include/linux/mm_types.h +12 -0
@@ -967,6 +967,11 @@ struct vm_area_struct {*/ unsigned int vm_lock_seq;#endif +/* +* Low 32-bits of anonymous page offset. +* See vma_start_anon_pgoff() comment for details. +*/ +unsigned int __vm_anon_pgoff_lo;/* * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma * list, after a COW of one of the file pages. A MAP_SHARED vma
@@ -1041,6 +1046,13 @@ struct vm_area_struct {#ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map vmlock_dep_map;#endif +#endif +#ifdef CONFIG_64BIT +/* +* High 32-bits of anonymous page offset. +* See vma_start_anon_pgoff() comment for details. +*/ +unsigned int __vm_anon_pgoff_hi;#endif /* * For areas with an address space and backing store,
mm/vma.h +14 -0
@@ -283,6 +283,20 @@ static inline void vma_set_pgoff(struct vm_area_struct *vma, pgoff_t pgoff) vma->vm_pgoff = pgoff; } +static inline void __vma_set_anon_pgoff (struct vm_area_struct *vma,pgoff_t pgoff) +{ +#ifdef CONFIG_64BIT + vma->__vm_anon_pgoff_hi = pgoff >>32 ; +#endif + vma->__vm_anon_pgoff_lo = pgoff &GENMASK (31 ,0 ); +} + +static inline void vma_set_anon_pgoff (struct vm_area_struct *vma,pgoff_t pgoff) +{ +vma_assert_can_modify (vma); +__vma_set_anon_pgoff (vma, pgoff); +} +static inline void vma_add_pgoff (struct vm_area_struct *vma,pgoff_t delta) {vma_assert_can_modify (vma);
mm/vma_init.c +1 -0
@@ -51,6 +51,7 @@ static void vm_area_init_from(const struct vm_area_struct *src, dest->vm_end = src->vm_end; dest->anon_vma = src->anon_vma; dest->vm_pgoff =vma_start_pgoff (src); +__vma_set_anon_pgoff (dest,vma_start_anon_pgoff (src)); dest->vm_file = src->vm_file; dest->vm_private_data = src->vm_private_data;vm_flags_init (dest, src->vm_flags);
tools/testing/vma/include/dup.h +26 -0
@@ -577,6 +577,7 @@ struct vm_area_struct {*/ unsigned int vm_lock_seq;#endif +unsigned int __vm_anon_pgoff_lo;/* * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma
@@ -612,6 +613,9 @@ struct vm_area_struct {#ifdef CONFIG_PER_VMA_LOCK /* Unstable RCU readers are allowed to read this. */ refcount_t vm_refcnt; +#endif +#ifdef CONFIG_64BIT +unsigned int __vm_anon_pgoff_hi;#endif /* * For areas with an address space and backing store,
@@ -1320,6 +1324,28 @@ static inline pgoff_t vma_end_pgoff(const struct vm_area_struct *vma)return vma_start_pgoff (vma) +vma_pages (vma); } +static inline pgoff_t vma_start_anon_pgoff (const struct vm_area_struct *vma) +{ +pgoff_t pgoff =0 ; + +#ifdef CONFIG_64BIT + pgoff += vma->__vm_anon_pgoff_hi; + pgoff <<=32 ; +#endif + pgoff += vma->__vm_anon_pgoff_lo; +return pgoff; +} + +static inline pgoff_t vma_end_anon_pgoff (const struct vm_area_struct *vma) +{ +return vma_start_anon_pgoff (vma) +vma_pages (vma); +} + +static inline pgoff_t vma_last_anon_pgoff (const struct vm_area_struct *vma) +{ +return vma_end_anon_pgoff (vma) -1 ; +} +static inline int vfs_mmap_prepare (struct file *file,struct vm_area_desc *desc) {return file->f_op->mmap_prepare (desc);
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 2/16 mm: provide vma_[flags_]is_cow_mapping() and remove is_cow_mapping()
In reply to Lorenzo Stoakes (ARM)All remaining callers of is_cow_mapping() are invoking it in the form of is_cow_mapping(vma->vm_flags) or an indirected version of this.
Therefore, provide a helper - vma_is_cow_mapping() to directly test the VMA.
Additionally provide a new helper vma_flags_is_cow_mapping() which performs the check using the new vma_flags_t type, and share this logic between vma_is_cow_mapping() and vma_desc_is_cow_mapping().
With these changes, no callers of is_cow_mapping() remain, so remove it.
Also update the userland VMA tests to reflect the change.
No functional change intended.
--- arch/s390/mm/gmap_helpers.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/panthor/panthor_gem.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_vm.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- fs/proc/task_mmu.c | 2 +- include/linux/mm.h | 71 ++++++++++++++++++++++++++++++--- kernel/events/uprobes.c | 2 +- mm/gup.c | 2 +- mm/huge_memory.c | 8 ++-- mm/hugetlb.c | 2 +- mm/internal.h | 2 +- mm/memory.c | 25 ++++++------ mm/mempolicy.c | 2 +- tools/testing/vma/include/dup.h | 11 +++++ 16 files changed, 105 insertions(+), 36 deletions(-)
arch/s390/mm/gmap_helpers.c +1 -1
@@ -200,7 +200,7 @@ static int find_zeropage_pte_entry(pte_t *pte, unsigned long addr,* currently only works in COW mappings, which is also where * mm_forbids_zeropage() is checked. */ -if (!is_cow_mapping (walk->vma->vm_flags)) +if (!vma_is_cow_mapping (walk->vma))return -EFAULT ; *found_addr = addr;
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +2 -2
@@ -377,9 +377,9 @@ static int amdgpu_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_str/* Workaround for Thunk bug creating PROT_NONE,MAP_PRIVATE mappings * for debugger access to invisible VRAM. Should have used MAP_SHARED * instead. Clearing VM_MAYWRITE prevents the mapping from ever -* becoming writable and makes is_cow_mapping(vm_flags) false. +* becoming writable and makes vma_is_cow_mapping(vma) false. */ -if (is_cow_mapping (vma->vm_flags) && +if (vma_is_cow_mapping (vma) && !(vma->vm_flags &VM_ACCESS_FLAGS ))vm_flags_clear (vma,VM_MAYWRITE );
drivers/gpu/drm/drm_gem_shmem_helper.c +1 -1
@@ -753,7 +753,7 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_structreturn ret; } -if (is_cow_mapping (vma->vm_flags)) +if (vma_is_cow_mapping (vma))return -EINVAL ;dma_resv_lock (shmem->base.resv,NULL );
drivers/gpu/drm/panthor/panthor_gem.c +1 -1
@@ -761,7 +761,7 @@ static int panthor_gem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vreturn ret; } -if (is_cow_mapping (vma->vm_flags)) +if (vma_is_cow_mapping (vma))return -EINVAL ;if (!refcount_inc_not_zero (&bo->cmap.mmap_count)) {
drivers/gpu/drm/ttm/ttm_bo_vm.c +1 -1
@@ -489,7 +489,7 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {int ttm_bo_mmap_obj (struct vm_area_struct *vma,struct ttm_buffer_object *bo) {/* Enforce no COW since would have really strange behavior with it. */ -if (is_cow_mapping (vma->vm_flags)) +if (vma_is_cow_mapping (vma))return -EINVAL ;drm_gem_object_get (&bo->base);
drivers/gpu/drm/xe/xe_device.c +1 -1
@@ -330,7 +330,7 @@ static int xe_pci_barrier_mmap(struct file *filp,if (vma->vm_end - vma->vm_start >SZ_4K )return -EINVAL ; -if (is_cow_mapping (vma->vm_flags)) +if (vma_is_cow_mapping (vma))return -EINVAL ;if (vma->vm_flags & (VM_READ |VM_EXEC ))
fs/proc/task_mmu.c +1 -1
@@ -1693,7 +1693,7 @@ static inline bool pte_is_pinned(struct vm_area_struct *vma, unsigned long addr,if (!pte_write (pte))return false; -if (!is_cow_mapping (vma->vm_flags)) +if (!vma_is_cow_mapping (vma))return false;if (likely (!mm_flags_test (MMF_HAS_PINNED , vma->vm_mm)))return false;
include/linux/mm.h +65 -6
82 lines not on this page · raw message
kernel/events/uprobes.c +1 -1
@@ -513,7 +513,7 @@ int uprobe_write(struct arch_uprobe *auprobe, struct vm_area_struct *vma, uprobe = container_of(auprobe, struct uprobe, arch); -if (WARN_ON_ONCE (!is_cow_mapping (vma->vm_flags))) +if (WARN_ON_ONCE (!vma_is_cow_mapping (vma)))return -EINVAL ;/*
mm/gup.c +1 -1
@@ -1236,7 +1236,7 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)* Anon pages in shared mappings are surprising: now * just reject it. */ -if (!is_cow_mapping (vm_flags)) +if (!vma_is_cow_mapping (vma))return -EFAULT ; } }else if (!(vm_flags &VM_READ )) {
mm/huge_memory.c +4 -4
@@ -1638,7 +1638,7 @@ vm_fault_t vmf_insert_pfn_pmd(struct vm_fault *vmf, unsigned long pfn,BUG_ON (!(vma->vm_flags & (VM_PFNMAP |VM_MIXEDMAP )));BUG_ON ((vma->vm_flags & (VM_PFNMAP |VM_MIXEDMAP )) == (VM_PFNMAP |VM_MIXEDMAP )); -BUG_ON ((vma->vm_flags &VM_PFNMAP ) &&is_cow_mapping (vma->vm_flags)); +BUG_ON ((vma->vm_flags &VM_PFNMAP ) &&vma_is_cow_mapping (vma));pfnmap_setup_cachemode_pfn (pfn, &pgprot);
@@ -1746,7 +1746,7 @@ vm_fault_t vmf_insert_pfn_pud(struct vm_fault *vmf, unsigned long pfn,BUG_ON (!(vma->vm_flags & (VM_PFNMAP |VM_MIXEDMAP )));BUG_ON ((vma->vm_flags & (VM_PFNMAP |VM_MIXEDMAP )) == (VM_PFNMAP |VM_MIXEDMAP )); -BUG_ON ((vma->vm_flags &VM_PFNMAP ) &&is_cow_mapping (vma->vm_flags)); +BUG_ON ((vma->vm_flags &VM_PFNMAP ) &&vma_is_cow_mapping (vma));pfnmap_setup_cachemode_pfn (pfn, &pgprot);
@@ -1888,7 +1888,7 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,* applied special bit, or we made the PRIVATE mapping be * able to wrongly write to the backend MMIO. */ -VM_WARN_ON_ONCE (is_cow_mapping (src_vma->vm_flags) &&pmd_write (pmd)); +VM_WARN_ON_ONCE (vma_is_cow_mapping (src_vma) &&pmd_write (pmd)); goto set_pmd; }
@@ -2009,7 +2009,7 @@ int copy_huge_pud(struct mm_struct *dst_mm, struct mm_struct *src_mm,* TODO: once we support anonymous pages, use * folio_try_dup_anon_rmap_*() and split if duplicating fails. */ -if (is_cow_mapping (vma->vm_flags) &&pud_write (pud)) { +if (vma_is_cow_mapping (vma) &&pud_write (pud)) {pudp_set_wrprotect (src_mm, addr, src_pud); pud =pud_wrprotect (pud); }
mm/hugetlb.c +1 -1
@@ -4905,7 +4905,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,pte_t *src_pte, *dst_pte, entry;struct folio *pte_folio;unsigned long addr; -bool cow =is_cow_mapping (src_vma->vm_flags); +bool cow =vma_is_cow_mapping (src_vma);struct hstate *h =hstate_vma (src_vma);unsigned long sz =huge_page_size (h);unsigned long npages =pages_per_huge_page (h);
mm/internal.h +1 -1
@@ -1341,7 +1341,7 @@ static inline bool gup_must_unshare(struct vm_area_struct *vma,* ... because we only care about writable private ("COW") * mappings where we have to break COW early. */ -return is_cow_mapping (vma->vm_flags); +return vma_is_cow_mapping (vma); }/* Paired with a memory barrier in folio_try_share_anon_rmap_*(). */
mm/memory.c +12 -13
@@ -780,7 +780,7 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,/* Only CoW'ed anon folios are "normal". */ if (pfn == index)return NULL ; -if (!is_cow_mapping (vma->vm_flags)) +if (!vma_is_cow_mapping (vma))return NULL ; } }
@@ -1002,7 +1002,6 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm, pte_t *dst_pte, pte_t *src_pte, struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma, unsigned long addr, int *rss) { -vm_flags_t vm_flags = dst_vma->vm_flags;pte_t orig_pte =ptep_get (src_pte);softleaf_t entry =softleaf_from_pte (orig_pte);pte_t pte = orig_pte;
@@ -1026,7 +1025,7 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm, rss[mm_counter (folio)]++;if (!softleaf_is_migration_read (entry) && -is_cow_mapping (vm_flags)) { +vma_is_cow_mapping (dst_vma)) {/* * COW mappings require pages in both parent and child * to be set to read. A previously exclusive entry is
@@ -1067,7 +1066,7 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,* save and restore device driver state). */ if (softleaf_is_device_private_write (entry) && -is_cow_mapping (vm_flags)) { +vma_is_cow_mapping (dst_vma)) { entry =make_readable_device_private_entry (swp_offset (entry)); pte =swp_entry_to_pte (entry);
@@ -1082,7 +1081,7 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,* exclusive entries currently only support private writable * (ie. COW) mappings. */ -VM_BUG_ON (!is_cow_mapping (src_vma->vm_flags)); +VM_BUG_ON (!vma_is_cow_mapping (src_vma));if (try_restore_exclusive_pte (src_vma, addr, src_pte, orig_pte))return -EBUSY ;return -ENOENT ;
@@ -1181,7 +1180,7 @@ static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma, }/* If it's a COW mapping, write protect it both processes. */ -if (is_cow_mapping (src_vma->vm_flags) && writable) { +if (vma_is_cow_mapping (src_vma) && writable) {wrprotect_ptes (src_mm, addr, src_pte, nr); pte =pte_wrprotect (pte); }
@@ -1602,9 +1601,9 @@ copy_page_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma)* We need to invalidate the secondary MMU mappings only when * there could be a permission downgrade on the ptes of the * parent mm. And a permission downgrade will only happen if -* is_cow_mapping() returns true. +* vma_is_cow_mapping() returns true. */ - is_cow =is_cow_mapping (src_vma->vm_flags); + is_cow =vma_is_cow_mapping (src_vma);if (is_cow) { mmu_notifier_range_init(&range, MMU_NOTIFY_PROTECTION_PAGE,
@@ -2388,7 +2387,7 @@ static bool vm_mixed_zeropage_allowed(struct vm_area_struct *vma)if (mm_forbids_zeropage (vma->vm_mm))return false;/* zeropages in COW mappings are common and unproblematic. */ -if (is_cow_mapping (vma->vm_flags)) +if (vma_is_cow_mapping (vma))return true;/* Mappings that do not allow for writable PTEs are unproblematic. */ if (!(vma->vm_flags & (VM_WRITE |VM_MAYWRITE )))
@@ -2839,7 +2838,7 @@ vm_fault_t vmf_insert_pfn_prot(struct vm_area_struct *vma, unsigned long addr,BUG_ON (!(vma->vm_flags & (VM_PFNMAP |VM_MIXEDMAP )));BUG_ON ((vma->vm_flags & (VM_PFNMAP |VM_MIXEDMAP )) == (VM_PFNMAP |VM_MIXEDMAP )); -BUG_ON ((vma->vm_flags &VM_PFNMAP ) &&is_cow_mapping (vma->vm_flags)); +BUG_ON ((vma->vm_flags &VM_PFNMAP ) &&vma_is_cow_mapping (vma));BUG_ON ((vma->vm_flags &VM_MIXEDMAP ) &&pfn_valid (pfn));if (addr < vma->vm_start || addr >= vma->vm_end)
@@ -3251,7 +3250,7 @@ static int remap_pfn_range_prepare_vma(struct vm_area_struct *vma, unsigned long size) {const unsigned long end = addr +PAGE_ALIGN (size); -const bool is_cow =is_cow_mapping (vma->vm_flags); +const bool is_cow =vma_is_cow_mapping (vma);int err; err = get_remap_pgoff(is_cow, addr, end, vma->vm_start, vma->vm_end,
@@ -6751,7 +6750,7 @@ static vm_fault_t sanitize_fault_flags(struct vm_area_struct *vma,* FAULT_FLAG_UNSHARE only applies to COW mappings. Let's * just treat it like an ordinary read-fault otherwise. */ -if (!is_cow_mapping (vma->vm_flags)) +if (!vma_is_cow_mapping (vma)) *flags &= ~FAULT_FLAG_UNSHARE ; }else if (*flags &FAULT_FLAG_WRITE ) {/* Write faults on read-only mappings are impossible ... */
@@ -6759,7 +6758,7 @@ static vm_fault_t sanitize_fault_flags(struct vm_area_struct *vma,return VM_FAULT_SIGSEGV ;/* ... and FOLL_FORCE only applies to COW mappings. */ if (WARN_ON_ONCE (!(vma->vm_flags &VM_WRITE ) && - !is_cow_mapping (vma->vm_flags))) + !vma_is_cow_mapping (vma)))return VM_FAULT_SIGSEGV ; }#ifdef CONFIG_PER_VMA_LOCK
mm/mempolicy.c +1 -1
@@ -844,7 +844,7 @@ bool folio_can_map_prot_numa(struct folio *folio, struct vm_area_struct *vma,return false;/* Also skip shared copy-on-write folios */ -if (is_cow_mapping (vma->vm_flags) &&folio_maybe_mapped_shared (folio)) +if (vma_is_cow_mapping (vma) &&folio_maybe_mapped_shared (folio))return false;/* Folios are pinned and can't be migrated */
tools/testing/vma/include/dup.h +11 -0
@@ -1162,6 +1162,17 @@ static inline bool vma_is_shared_maywrite(struct vm_area_struct *vma)return is_shared_maywrite (&vma->flags); } +static inline bool vma_flags_is_cow_mapping (const vma_flags_t *flags) +{ +return vma_flags_test (flags,VMA_MAYWRITE_BIT ) && + !vma_flags_test (flags,VMA_SHARED_BIT ); +} + +static inline bool vma_is_cow_mapping (const struct vm_area_struct *vma) +{ +return vma_flags_is_cow_mapping (&vma->flags); +} +static inline struct vm_area_struct *vma_next (struct vma_iterator *vmi) {/*
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerRe: [PATCH v5 02/16] mm: provide vma_[flags_]is_cow_mapping() and remove is_cow_mapping()
(trim cc-list)
On Thu, Aug 13, 2026 at 06:32:19PM +0100, Lorenzo Stoakes (ARM) wrote:
...
Quoted 8 lines
include/linux/mm.h +0
-0@@ -2271,17 +2271,76 @@ void unpin_user_pages(struct page **pages, unsigned long npages);void unpin_user_folio (struct folio *folio,unsigned long npages);void unpin_folios (struct folio **folios,unsigned long nfolios);
...
Quoted 3 lines
+/**
+ *vma_is_cow_mapping()- Is this VMA a CoW mapping?
+ * @desc: The VMA to check.
Andrew - a small typo here, this should be @vma not @desc, are you still to fix this in place? If not can always send a fixup patch next cycle :))
(Credit to David's scripts for finding this! :)
include/linux/mm.h void unpin_user_pages(struct page **pages, unsigned long npages); · line 2330
Quoted 11 lines
+ *
+ * Seevma_flags_is_cow_mapping()for details.
+ *
+ * Returns: true if the VMA is a CoW mapping, otherwise false.
+ */include/linux/mm.h +2
-1+static inline bool vma_is_cow_mapping (const struct vm_area_struct *vma) { -return (flags & (VM_SHARED |VM_MAYWRITE )) ==VM_MAYWRITE ; +return vma_flags_is_cow_mapping (&vma->flags); }
--
Cheers, Lorenzo
Lorenzo Stoakes (ARM)
maintainerPATCH v5 3/16 mm: introduce linear_anon_page_index()
In reply to Lorenzo Stoakes (ARM)This function provides the anonymous equivalent of linear_page_index(), instead offsetting based on the anonymous page offset of the VMA.
It is valid only for anonymous or MAP_PRIVATE file-backed mappings, in other words CoW mappings.
For pure anon VMAs, this will be equal to linear_page_index().
Assert that both of these invariants are true in linear_anon_page_index() and implement the algorithm in __linear_anon_page_index().
Note that MAP_PRIVATE-/dev/zero mappings will satisfy vma_is_anonymous() but not fulfill this invariant, so when asserting this we check vma->vm_file to account for this.
We do not update callsites yet, so no functional change intended.
Also const-ify vma_is_anonymous() to make it compatible with the const-ified linear_anon_page_index().
While we're here, update linear_page_index() to be more succinct.
VMA userland tests are also updated accordingly.
--- include/linux/mm.h | 2 +- include/linux/pagemap.h | 40 +++++++++++++++++++++++++++++++++++++--- tools/testing/vma/include/dup.h | 25 ++++++++++++++++++++++++- 3 files changed, 62 insertions(+), 5 deletions(-)
include/linux/mm.h +1 -1
@@ -1556,7 +1556,7 @@ static inline void vma_desc_set_anonymous(struct vm_area_desc *desc) desc->vm_ops =NULL ; } -static inline bool vma_is_anonymous (struct vm_area_struct *vma) +static inline bool vma_is_anonymous (const struct vm_area_struct *vma) {return !vma->vm_ops; }
include/linux/pagemap.h +37 -3
@@ -1094,10 +1094,44 @@ static inline pgoff_t linear_page_delta(const struct vm_area_struct *vma,static inline pgoff_t linear_page_index (const struct vm_area_struct *vma,const unsigned long address) { -pgoff_t pgoff; +return linear_page_delta (vma, address) +vma_start_pgoff (vma); +} + +static inline pgoff_t __linear_anon_page_index (const struct vm_area_struct *vma, +const unsigned long address) +{ +return linear_page_delta (vma, address) +vma_start_anon_pgoff (vma); +} + +/** +* linear_anon_page_index() - Determine the absolute anonymous page offset of +* @address within @vma. +* @vma: An anonymous or MAP_PRIVATE file-backed VMA in which @address resides. +* @address: The address whose absolute page offset is required. +* +* This returns the anonymous page offset of @address, which is the page offset +* the address possessed at the time the VMA was first faulted. +* +* For anonymous mappings, this returns the same value as linear_page_index(). +* +* For MAP_PRIVATE file-backed mappings, this returns the anonymous page offset +* of @address, which is the page offset the address possessed at the time the +* VMA was first faulted. +* +* It is not valid to call this function for shared file-backed mappings. +* +* Returns: The absolute anonymous page offset of @address within @vma. +*/ +static inline pgoff_t linear_anon_page_index (const struct vm_area_struct *vma, +const unsigned long address) +{ +const pgoff_t pgoff =__linear_anon_page_index (vma, address); + +VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file) +VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address)); - pgoff =linear_page_delta (vma, address); - pgoff +=vma_start_pgoff (vma);return pgoff; }
tools/testing/vma/include/dup.h +24 -1
@@ -1428,7 +1428,7 @@ static inline void vma_iter_set(struct vma_iterator *vmi, unsigned long addr)mas_set (&vmi->mas, addr); } -static inline bool vma_is_anonymous (struct vm_area_struct *vma) +static inline bool vma_is_anonymous (const struct vm_area_struct *vma) {return !vma->vm_ops; }
@@ -1621,3 +1621,26 @@ static inline pgprot_t vma_get_page_prot(const struct vm_area_struct *vma) {return vma_flags_to_page_prot (vma->flags); } + +static inline pgoff_t __linear_anon_page_index (const struct vm_area_struct *vma, +const unsigned long address) +{ +pgoff_t pgoff; + + pgoff =linear_page_delta (vma, address); + pgoff +=vma_start_anon_pgoff (vma); +return pgoff; +} + +static inline pgoff_t linear_anon_page_index (const struct vm_area_struct *vma, +const unsigned long address) +{ +const pgoff_t pgoff =__linear_anon_page_index (vma, address); + +VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file) +VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address)); + +return pgoff; +}
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 4/16 mm: abstract vma_address() and introduce vma_anon_address()
In reply to Lorenzo Stoakes (ARM)Introduce __vma_address() which abstracts the VMA start page offset field as pgoff_start, then update vma_address() to use it.
Then introduce vma_anon_address() which does the equivalent of vma_address(), only using the anonymous page offset of the VMA rather than the file-backed one.
Also add an assert to ensure that the function is not called for mappings which are file-backed but not MAP_PRIVATE to ensure it is only used in the correct places.
This will be necessary for determining the address of a folio's index within a VMA when the folio belongs to a MAP_PRIVATE file-backed VMA but has been CoW'd, and thus is anonymous, once the anonymous VMA page offset field is used for the reverse mapping.
No callers are updated, so no functional change intended.
--- mm/internal.h | 49 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 12 deletions(-)
mm/internal.h +37 -12
@@ -1005,19 +1005,9 @@ void mlock_drain_remote(int cpu);extern pmd_t maybe_pmd_mkwrite (pmd_t pmd,struct vm_area_struct *vma); -/** -* vma_address - Find the virtual address a page range is mapped at -* @vma: The vma which maps this object. -* @pgoff: The page offset within its object. -* @nr_pages: The number of pages to consider. -* -* If any page in this range is mapped by this VMA, return the first address -* where any of these pages appear. Otherwise, return -EFAULT. -*/ -static inline unsigned long vma_address (const struct vm_area_struct *vma, -pgoff_t pgoff,unsigned long nr_pages) +static inline unsigned long __vma_address (const struct vm_area_struct *vma, +pgoff_t pgoff,pgoff_t pgoff_start,unsigned long nr_pages) { -const pgoff_t pgoff_start =vma_start_pgoff (vma);unsigned long address;if (pgoff >= pgoff_start) {
@@ -1035,6 +1025,41 @@ static inline unsigned long vma_address(const struct vm_area_struct *vma,return address; } +/** +* vma_address - Find the virtual address a page range is mapped at. +* @vma: The vma which maps this object. +* @pgoff: The page offset within its object. +* @nr_pages: The number of pages to consider. +* +* If any page in this range is mapped by this VMA, return the first address +* where any of these pages appear. Otherwise, return -EFAULT. +*/ +static inline unsigned long vma_address (const struct vm_area_struct *vma, +pgoff_t pgoff,unsigned long nr_pages) +{ +return __vma_address (vma, pgoff,vma_start_pgoff (vma), nr_pages); +} + +/** +* vma_anon_address - Find the address an anonymous folio with index @pgoff_anon +* is mapped at. +* @vma: The vma which maps this object. +* @pgoff_anon: The anonymous page index belonging to the folio. +* @nr_pages: The number of pages to consider. +* +* This is only valid for anonymous or MAP_PRIVATE-mapped file-backed VMAs. +* +* Returns: If any page in this range is mapped by this VMA, return the first +* address where any of these pages appear. Otherwise, return -EFAULT. +*/ +static inline unsigned long vma_anon_address (const struct vm_area_struct *vma, +pgoff_t pgoff_anon,unsigned long nr_pages) +{ +VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); + +return __vma_address (vma, pgoff_anon,vma_start_anon_pgoff (vma), nr_pages); +} +/* * Then at what user virtual address will none of the range be found in vma? * Assumes that vma_address() already returned a good starting address.
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 5/16 mm: update print_bad_page_map() to show anon index if appropriate
In reply to Lorenzo Stoakes (ARM)If the VMA is a CoW mapping page offset may differ from anon page offset, indicating different positions in the relevant rmap trees.
Update print_bad_page_map() to reflect that - if the mapping is non-CoW or the indexes match, then output only one index as before, otherwise output both with (file) or (anon) suffixes to reflect which is which.
It's not possible to give only one index as there is no folio available to perform folio_test_anon() upon (the page table entry is bad so this is unavailable).
This is potentially useful debugging information and matches the existing page offset provided.
Use the raw __linear_anon_page_index() function so as to always output this value regardless of whether the mapping is file-backed or not and to avoid asserts that shouldn't apply here.
--- mm/memory.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
mm/memory.c +10 -3
@@ -631,13 +631,14 @@ static void print_bad_page_map(struct vm_area_struct *vma, {struct address_space *mapping;char entry_str[PTVAL_STR_MAX ]; -pgoff_t index; +pgoff_t index, anon_index;if (is_bad_page_map_ratelimited ())return ; mapping = vma->vm_file ? vma->vm_file->f_mapping :NULL ; index =linear_page_index (vma, addr); + anon_index =__linear_anon_page_index (vma, addr);ptval_bytes_to_hex_str (entry_str,sizeof (entry_str), entry, entry_size); pr_alert("BUG: Bad page map in process %s %s:%s", current->comm,
@@ -645,8 +646,14 @@ static void print_bad_page_map(struct vm_area_struct *vma,__print_bad_page_map_pgtable (vma->vm_mm, addr);if (page)dump_page (page,"bad page map" ); -pr_alert ("addr:%px vm_flags:%08lx anon_vma:%px mapping:%px index:%lx\n" , - (void *)addr, vma->vm_flags, vma->anon_vma, mapping, index); +pr_alert ("addr:%px vm_flags:%08lx anon_vma:%px mapping:%px" , + (void *)addr, vma->vm_flags, vma->anon_vma, mapping); +if (!vma_is_cow_mapping (vma) || index == anon_index) { +pr_cont (" index:%lx\n" , index); + }else { +pr_cont (" index:%lx (file) %lx (anon)\n" , index, anon_index); + } + pr_alert("file:%pD fault:%ps mmap:%ps mmap_prepare: %ps read_folio:%ps\n", vma->vm_file, vma->vm_ops ? vma->vm_ops->fault : NULL,
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 6/16 mm: introduce and use vma_filebacked_address()
In reply to Lorenzo Stoakes (ARM)In cases where we know that the VMA is file-backed, use vma_filebacked_address() rather than vma_address().
This lays the foundation for using the anonymous page offset via vma_anon_address().
Also add an assert to ensure that the VMA whose address is required is not anonymous.
No functional change intended.
--- mm/internal.h | 18 ++++++++++++++++++ mm/memory-failure.c | 4 ++-- mm/page_vma_mapped.c | 6 +++++- mm/rmap.c | 10 ++++++---- 4 files changed, 31 insertions(+), 7 deletions(-)
mm/internal.h +18 -0
@@ -1025,6 +1025,24 @@ static inline unsigned long __vma_address(const struct vm_area_struct *vma,return address; } +/** +* vma_filebacked_address - Find the virtual address a file-backed page range is +* mapped at. +* @vma: The vma which maps this object. +* @pgoff: The page offset within its object. +* @nr_pages: The number of pages to consider. +* +* Returns: If any page in this range is mapped by this VMA, return the first +* address where any of these pages appear. Otherwise, return -EFAULT. +*/ +static inline unsigned long vma_filebacked_address (const struct vm_area_struct *vma, +pgoff_t pgoff,unsigned long nr_pages) +{ +VM_WARN_ON_ONCE (vma_is_anonymous (vma)); + +return __vma_address (vma, pgoff,vma_start_pgoff (vma), nr_pages); +} +/** * vma_address - Find the virtual address a page range is mapped at. * @vma: The vma which maps this object.
mm/memory-failure.c +2 -2
@@ -620,7 +620,7 @@ static void add_to_kill_fsdax(struct task_struct *tsk, const struct page *p,struct vm_area_struct *vma, struct list_head *to_kill, pgoff_t pgoff) { - unsigned long addr = vma_address(vma, pgoff, 1); + unsigned long addr = vma_filebacked_address(vma, pgoff, 1);__add_to_kill (tsk, p, vma, to_kill, addr); }
@@ -2265,7 +2265,7 @@ static void add_to_kill_pgoff(struct task_struct *tsk, }/* Check for pgoff not backed by struct page */ - tk->addr =vma_address (vma, pgoff,1 ); + tk->addr =vma_filebacked_address (vma, pgoff,1 ); tk->size_shift =PAGE_SHIFT ;if (tk->addr == -EFAULT )
mm/page_vma_mapped.c +5 -1
@@ -356,6 +356,7 @@ unsigned long page_mapped_in_vma(const struct page *page, struct vm_area_struct *vma) {const struct folio *folio =page_folio (page); +const pgoff_t pgoff =page_pgoff (folio, page);struct page_vma_mapped_walk pvmw = { .pfn =page_to_pfn (page), .nr_pages =1 ,
@@ -363,7 +364,10 @@ unsigned long page_mapped_in_vma(const struct page *page, .flags = PVMW_SYNC, }; - pvmw.address =vma_address (vma,page_pgoff (folio, page),1 ); +if (folio_test_anon (folio)) + pvmw.address =vma_address (vma, pgoff,1 ); +else + pvmw.address =vma_filebacked_address (vma, pgoff,1 );if (pvmw.address == -EFAULT ) goto out;if (!page_vma_mapped_walk (&pvmw))
mm/rmap.c +6 -4
@@ -865,14 +865,15 @@ unsigned long page_address_in_vma(const struct folio *folio,if (!vma->anon_vma || !anon_vma || vma->anon_vma->root != anon_vma->root)return -EFAULT ; +/* KSM folios don't reach here because of the !anon_vma check */ +return vma_address (vma,page_pgoff (folio, page),1 ); }else if (!vma->vm_file) {return -EFAULT ; }else if (vma->vm_file->f_mapping != folio->mapping) {return -EFAULT ; } -/* KSM folios don't reach here because of the !anon_vma check */ -return vma_address (vma,page_pgoff (folio, page),1 ); +return vma_filebacked_address (vma,page_pgoff (folio, page),1 ); }/*
@@ -1321,7 +1322,7 @@ int pfn_mkclean_range(unsigned long pfn, unsigned long nr_pages, pgoff_t pgoff,if (invalid_mkclean_vma (vma,NULL ))return 0 ; - pvmw.address =vma_address (vma, pgoff, nr_pages); + pvmw.address =vma_filebacked_address (vma, pgoff, nr_pages);VM_BUG_ON_VMA (pvmw.address == -EFAULT , vma);return page_vma_mkclean_one (&pvmw);
@@ -3100,7 +3101,8 @@ static void __rmap_walk_file(struct folio *folio, struct address_space *mapping, } lookup: mapping_rmap_tree_foreach(vma, mapping, pgoff_start, pgoff_end) { -unsigned long address =vma_address (vma, pgoff_start, nr_pages); +unsigned long address =vma_filebacked_address (vma, pgoff_start, + nr_pages);VM_BUG_ON_VMA (address == -EFAULT , vma);cond_resched ();
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 7/16 mm/vma: fix self-merge check in copy_vma()
In reply to Lorenzo Stoakes (ARM)The existing logic is very confusing so improve things. Firstly rename the confusing faulted_in_anon_vma variable to can_self_merge and update this when the page offset is updated.
What is being checked for is a 'self-merge' - that is between the VMA being remapped and its prior VMA (remember that this is copy_vma() - if a non-MREMAP_DONTUNMAP remap the original VMA is only removed afterwards).
This can happen if the VMA is moved immediately adjacent to itself, either before or after it:
|----------------|----------------| | | | v | v |...............||---------------||...............| | new || old || new | |...............||---------------||---------------|
In these cases the old VMA is simply expanded to cover the new range.
It is also possible for the move to both self-merge and merge with a prior VMA if it is placed between a preceding VMA and its old self:
|---------------| | | v | |---------------||...............||---------------| | prev || new || old | |---------------||...............||---------------|
In this case, the old VMA is removed and 'prev' is expanded and replaces it.
Since copy_vma_and_data() which calls copy_vma() intends to reference the old VMA after the merge, it must have this pointer updated.
This kind of self-merge is not possible with a succeeding merge, as the merge always prefers to expand the preceding VMA if possible.
copy_vma() accounts for this by explicitly checking to see if a self-merge occurred and updating the vmap pointer if so. However it incorrect did so even for a subsequent merge (this is simply a noop so it had no impact).
So change this to only check for the case which matters - a backwards merge - and rearrange the parameters to make it clearer we're doing that - i.e. check new_vma->vm_start < old_vma_start (having already renamed vma_start to old_vma_start to make it clear this is the previous VMA).
Also update the existing wall-of-text comment to be a lot clearer.
While we're here, replace the VM_BUG_ON_VMA() with a VM_WARN_ON_ONCE_VMA() and update the VMA userland tests accordingly.
No functional change intended.
--- mm/vma.c | 35 ++++++++++++++++------------------- tools/testing/vma/vma_internal.h | 1 + 2 files changed, 17 insertions(+), 19 deletions(-)
mm/vma.c +16 -19
@@ -1911,10 +1911,10 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, bool *need_rmap_locks) {struct vm_area_struct *vma = *vmap; -unsigned long vma_start = vma->vm_start; +unsigned long old_vma_start = vma->vm_start;struct mm_struct *mm = vma->vm_mm;struct vm_area_struct *new_vma; -bool faulted_in_anon_vma = true; +bool can_self_merge = false;VMA_ITERATOR (vmi, mm, addr);VMG_VMA_STATE (vmg, &vmi,NULL , vma, addr, addr + len);
@@ -1924,7 +1924,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,*/ if (unlikely (vma_is_anonymous (vma) && !vma->anon_vma)) { pgoff = addr >>PAGE_SHIFT ; - faulted_in_anon_vma = false; + can_self_merge = true; }/*
@@ -1944,24 +1944,21 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, new_vma =vma_merge_copied_range (&vmg);if (new_vma) { -/* -* Source vma may have been merged into new_vma -*/ -if (unlikely (vma_start >= new_vma->vm_start && - vma_start < new_vma->vm_end)) { +/* Self-merged and VMA replaced. */ +if (unlikely (new_vma->vm_start < old_vma_start && + new_vma->vm_end > old_vma_start)) {/* -* The only way we can get a vma_merge with -* self during an mremap is if the vma hasn't -* been faulted in yet and we were allowed to -* reset the dst vma->vm_pgoff to the -* destination address of the mremap to allow -* the merge to happen. mremap must change the -* vm_pgoff linearity between src and dst vmas -* (in turn preventing a vma_merge) to be -* safe. It is only safe to keep the vm_pgoff -* linear if there are no pages mapped yet. +* The only way a VMA can both self-merge and be +* replaced is if the remap places the new VMA +* immediately prior to its old self ('next') and +* immediately after another VMA ('prev') causing the +* next to be removed and prev to be expanded to cover +* the entire range. +* +* This should only be possible if the page offset was +* updated, i.e. the VMA is unfaulted. */ -VM_BUG_ON_VMA (faulted_in_anon_vma, new_vma); +VM_WARN_ON_ONCE_VMA (!can_self_merge, new_vma); *vmap = vma = new_vma; } *need_rmap_locks =
tools/testing/vma/vma_internal.h +1 -0
@@ -53,6 +53,7 @@ typedef __bitwise unsigned int vm_fault_t;#define VM_WARN_ON (_expr) (WARN_ON(_expr))#define VM_WARN_ON_ONCE (_expr) (WARN_ON_ONCE(_expr)) +#define VM_WARN_ON_ONCE_VMA (_expr, _vma) (WARN_ON_ONCE(_expr))#define VM_WARN_ON_VMG (_expr, _vmg) (WARN_ON(_expr))#define VM_BUG_ON (_expr) (BUG_ON(_expr))#define VM_BUG_ON_VMA (_expr, _vma) (BUG_ON(_expr))
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 8/16 tools/testing/vma: add tests for copy_vma() self-merge
In reply to Lorenzo Stoakes (ARM)Assert that a VMA can be moved backwards, forwards and between a preceding VMA and its old self.
In the cases in which the VMA merges only with itself expect that to be achieved by expanding its old self, so assert that these function correctly.
However in the case of a merge between a preceding VMA and itself the original VMA is removed, so assert that the preceding VMA replaces the one passed in as vmap and the merge is as expected.
--- tools/testing/vma/tests/vma.c | 46 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
tools/testing/vma/tests/vma.c +45 -1
@@ -33,7 +33,51 @@ static bool test_copy_vma(void)struct mm_struct mm = {};bool need_locks = false;VMA_ITERATOR (vmi, &mm,0 ); -struct vm_area_struct *vma, *vma_new, *vma_next; +struct vm_area_struct *vma, *vma_prev, *vma_new, *vma_next, *vma_orig; + +/* Move forwards, adjacent to old self - self-merge. */ + + vma =alloc_and_link_vma (&mm,0x1000 ,0x2000 ,1 , vma_flags); +vma_set_anonymous (vma); + vma_orig = vma; + vma_new =copy_vma (&vma,0x2000 ,0x1000 ,1 , &need_locks); +ASSERT_EQ (vma_new, vma_orig); +ASSERT_EQ (vma, vma_orig); +ASSERT_EQ (vma_new->vm_start,0x1000 ); +ASSERT_EQ (vma_new->vm_end,0x3000 ); + +cleanup_mm (&mm, &vmi); + +/* Move backwards, adjacent to old self - self-merge. */ + + vma =alloc_and_link_vma (&mm,0x2000 ,0x3000 ,2 , vma_flags); +vma_set_anonymous (vma); + vma_orig = vma; + vma_new =copy_vma (&vma,0x1000 ,0x1000 ,2 , &need_locks); +ASSERT_EQ (vma_new, vma_orig); +ASSERT_EQ (vma, vma_orig); +ASSERT_EQ (vma_new->vm_start,0x1000 ); +ASSERT_EQ (vma_new->vm_end,0x3000 ); + +cleanup_mm (&mm, &vmi); + +/* +* Move backwards between prior VMA and old self - self-merge and vma +* updated to a new VMA. +*/ + + vma_prev =alloc_and_link_vma (&mm,0x1000 ,0x2000 ,1 , vma_flags); +vma_set_anonymous (vma_prev); + vma =alloc_and_link_vma (&mm,0x3000 ,0x4000 ,3 , vma_flags); +vma_set_anonymous (vma); + vma_orig = vma; + vma_new =copy_vma (&vma,0x2000 ,0x1000 ,3 , &need_locks); +ASSERT_NE (vma_new, vma_orig); +ASSERT_EQ (vma_new, vma); +ASSERT_EQ (vma_new->vm_start,0x1000 ); +ASSERT_EQ (vma_new->vm_end,0x4000 ); + +cleanup_mm (&mm, &vmi);/* Move backwards and do not merge. */
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 9/16 mm: propagate VMA anonymous page offset on map, remap, split + merge
In reply to Lorenzo Stoakes (ARM)We must correctly update VMA anonymous page offset state on all VMA operations that would result in it changing, with special attention given to remapping.
We cover most cases by simply updating vma_set_range() to do so (with a new anonymous page offset parameter), but also notably must update the merging and mapping logic to propagate this parameter correctly.
The remap logic remains the same - we may update the anonymous page offset if the VMA is unfaulted, but now this applies to MAP_PRIVATE file-backed mappings too, so we update the code to reflect this.
Note that we use __linear_anon_page_index() upon remap as the VMA may be shared, in order that we update the field consistently regardless of VMA type.
Similarly, pass through anon page offset to the merge logic, updating the vma_merge_struct struct to propagate it, and also use __linear_anon_page_index() to obtain the anonymous page index so it can be safely used for both shared and MAP_PRIVATE file-backed mappings.
In copy_vma(), the anonymous page offset is updated regardless of whether the mapping is a CoW mapping or not. This is both to keep the anonymous page offset consistent even for non-CoW mappings (it is set so should at least remain correct) and makes the logic cleaner.
A self-merge however remains permitted only for mappings which can have a populated vma->anon_vma and do not require alignment on a separate file offset - that is pure anonymous VMAs, so only set can_self_merge if vma_is_anonymous().
Finally, we update insert_vm_struct() to correctly set the anonymous page offset on insertion of a VMA.
We simply ensure state is correctly propagated here, so no functional changes are intended.
Also update VMA userland tests to reflect this change.
--- mm/mremap.c | 6 ++-- mm/vma.c | 52 ++++++++++++++++++---------- mm/vma.h | 75 ++++++++++++++++++++++++----------------- mm/vma_exec.c | 2 +- tools/testing/vma/shared.c | 3 +- tools/testing/vma/tests/merge.c | 4 ++- tools/testing/vma/tests/vma.c | 10 +++--- 7 files changed, 95 insertions(+), 57 deletions(-)
mm/mremap.c +4 -2
@@ -1265,7 +1265,9 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)static int copy_vma_and_data (struct vma_remap_struct *vrm,struct vm_area_struct **new_vma_ptr) { -const unsigned long new_pgoff =linear_page_index (vrm->vma, vrm->addr); +const pgoff_t new_pgoff =linear_page_index (vrm->vma, vrm->addr); +const pgoff_t new_anon_pgoff = +__linear_anon_page_index (vrm->vma, vrm->addr);struct vm_area_struct *vma = vrm->vma;struct vm_area_struct *new_vma;unsigned long moved_len;
@@ -1273,7 +1275,7 @@ static int copy_vma_and_data(struct vma_remap_struct *vrm,PAGETABLE_MOVE (pmc,NULL ,NULL , vrm->addr, vrm->new_addr, vrm->old_len); new_vma =copy_vma (&vma, vrm->new_addr, vrm->new_len, new_pgoff, - &pmc.need_rmap_locks); + new_anon_pgoff, &pmc.need_rmap_locks);if (!new_vma) {vrm_uncharge (vrm); *new_vma_ptr =NULL ;
mm/vma.c +35 -17
194 lines not on this page · raw message
mm/vma.h +45 -30
125 lines not on this page · raw message
mm/vma_exec.c +1 -1
@@ -41,7 +41,7 @@ int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift)unsigned long new_end = old_end - shift;VMA_ITERATOR (vmi, mm, new_start);VMG_STATE (vmg, mm, &vmi, new_start, old_end,EMPTY_VMA_FLAGS , -vma_start_pgoff (vma)); +vma_start_pgoff (vma),vma_start_anon_pgoff (vma));struct vm_area_struct *next;struct mmu_gather tlb;PAGETABLE_MOVE (pmc, vma, vma, old_start, new_start, length);
tools/testing/vma/shared.c +2 -1
@@ -23,7 +23,8 @@ struct vm_area_struct *alloc_vma(struct mm_struct *mm, vma->vm_start = start; vma->vm_end = end; - vma->vm_pgoff = pgoff; +vma_set_pgoff (vma, pgoff); +vma_set_anon_pgoff (vma, start >>PAGE_SHIFT ); vma->flags = vma_flags;vma_assert_detached (vma);
tools/testing/vma/tests/merge.c +3 -1
@@ -45,6 +45,7 @@ void vmg_set_range(struct vma_merge_struct *vmg, unsigned long start, vmg->start = start; vmg->end = end; vmg->pgoff = pgoff; + vmg->anon_pgoff = start >>PAGE_SHIFT ; vmg->vma_flags = vma_flags; vmg->just_expand = false;
@@ -108,6 +109,7 @@ static bool test_simple_merge(void) .end = 0x2000, .vma_flags = vma_flags, .pgoff = 1, + .anon_pgoff = 1, };ASSERT_FALSE (attach_vma (&mm, vma_left));
@@ -1431,7 +1433,7 @@ static bool test_expand_only_mode(void)struct mm_struct mm = {};VMA_ITERATOR (vmi, &mm,0 );struct vm_area_struct *vma_prev, *vma; -VMG_STATE (vmg, &mm, &vmi,0x5000 ,0x9000 , vma_flags,5 ); +VMG_STATE (vmg, &mm, &vmi,0x5000 ,0x9000 , vma_flags,5 ,5 );/* * Place a VMA prior to the one we're expanding so we assert that we do
tools/testing/vma/tests/vma.c +5 -5
@@ -40,7 +40,7 @@ static bool test_copy_vma(void) vma =alloc_and_link_vma (&mm,0x1000 ,0x2000 ,1 , vma_flags);vma_set_anonymous (vma); vma_orig = vma; - vma_new =copy_vma (&vma,0x2000 ,0x1000 ,1 , &need_locks); + vma_new =copy_vma (&vma,0x2000 ,0x1000 ,1 ,1 , &need_locks);ASSERT_EQ (vma_new, vma_orig);ASSERT_EQ (vma, vma_orig);ASSERT_EQ (vma_new->vm_start,0x1000 );
@@ -53,7 +53,7 @@ static bool test_copy_vma(void) vma =alloc_and_link_vma (&mm,0x2000 ,0x3000 ,2 , vma_flags);vma_set_anonymous (vma); vma_orig = vma; - vma_new =copy_vma (&vma,0x1000 ,0x1000 ,2 , &need_locks); + vma_new =copy_vma (&vma,0x1000 ,0x1000 ,2 ,2 , &need_locks);ASSERT_EQ (vma_new, vma_orig);ASSERT_EQ (vma, vma_orig);ASSERT_EQ (vma_new->vm_start,0x1000 );
@@ -71,7 +71,7 @@ static bool test_copy_vma(void) vma =alloc_and_link_vma (&mm,0x3000 ,0x4000 ,3 , vma_flags);vma_set_anonymous (vma); vma_orig = vma; - vma_new =copy_vma (&vma,0x2000 ,0x1000 ,3 , &need_locks); + vma_new =copy_vma (&vma,0x2000 ,0x1000 ,3 ,3 , &need_locks);ASSERT_NE (vma_new, vma_orig);ASSERT_EQ (vma_new, vma);ASSERT_EQ (vma_new->vm_start,0x1000 );
@@ -82,7 +82,7 @@ static bool test_copy_vma(void)/* Move backwards and do not merge. */ vma =alloc_and_link_vma (&mm,0x3000 ,0x5000 ,3 , vma_flags); - vma_new =copy_vma (&vma,0 ,0x2000 ,0 , &need_locks); + vma_new =copy_vma (&vma,0 ,0x2000 ,0 ,3 , &need_locks);ASSERT_NE (vma_new, vma);ASSERT_EQ (vma_new->vm_start,0 );ASSERT_EQ (vma_new->vm_end,0x2000 );
@@ -95,7 +95,7 @@ static bool test_copy_vma(void) vma =alloc_and_link_vma (&mm,0 ,0x2000 ,0 , vma_flags); vma_next =alloc_and_link_vma (&mm,0x6000 ,0x8000 ,6 , vma_flags); - vma_new =copy_vma (&vma,0x4000 ,0x2000 ,4 , &need_locks); + vma_new =copy_vma (&vma,0x4000 ,0x2000 ,4 ,4 , &need_locks);vma_assert_attached (vma_new);ASSERT_EQ (vma_new, vma_next);
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 10/16 mm/rmap: track whether the page VMA mapped pgoff is anonymous
In reply to Lorenzo Stoakes (ARM)Update the page_vma_mapped_walk structure to track whether the pgoff being tracked is an anonymous pgoff or not and update the comments to reflect this.
This is necessary in order to determine the correct VMA page offset in vma_address_end() when pvmw->nr_pages > 1.
Also document that pvmw->pgoff is meaningless for pvmw->nr_pages == 1 and for KSM.
Do not set this field where pgoff is not specified.
This is laying the groundwork for eventually using anonymous page offsets as the index for all anonymous folios.
No functional change intended.
--- include/linux/rmap.h | 4 +++- mm/rmap.c | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-)
include/linux/rmap.h +3 -1
@@ -864,13 +864,14 @@ struct page *make_device_exclusive(struct mm_struct *mm, unsigned long addr,struct page_vma_mapped_walk {unsigned long pfn;unsigned long nr_pages; -pgoff_t pgoff; +pgoff_t pgoff;/* Only meaningful if nr_pages > 1 and not a KSM walk */ struct vm_area_struct *vma;unsigned long address;pmd_t *pmd;pte_t *pte;spinlock_t *ptl;unsigned int flags; +bool pgoff_is_anon :1 ; };#define DEFINE_FOLIO_VMA_WALK (name, _folio, _vma, _address, _flags) \
@@ -881,6 +882,7 @@ struct page_vma_mapped_walk { .vma = _vma, \ .address = _address, \ .flags = _flags, \ + .pgoff_is_anon = folio_test_anon(_folio), \ }static inline void page_vma_mapped_walk_done (struct page_vma_mapped_walk *pvmw)
mm/rmap.c +2 -0
@@ -1240,6 +1240,7 @@ static bool mapping_wrprotect_range_one(struct folio *folio, .vma = vma, .address = address, .flags = PVMW_SYNC, + .pgoff_is_anon = false, }; state->cleaned +=page_vma_mkclean_one (&pvmw);
@@ -1317,6 +1318,7 @@ int pfn_mkclean_range(unsigned long pfn, unsigned long nr_pages, pgoff_t pgoff, .pgoff = pgoff, .vma = vma, .flags = PVMW_SYNC, + .pgoff_is_anon = false, };if (invalid_mkclean_vma (vma,NULL ))
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 11/16 mm: clean up vma_address_end()
In reply to Lorenzo Stoakes (ARM)vma_address_end() is a confusing function with a lot of moving parts so clean it up prior to extending it for anon page indexed mappings.
Const-ify some variables and establish pgoff_vma_start and pgoff_end variables to clearly identify the page offset for the start of the VMA and the end of the page offset range specified by the page walk.
This simplifies the function significantly and lays the groundwork for a future change to update this function to account for anonymously page indexed folios.
No functional change intended.
--- mm/internal.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
mm/internal.h +7 -5
@@ -1079,22 +1079,24 @@ static inline unsigned long vma_anon_address(const struct vm_area_struct *vma, }/* -* Then at what user virtual address will none of the range be found in vma? +* At what user virtual address will none of the range be found in vma? * Assumes that vma_address() already returned a good starting address. */ static inline unsigned long vma_address_end (struct page_vma_mapped_walk *pvmw) { -struct vm_area_struct *vma = pvmw->vma; -pgoff_t pgoff; +const pgoff_t pgoff_end = pvmw->pgoff + pvmw->nr_pages; +const struct vm_area_struct *vma = pvmw->vma; +pgoff_t pgoff_vma_start;unsigned long address;/* Common case, plus ->pgoff is invalid for KSM */ if (pvmw->nr_pages ==1 )return pvmw->address +PAGE_SIZE ; - pgoff = pvmw->pgoff + pvmw->nr_pages; + pgoff_vma_start =vma_start_pgoff (vma); + address = vma->vm_start + - ((pgoff -vma_start_pgoff (vma)) <<PAGE_SHIFT ); + ((pgoff_end - pgoff_vma_start) <<PAGE_SHIFT );/* Check for address beyond vma (or wrapped through 0?) */ if (address < vma->vm_start || address > vma->vm_end) address = vma->vm_end;
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 12/16 mm/huge_memory: update remove_migration_pmd() to accept a folio
In reply to Lorenzo Stoakes (ARM)This function does not need to accept a page and requiring it to is unnecessary and misleading.
make_[writable, readable]_device_private_entry() must be passed a PMD-aligned PFN as they immediately used to obtain a softleaf PMD entry and the same argument applies to folio_add_[anon, file]_rmap_pmd().
While we are here, update a VM_BUG_ON() to a VM_WARN_ON_ONCE().
No functional change intended.
--- include/linux/swapops.h | 6 +++--- mm/huge_memory.c | 16 +++++++--------- mm/migrate.c | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-)
include/linux/swapops.h +3 -3
@@ -325,8 +325,8 @@ struct page_vma_mapped_walk;extern int set_pmd_migration_entry (struct page_vma_mapped_walk *pvmw,struct page *page); -extern void remove_migration_pmd (struct page_vma_mapped_walk *pvmw, -struct page *new); +void remove_migration_pmd (struct page_vma_mapped_walk *pvmw, +struct folio *folio);extern void pmd_migration_entry_wait (struct mm_struct *mm,pmd_t *pmd);
@@ -346,7 +346,7 @@ static inline int set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw, }static inline void remove_migration_pmd (struct page_vma_mapped_walk *pvmw, -struct page *new) +struct folio *folio) {BUILD_BUG (); }
mm/huge_memory.c +7 -9
@@ -5020,9 +5020,8 @@ int set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,return 0 ; } -void remove_migration_pmd (struct page_vma_mapped_walk *pvmw,struct page *new) +void remove_migration_pmd (struct page_vma_mapped_walk *pvmw,struct folio *folio) { -struct folio *folio =page_folio (new);struct vm_area_struct *vma = pvmw->vma;struct mm_struct *mm = vma->vm_mm;unsigned long address = pvmw->address;
@@ -5058,11 +5057,9 @@ void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)swp_entry_t entry;if (pmd_write (pmde)) - entry =make_writable_device_private_entry ( -page_to_pfn (new)); + entry =make_writable_device_private_entry (folio_pfn (folio));else - entry =make_readable_device_private_entry ( -page_to_pfn (new)); + entry =make_readable_device_private_entry (folio_pfn (folio)); pmde =softleaf_to_pmd (entry);if (pmd_swp_soft_dirty (*pvmw->pmd))
@@ -5077,11 +5074,12 @@ void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)if (!softleaf_is_migration_read (entry)) rmap_flags |=RMAP_EXCLUSIVE ; -folio_add_anon_rmap_pmd (folio, new, vma, haddr, rmap_flags); +folio_add_anon_rmap_pmd (folio, &folio->page, vma, haddr, rmap_flags); }else { -folio_add_file_rmap_pmd (folio, new, vma); +folio_add_file_rmap_pmd (folio, &folio->page, vma); } -VM_BUG_ON (pmd_write (pmde) &&folio_test_anon (folio) && !PageAnonExclusive (new)); +VM_WARN_ON_ONCE (pmd_write (pmde) &&folio_test_anon (folio) && + !PageAnonExclusive (&folio->page));set_pmd_at (mm, haddr, pvmw->pmd, pmde);/* No need to invalidate - it was non-present before */
mm/migrate.c +1 -1
@@ -372,7 +372,7 @@ static bool remove_migration_pte(struct folio *folio,if (!pvmw.pte) {VM_BUG_ON_FOLIO (folio_test_hugetlb (folio) || !folio_test_pmd_mappable (folio), folio); -remove_migration_pmd (&pvmw, new); +remove_migration_pmd (&pvmw, folio);continue ; }#endif
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 13/16 mm/migrate: calculate large folio page index using PFN
In reply to Lorenzo Stoakes (ARM)Rather than having to figure out the page index to use using linear_page_index(), calculate it using PFN.
This is a more natural fit as the linear page index is immaterial to determining the folio page index.
Derive the page index from the offset between migration entry PFN and folio PFN - pvmw.pfn (set via DEFINE_FOLIO_VMA_WALK() which uses folio_pfn() to obtain it).
Additionally remove a not so useful comment and clean the code layout up.
No functional change intended.
--- mm/migrate.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
mm/migrate.c +9 -10
@@ -356,16 +356,11 @@ static bool remove_migration_pte(struct folio *folio,while (page_vma_mapped_walk (&pvmw)) {rmap_t rmap_flags =RMAP_NONE ; -pte_t old_pte; -pte_t pte; +unsigned long idx =0 ;softleaf_t entry;struct page *new; -unsigned long idx =0 ; - -/* pgoff is invalid for ksm pages, but they are never large */ -if (folio_test_large (folio) && !folio_test_hugetlb (folio)) - idx =linear_page_index (vma, pvmw.address) - pvmw.pgoff; - new =folio_page (folio, idx); +pte_t old_pte; +pte_t pte;#ifdef CONFIG_ARCH_HAS_PMD_SOFTLEAVES /* PMD-mapped THP migration entry */
@@ -381,14 +376,18 @@ static bool remove_migration_pte(struct folio *folio, pvmw.pte); else old_pte =ptep_get (pvmw.pte); + + entry =softleaf_from_pte (old_pte); +if (folio_test_large (folio) && !folio_test_hugetlb (folio)) + idx =softleaf_to_pfn (entry) - pvmw.pfn; +if (rmap_walk_arg->map_unused_to_zeropage &&try_to_map_unused_to_zeropage (&pvmw, folio, old_pte, idx))continue ;folio_get (folio); + new =folio_page (folio, idx); pte =mk_pte (new,READ_ONCE (vma->vm_page_prot)); - - entry =softleaf_from_pte (old_pte);if (!softleaf_is_migration_young (entry)) pte =pte_mkold (pte);if (folio_test_dirty (folio) &&softleaf_is_migration_dirty (entry))
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 14/16 mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios
In reply to Lorenzo Stoakes (ARM)Currently anonymous folios belonging to CoW'd MAP_PRIVATE file-backed mappings are indexed by their page offset within the file in which they were originally mapped.
This differs from anonymous folios belonging to pure anon mappings which are indexed by their anonymous page offset (the address at which they'd belong in the VMA when first faulted).
This change fixes this inconsistency, always indexing anonymous folios by their anonymous page offset regardless of the VMA to which they belong.
The foundations have been laid such that we need only switch this functionality on such by:
* Using linear_anon_page_index() in __folio_set_anon() to assign the folio's index to the anonymous linear index rather than the file-backed one.
* Otherwise using linear_anon_page_index() in all instances where anonymous folios are being referenced or manipulated.
* Replacing vma_address() with vma_filebacked_address() or vma_anon_address() as appropriate.
* Updating the merging logic to check that anonymous page offsets are aligned as well as filebacked ones for MAP_PRIVATE file-backed VMAs, introducing needs_adjacent_anon_pgoff() to figure out when this is required.
* Updating linear_folio_page_index() to invoke linear_anon_page_index() if the folio is anonymous.
* Updating vma_address_end() to use the VMA's anonymous page offset when pvmw->pgoff is anonymous.
* Correcting folio_within_range() to use anonymous page offset for anonymous folios.
This will have no impact on merging of anonymous VMAs, whose page offset and anonymous page offset are identical, nor will it impact shared file-backed VMAs, which will continue to be merged based on the file-backed page offset.
However, MAP_PRIVATE file-backed mappings must now be aligned on anonymous page offset as well.
In most instances this should have no impact on merging of file-backed mappings, which are usually not merged all that often, let alone MAP_PRIVATE mapped ones, and rarely remapped and faulted before being moved back in place (the case in which a merge may now fail).
One subtle impact of this change is in NUMA interleaving - since commit 88c91dc58582 ("mempolicy: migration attempt to match interleave nodes"), migration heuristically tries to maintain interleaving behaviour matching the policy using folio indices.
When doing migration of CoW'd MAP_PRIVATE-file backed ranges, the 'base' upon which the interleaving behaviour is performed will vary for these ranges. However the commit notes that ranges spanning multiple VMAs will already cause varying bases, and that this is an acceptable approximation.
It is very unlikely real world use-cases will be impacted by this (MAP_PRIVATE file-backed mappings are already an edge case), and all that will happen is that such ranges will cause interleaving to be rotated over the CoW'd range, with little to no impact.
This commit lays the foundations for future scalable CoW work which needs to track some remaps, meaning that most remap tracking can be avoided, and in nearly all cases the anonymous page offset will be able to be used to quickly find the VMA in an mm.
Note that the need_rmap_locks check doesn't need to be updated, as any remapping will offset both the anonymous and file-backed page offset, so it suffices to check only one.
--- mm/huge_memory.c | 2 +- mm/internal.h | 27 ++++++++------------------- mm/interval_tree.c | 4 ++-- mm/ksm.c | 6 +++--- mm/page_vma_mapped.c | 2 +- mm/rmap.c | 12 ++++++------ mm/userfaultfd.c | 4 ++-- mm/vma.c | 32 +++++++++++++++++++++++++++++++- 8 files changed, 54 insertions(+), 35 deletions(-)
mm/huge_memory.c +1 -1
@@ -2887,7 +2887,7 @@ int move_pages_huge_pmd(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd, pm }folio_move_anon_rmap (src_folio, dst_vma); - src_folio->index =linear_page_index (dst_vma, dst_addr); + src_folio->index =linear_anon_page_index (dst_vma, dst_addr); _dst_pmd =folio_mk_pmd (src_folio, dst_vma->vm_page_prot);/* Follow mremap() behavior and treat the entry dirty after the move */
mm/internal.h +8 -19
@@ -933,7 +933,8 @@ folio_within_range(struct folio *folio, struct vm_area_struct *vma,return false; pgoff_folio =folio_pgoff (folio); - pgoff_vma_start =vma_start_pgoff (vma); + pgoff_vma_start =folio_test_anon (folio) ? +vma_start_anon_pgoff (vma) :vma_start_pgoff (vma);if (start < vma->vm_start) start = vma->vm_start;
@@ -1044,23 +1045,8 @@ static inline unsigned long vma_filebacked_address(const struct vm_area_struct * }/** -* vma_address - Find the virtual address a page range is mapped at. -* @vma: The vma which maps this object. -* @pgoff: The page offset within its object. -* @nr_pages: The number of pages to consider. -* -* If any page in this range is mapped by this VMA, return the first address -* where any of these pages appear. Otherwise, return -EFAULT. -*/ -static inline unsigned long vma_address (const struct vm_area_struct *vma, -pgoff_t pgoff,unsigned long nr_pages) -{ -return __vma_address (vma, pgoff,vma_start_pgoff (vma), nr_pages); -} - -/** -* vma_anon_address - Find the address an anonymous folio with index @pgoff_anon -* is mapped at. +* vma_anon_address - Find the virtual address an anonymous page range is mapped +* at. * @vma: The vma which maps this object. * @pgoff_anon: The anonymous page index belonging to the folio. * @nr_pages: The number of pages to consider.
@@ -1093,7 +1079,10 @@ static inline unsigned long vma_address_end(struct page_vma_mapped_walk *pvmw)if (pvmw->nr_pages ==1 )return pvmw->address +PAGE_SIZE ; - pgoff_vma_start =vma_start_pgoff (vma); +if (pvmw->pgoff_is_anon) + pgoff_vma_start =vma_start_anon_pgoff (vma); +else + pgoff_vma_start =vma_start_pgoff (vma); address = vma->vm_start + ((pgoff_end - pgoff_vma_start) <<PAGE_SHIFT );
mm/interval_tree.c +2 -2
@@ -83,12 +83,12 @@ mapping_rmap_tree_iter_next(struct vm_area_struct *vma,static pgoff_t avc_start_pgoff (struct anon_vma_chain *avc) { -return vma_start_pgoff (avc->vma); +return vma_start_anon_pgoff (avc->vma); }static pgoff_t avc_last_pgoff (struct anon_vma_chain *avc) { -return vma_last_pgoff (avc->vma); +return vma_last_anon_pgoff (avc->vma); } INTERVAL_TREE_DEFINE(struct anon_vma_chain, rb, pgoff_t, rb_subtree_last,
mm/ksm.c +3 -3
@@ -1625,7 +1625,7 @@ static int try_to_merge_with_ksm_page(struct ksm_rmap_item *rmap_item,* stable_tree, break_cow() will clean it up. */ rmap_item->anon_vma = vma->anon_vma; - rmap_item->linear_page_index =linear_page_index (vma, rmap_item->address); + rmap_item->linear_page_index =linear_anon_page_index (vma, rmap_item->address);get_anon_vma (vma->anon_vma); out:mmap_read_unlock (mm);
@@ -3152,7 +3152,7 @@ struct folio *ksm_might_need_to_copy(struct folio *folio,return folio;/* no need to copy it */ }else if (!anon_vma) {return folio;/* no need to copy it */ - }else if (folio->index ==linear_page_index (vma, addr) && + }else if (folio->index ==linear_anon_page_index (vma, addr) && anon_vma->root == vma->anon_vma->root) {return folio;/* still no need to copy it */ }
@@ -3222,7 +3222,7 @@ void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc)/* * Currently, KSM folios are always small folios, so it's * sufficient to search for a single page. We can simply use -* the linear_page_index of the original de-duplicate +* the linear_anon_page_index of the original de-duplicate * anonymous page that we remembered in the rmap_item while * de-duplicating. Note that mremap() always de-duplicates KSM * folios: so if there was mremap() in our parent or our child,
mm/page_vma_mapped.c +1 -1
@@ -365,7 +365,7 @@ unsigned long page_mapped_in_vma(const struct page *page, };if (folio_test_anon (folio)) - pvmw.address =vma_address (vma, pgoff,1 ); + pvmw.address =vma_anon_address (vma, pgoff,1 );else pvmw.address =vma_filebacked_address (vma, pgoff,1 );if (pvmw.address == -EFAULT )
mm/rmap.c +6 -6
@@ -866,7 +866,7 @@ unsigned long page_address_in_vma(const struct folio *folio, vma->anon_vma->root != anon_vma->root)return -EFAULT ;/* KSM folios don't reach here because of the !anon_vma check */ -return vma_address (vma,page_pgoff (folio, page),1 ); +return vma_anon_address (vma,page_pgoff (folio, page),1 ); }else if (!vma->vm_file) {return -EFAULT ; }else if (vma->vm_file->f_mapping != folio->mapping) {
@@ -1485,7 +1485,7 @@ static void __folio_set_anon(struct folio *folio, struct vm_area_struct *vma,*/ anon_vma = (void *) anon_vma +FOLIO_MAPPING_ANON ;WRITE_ONCE (folio->mapping, (struct address_space *) anon_vma); - folio->index =linear_page_index (vma, address); + folio->index =linear_anon_page_index (vma, address); }/**
@@ -1512,8 +1512,8 @@ static void __page_check_anon_rmap(const struct folio *folio,*/ VM_BUG_ON_FOLIO (folio_anon_vma (folio)->root != vma->anon_vma->root, folio); -VM_BUG_ON_PAGE (page_pgoff (folio, page) !=linear_page_index (vma, address), - page); +VM_BUG_ON_PAGE (page_pgoff (folio, page) != +linear_anon_page_index (vma, address), page); } static __always_inline void __folio_add_anon_rmap(struct folio *folio,
@@ -3040,10 +3040,10 @@ static void rmap_walk_anon(struct folio *folio, pgoff_end = pgoff_start +folio_nr_pages (folio) -1 ; anon_rmap_tree_foreach(avc, anon_vma, pgoff_start, pgoff_end) {struct vm_area_struct *vma = avc->vma; -unsigned long address =vma_address (vma, pgoff_start, +const unsigned long address =vma_anon_address (vma, pgoff_start,folio_nr_pages (folio)); -VM_BUG_ON_VMA (address == -EFAULT , vma); +VM_WARN_ON_ONCE_VMA (address == -EFAULT , vma);cond_resched ();if (rwc->invalid_vma && rwc->invalid_vma (vma, rwc->arg))
mm/userfaultfd.c +2 -2
@@ -1352,7 +1352,7 @@ static long move_present_ptes(struct mm_struct *mm, }folio_move_anon_rmap (src_folio, dst_vma); - src_folio->index =linear_page_index (dst_vma, dst_addr); + src_folio->index =linear_anon_page_index (dst_vma, dst_addr); orig_dst_pte =folio_mk_pte (src_folio, dst_vma->vm_page_prot);/* Set soft dirty bit so userspace can notice the pte was moved */
@@ -1428,7 +1428,7 @@ static int move_swap_pte(struct mm_struct *mm, struct vm_area_struct *dst_vma,*/ if (src_folio) {folio_move_anon_rmap (src_folio, dst_vma); - src_folio->index =linear_page_index (dst_vma, dst_addr); + src_folio->index =linear_anon_page_index (dst_vma, dst_addr); }else {/* * Check if the swap entry is cached after acquiring the src_pte
mm/vma.c +31 -1
@@ -204,6 +204,25 @@ static void init_multi_vma_prep(struct vma_prepare *vp, vp->skip_vma_uprobe = true; } +/* +* Does this merge require that adjacent VMAs must have adjacent anonymous page +* offsets in addition to having adjacent vma->vm_pgoff? +* +* This is only required for MAP_PRIVATE-file backed mappings as the page offset +* for pure anonymous VMAs is equal to the anonymous page offset. +* +* Read-only shared mappings (with VMA_SHARED_BIT cleared) are always unfaulted +* so automatically have correct anonymous page offset (as it is always updated +* on remap). +* +* 'Special' mappings in the sense of VDSO, VVAR etc. have !file but would in +* any case not be candidates for merge nor be mergeable. +*/ +static bool needs_adjacent_anon_pgoff (const struct vma_merge_struct *vmg) +{ +return vmg->file &&vma_flags_is_cow_mapping (&vmg->vma_flags); +} +/* * Return true if we can merge this (vma_flags,anon_vma,file,vm_pgoff) * in front of (at a lower virtual address and file offset than) the vma.
@@ -225,6 +244,9 @@ static bool can_vma_merge_before(struct vma_merge_struct *vmg)return false;if (vmg_end_pgoff (vmg) !=vma_start_pgoff (vmg->next))return false; +if (needs_adjacent_anon_pgoff (vmg) && +vmg_end_anon_pgoff (vmg) !=vma_start_anon_pgoff (vmg->next)) +return false;return true; }
@@ -245,6 +267,9 @@ static bool can_vma_merge_after(struct vma_merge_struct *vmg)return false;if (vma_end_pgoff (vmg->prev) !=vmg_start_pgoff (vmg))return false; +if (needs_adjacent_anon_pgoff (vmg) && +vma_end_anon_pgoff (vmg->prev) !=vmg_start_anon_pgoff (vmg)) +return false;return true; }
@@ -2048,7 +2073,12 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *if (!vma_flags_empty (&diff))return false;/* Page offset must align. */ -return vma_end_pgoff (a) ==vma_start_pgoff (b); +if (vma_end_pgoff (a) !=vma_start_pgoff (b)) +return false; +/* Only reached from anon path, so either MAP_PRIVATE file or anon. */ +if (vma_end_anon_pgoff (a) !=vma_start_anon_pgoff (b)) +return false; +return true; }/*
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 15/16 tools/testing/vma: expand VMA merge tests to assert anon pgoff
In reply to Lorenzo Stoakes (ARM)Now we have introduced the VMA anonymous page offset attribute and update it when VMAs are manipulated, update VMA merge tests to assert that the anonymous page offset is as expected.
Also update instances where we could use vma_start_pgoff() to do so.
--- tools/testing/vma/tests/merge.c | 45 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-)
tools/testing/vma/tests/merge.c +36 -9
@@ -121,6 +121,7 @@ static bool test_simple_merge(void)ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x3000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );ASSERT_FLAGS_SAME_MASK (&vma->flags, vma_flags);detach_free_vma (vma);
@@ -153,6 +154,7 @@ static bool test_simple_modify(void)ASSERT_EQ (vma->vm_start,0x1000 );ASSERT_EQ (vma->vm_end,0x2000 );ASSERT_EQ (vma_start_pgoff (vma),1 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),1 );/* * Now walk through the three split VMAs and make sure they are as
@@ -165,6 +167,7 @@ static bool test_simple_modify(void)ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x1000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );detach_free_vma (vma);vma_iter_clear (&vmi);
@@ -174,6 +177,7 @@ static bool test_simple_modify(void)ASSERT_EQ (vma->vm_start,0x1000 );ASSERT_EQ (vma->vm_end,0x2000 );ASSERT_EQ (vma_start_pgoff (vma),1 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),1 );detach_free_vma (vma);vma_iter_clear (&vmi);
@@ -183,6 +187,7 @@ static bool test_simple_modify(void)ASSERT_EQ (vma->vm_start,0x2000 );ASSERT_EQ (vma->vm_end,0x3000 );ASSERT_EQ (vma_start_pgoff (vma),2 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),2 );detach_free_vma (vma);mtree_destroy (&mm.mm_mt);
@@ -212,6 +217,7 @@ static bool test_simple_expand(void)ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x3000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );detach_free_vma (vma);mtree_destroy (&mm.mm_mt);
@@ -234,6 +240,7 @@ static bool test_simple_shrink(void)ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x1000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );detach_free_vma (vma);mtree_destroy (&mm.mm_mt);
@@ -346,6 +353,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x5000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );ASSERT_EQ (vma->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,3 );
@@ -367,6 +375,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,ASSERT_EQ (vma->vm_start,0x6000 );ASSERT_EQ (vma->vm_end,0x9000 );ASSERT_EQ (vma_start_pgoff (vma),6 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),6 );ASSERT_EQ (vma->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,3 );
@@ -387,6 +396,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x9000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );ASSERT_EQ (vma->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,2 );
@@ -407,6 +417,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,ASSERT_EQ (vma->vm_start,0xa000 );ASSERT_EQ (vma->vm_end,0xc000 );ASSERT_EQ (vma_start_pgoff (vma),0xa ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0xa );ASSERT_EQ (vma->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,2 );
@@ -426,6 +437,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0xc000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );ASSERT_EQ (vma->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,1 );
@@ -446,6 +458,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0xc000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );ASSERT_EQ (vma->anon_vma, &dummy_anon_vma);detach_free_vma (vma);
@@ -642,7 +655,8 @@ static bool test_vma_merge_with_close(void)ASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_prev->vm_start,0 );ASSERT_EQ (vma_prev->vm_end,0x5000 ); -ASSERT_EQ (vma_prev->vm_pgoff,0 ); +ASSERT_EQ (vma_start_pgoff (vma_prev),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_prev),0 );ASSERT_EQ (cleanup_mm (&mm, &vmi),2 );
@@ -753,7 +767,8 @@ static bool test_vma_merge_with_close(void)ASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_prev->vm_start,0 );ASSERT_EQ (vma_prev->vm_end,0x5000 ); -ASSERT_EQ (vma_prev->vm_pgoff,0 ); +ASSERT_EQ (vma_start_pgoff (vma_prev),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_prev),0 );ASSERT_EQ (cleanup_mm (&mm, &vmi),2 );
@@ -808,6 +823,7 @@ static bool test_vma_merge_new_with_close(void)ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x5000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );ASSERT_EQ (vma->vm_ops, &vm_ops);ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,2 );
@@ -863,11 +879,13 @@ static bool __test_merge_existing(bool prev_is_sticky, bool middle_is_sticky, boASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_next->vm_start,0x3000 );ASSERT_EQ (vma_next->vm_end,0x9000 ); -ASSERT_EQ (vma_next->vm_pgoff,3 ); +ASSERT_EQ (vma_start_pgoff (vma_next),3 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_next),3 );ASSERT_EQ (vma_next->anon_vma, &dummy_anon_vma);ASSERT_EQ (vma->vm_start,0x2000 );ASSERT_EQ (vma->vm_end,0x3000 );ASSERT_EQ (vma_start_pgoff (vma),2 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),2 );ASSERT_TRUE (vma_write_started (vma));ASSERT_TRUE (vma_write_started (vma_next));ASSERT_EQ (mm.map_count,2 );
@@ -897,7 +915,8 @@ static bool __test_merge_existing(bool prev_is_sticky, bool middle_is_sticky, boASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_next->vm_start,0x2000 );ASSERT_EQ (vma_next->vm_end,0x9000 ); -ASSERT_EQ (vma_next->vm_pgoff,2 ); +ASSERT_EQ (vma_start_pgoff (vma_next),2 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_next),2 );ASSERT_EQ (vma_next->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma_next));ASSERT_EQ (mm.map_count,1 );
@@ -929,11 +948,13 @@ static bool __test_merge_existing(bool prev_is_sticky, bool middle_is_sticky, boASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_prev->vm_start,0 );ASSERT_EQ (vma_prev->vm_end,0x6000 ); -ASSERT_EQ (vma_prev->vm_pgoff,0 ); +ASSERT_EQ (vma_start_pgoff (vma_prev),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_prev),0 );ASSERT_EQ (vma_prev->anon_vma, &dummy_anon_vma);ASSERT_EQ (vma->vm_start,0x6000 );ASSERT_EQ (vma->vm_end,0x7000 );ASSERT_EQ (vma_start_pgoff (vma),6 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),6 );ASSERT_TRUE (vma_write_started (vma_prev));ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,2 );
@@ -964,7 +985,8 @@ static bool __test_merge_existing(bool prev_is_sticky, bool middle_is_sticky, boASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_prev->vm_start,0 );ASSERT_EQ (vma_prev->vm_end,0x7000 ); -ASSERT_EQ (vma_prev->vm_pgoff,0 ); +ASSERT_EQ (vma_start_pgoff (vma_prev),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_prev),0 );ASSERT_EQ (vma_prev->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma_prev));ASSERT_EQ (mm.map_count,1 );
@@ -996,7 +1018,8 @@ static bool __test_merge_existing(bool prev_is_sticky, bool middle_is_sticky, boASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_prev->vm_start,0 );ASSERT_EQ (vma_prev->vm_end,0x9000 ); -ASSERT_EQ (vma_prev->vm_pgoff,0 ); +ASSERT_EQ (vma_start_pgoff (vma_prev),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_prev),0 );ASSERT_EQ (vma_prev->anon_vma, &dummy_anon_vma);ASSERT_TRUE (vma_write_started (vma_prev));ASSERT_EQ (mm.map_count,1 );
@@ -1126,7 +1149,8 @@ static bool test_anon_vma_non_mergeable(void)ASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_prev->vm_start,0 );ASSERT_EQ (vma_prev->vm_end,0x7000 ); -ASSERT_EQ (vma_prev->vm_pgoff,0 ); +ASSERT_EQ (vma_start_pgoff (vma_prev),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_prev),0 );ASSERT_TRUE (vma_write_started (vma_prev));ASSERT_FALSE (vma_write_started (vma_next));
@@ -1157,7 +1181,8 @@ static bool test_anon_vma_non_mergeable(void)ASSERT_EQ (vmg.state,VMA_MERGE_SUCCESS );ASSERT_EQ (vma_prev->vm_start,0 );ASSERT_EQ (vma_prev->vm_end,0x7000 ); -ASSERT_EQ (vma_prev->vm_pgoff,0 ); +ASSERT_EQ (vma_start_pgoff (vma_prev),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma_prev),0 );ASSERT_TRUE (vma_write_started (vma_prev));ASSERT_FALSE (vma_write_started (vma_next));
@@ -1419,6 +1444,7 @@ static bool test_merge_extend(void)ASSERT_EQ (vma->vm_start,0 );ASSERT_EQ (vma->vm_end,0x4000 );ASSERT_EQ (vma_start_pgoff (vma),0 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),0 );ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (mm.map_count,1 );
@@ -1459,6 +1485,7 @@ static bool test_expand_only_mode(void)ASSERT_EQ (vma->vm_start,0x3000 );ASSERT_EQ (vma->vm_end,0x9000 );ASSERT_EQ (vma_start_pgoff (vma),3 ); +ASSERT_EQ (vma_start_anon_pgoff (vma),3 );ASSERT_TRUE (vma_write_started (vma));ASSERT_EQ (vma_iter_addr (&vmi),0x3000 );vma_assert_attached (vma);
Signature · 1 line
2.55.0
Lorenzo Stoakes (ARM)
maintainerPATCH v5 16/16 tools/testing/selftests/mm: test anonymous page offset merge behaviour
In reply to Lorenzo Stoakes (ARM)While maintaining anonymous page offsets for VMAs has no impact for most merge cases, it does impact MAP_PRIVATE-mapped file-backed mappings which happen to have matching page offset but not matching anonymous page offset.
Assert this behaviour by attempting to map an unfaulted MAP_PRIVATE-memfd region with a faulted one with compatible file page offsets but incompatible anonymous page offsets.
--- tools/testing/selftests/mm/merge.c | 57 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
tools/testing/selftests/mm/merge.c +57 -0
@@ -1305,6 +1305,63 @@ TEST_F(merge, merge_vmas_with_mseal)ASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +2 * page_size); } +TEST_F(merge, anon_and_page_offset_mismatch_memfd) +{ +struct procmap_fd *procmap = &self->procmap; +unsigned int page_size = self->page_size; +char *carveout = self->carveout; +char *ptr, *ptr2; +int fd; + +/* Create a 10 page memfd descriptor. */ + fd =memfd_create ("anon_page_offset_test" ,MFD_CLOEXEC ); +ASSERT_NE (fd,-1 ); +ASSERT_EQ (ftruncate (fd,10 * page_size),0 ); + +/* Map a region using the memfd at page offset 0. */ + ptr =mmap (carveout,5 * page_size,PROT_READ |PROT_WRITE , +MAP_FIXED |MAP_PRIVATE , fd,0 ); +ASSERT_NE (ptr,MAP_FAILED ); + +/* +* Map another separately and trigger a CoW fault at page offset 5: +* +* |-----------| |---------| +* | unfaulted | | faulted | +* |-----------| |---------| +*/ + ptr2 =mmap (&carveout[10 * page_size],5 * page_size, +PROT_READ |PROT_WRITE ,MAP_FIXED |MAP_PRIVATE , + fd,5 * page_size); +ASSERT_NE (ptr2,MAP_FAILED ); + ptr2[0 ] ='x' ; + +/* +* Now move it in place: +* +* |----------| +* | | +* v | +* |-----------| |---------| +* | unfaulted | | faulted | +* |-----------| |---------| +* +* Because the anonymous page offset of the faulted region is now +* &carveout[10 * page_size], despite the two regions being mergeable +* due to file page offset, they are NOT mergeable due to anonymous +* page offset. +*/ + ptr2 =sys_mremap (ptr2,5 * page_size,5 * page_size, +MREMAP_MAYMOVE |MREMAP_FIXED , + &carveout[5 * page_size]); +ASSERT_NE (ptr2,MAP_FAILED ); + +/* Assert that they did not merge. */ +ASSERT_TRUE (find_vma_procmap (procmap, ptr)); +ASSERT_EQ (procmap->query.vma_start, (unsigned long )ptr); +ASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +5 * page_size); +} + TEST_F(merge_with_fork, mremap_faulted_to_unfaulted_prev) {struct procmap_fd *procmap = &self->procmap;
Signature · 1 line
2.55.0
Andrew Morton
maintainerQuoted 17 lines · On Thu, 13 Aug 2026 18:32:17 +0100 "Lorenzo Stoakes (ARM)" <[email protected]>…
In memory management we've managed to manufacture a great deal of confusion around the concept of anonymous memory. We have:
1. 'Pure anon' memory - anonymous VMAs whose folios are anonymous and swap-backed (thus for reclaim purposes, treated as anonymous). These are simple enough.
2. shmem - file-backed VMAs, file-backed folios (from rmap perspective) so present in the page cache and mapped by an
address_spaceobject, but whose folios are also swap-backed (thus treated as anonymous for reclaim purposes).3. MAP_PRIVATE-mapped
/dev/zero- a strange beast whose VMAs havevma->vm_fileset, but which clearsvma->vm_opsto satisfyvma_is_anonymous(), resulting in VMAs that were mmap()'d referencing a file, but are in every other sense anonymous, including the folios.
Quoted 5 lines
4. Other MAP_PRIVATE-file backed mappings - These possess file-backed VMAs and have file-backed folios until CoW'd, at which point those CoW'd folios are anonymous.
This series fixes issue 3.
Thanks. I've updated mm.git's mm-unstable branch to this version. Looking good for the second week of the upcoming merge window.
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
maybe
hp2:/usr/src/linux-next> git log --oneline | grep "mm/vma: introduce VMA anon page offset field and add helpers" 249646a587dc mm/vma: introduce VMA anon page offset field and add helpers
base-commit: 249646a587dc^
But that requires that Sashiko be able to poke around in linux-next from previous days.
Quoted 11 lines
v5:
* Accumulated tags (thanks everybody!)
* Removed the final 4 patches to be handled later as there are nuances with the/dev/zerostuff we need to figure out, as discussed with David.
* Updated the cover letter to reflect this.
* Added comments tovma_flags_is_cow_mapping(), vma_[desc_]is_cow_mapping() as per Suren.
* Correct typo as per Suren.
* Reworded test comment in patch 16 from 'fault in' to 'trigger a CoW fault' as per David.
* Fix -> 75 char limit in patch 12's commit msg.
Here's how v5 altered mm.git. It's rather substantial, but mainly selftests:
drivers/char/mem.c | 8 include/linux/mm.h | 18 - include/linux/pagemap.h | 3 mm/internal.h | 17 - mm/vma.c | 52 ---- mm/vma.h | 3 mm/vma_internal.h | 1 tools/testing/selftests/mm/merge.c | 106 ---------- tools/testing/selftests/proc/proc-self-map-files-001.c | 2 tools/testing/selftests/proc/proc-self-map-files-002.c | 2 tools/testing/vma/include/dup.h | 40 --- tools/testing/vma/tests/mmap.c | 50 ---- 12 files changed, 40 insertions(+), 262 deletions(-)
drivers/char/mem.c renamed from drivers/char/mem.c~b +6 -2
@@ -506,7 +506,11 @@ static int mmap_zero_prepare(struct vm_aif (vma_desc_test (desc,VMA_SHARED_BIT ))return shmem_zero_setup_desc (desc); -/* MAP_PRIVATE semantics are taken care for us by core mm. */ +/* +* This is a highly unique situation where we mark a MAP_PRIVATE mapping +* of /dev/zero anonymous, despite it not being. +*/ +vma_desc_set_anonymous (desc);return 0 ; }
@@ -694,7 +698,7 @@ static const struct memdev { #ifdef CONFIG_DEVPORT [4 ] = {"port" , &port_fops,0 ,0 }, #endif - [DEVZERO_MINOR] = { "zero", &zero_fops, FMODE_NOWAIT, 0666 }, + [5] = { "zero", &zero_fops, FMODE_NOWAIT, 0666 }, [7] = { "full", &full_fops, 0, 0666 }, [8] = { "random", &random_fops, FMODE_NOWAIT, 0666 }, [9] = { "urandom", &urandom_fops, FMODE_NOWAIT, 0666 },
include/linux/mm.h renamed from include/linux/mm.h~b +10 -8
@@ -740,9 +740,6 @@ static inline bool fault_flag_allow_retr { FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }, \ { FAULT_FLAG_VMA_LOCK, "VMA_LOCK" } -/* /dev/zero minor device number. Special due to MAP_PRIVATE semantics. */ -#define DEVZERO_MINOR 5 -/* * vm_fault is filled by the pagefault handler and passed to the vma's * ->fault function. The vma's ->fault is responsible for returning a bitmask
@@ -1554,6 +1551,11 @@ static inline void vma_set_anonymous(str vma->vm_ops =NULL ; } +static inline void vma_desc_set_anonymous (struct vm_area_desc *desc) +{ + desc->vm_ops =NULL ; +} +static inline bool vma_is_anonymous (const struct vm_area_struct *vma) {return !vma->vm_ops;
@@ -2279,8 +2281,7 @@ void unpin_folios(struct folio **folios,* All mappings backed by anonymous folios (all anonymous mappings and most * MAP_PRIVATE-file backed ranges) are CoW mappings. * -* All other mappings (including all writable MAP_SHARED mappings) are -* non-CoW. +* All other mappings (including all MAP_SHARED mappings) are non-CoW. * * The criteria are !VMA_SHARED_BIT, VMA_MAYWRITE_BIT. *
@@ -2317,7 +2318,7 @@ static inline bool vma_flags_is_cow_mapp/** * vma_is_cow_mapping() - Is this VMA a CoW mapping? -* @vma: The VMA to check. +* @desc: The VMA to check. * * See vma_flags_is_cow_mapping() for details. *
@@ -4407,8 +4408,9 @@ static inline unsigned long vma_pages(co* If @vma is a MAP_PRIVATE file-backed mapping, then this returns the * page offset within the file. * -* Edge cases: nommu does not abide by these and CoW MAP_PRIVATE-pfnmap regions -* have their page offset set to the first PFN in the range. +* Edge cases: nommu does not abide by these, MAP_PRIVATE-/dev/zero satisfies +* vma_is_anonymous() but has file-backed page offset, and MAP_PRIVATE-pfnmap +* regions have their page offset set to the first PFN in the range. * * Returns: The page offset of the start of @vma. */
include/linux/pagemap.h renamed from include/linux/pagemap.h~b +2 -1
@@ -1128,7 +1128,8 @@ static inline pgoff_t linear_anon_page_iconst pgoff_t pgoff =__linear_anon_page_index (vma, address);VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); -if (vma_is_anonymous (vma)) +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file)VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address));return pgoff;
mm/internal.h renamed from mm/internal.h~b +7 -10
@@ -240,18 +240,15 @@ static inline int mmap_file(struct file {int err =vfs_mmap (file, vma); +if (likely (!err)) +return 0 ; +/* -* Either we tried to call the file hook for mmap() and an error arose -* or a driver set vma->vm_ops = NULL intending there to be no VMA -* operations. -* -* In the former case the VMA is in an inconsistent state and we mustn't -* invoke any further hooks on it, in the latter case the hook actually -* wanted no further hooks to be invoked, so fix both by setting dummy -* VMA ops. +* OK, we tried to call the file hook for mmap(), but an error +* arose. The mapping is in an inconsistent state and we must not invoke +* any further hooks on it. */ -if (unlikely (err || !vma->vm_ops)) - vma->vm_ops = &vma_dummy_vm_ops; + vma->vm_ops = &vma_dummy_vm_ops;return err; }
mm/vma.c renamed from mm/vma.c~b +7 -45
88 lines not on this page · raw message
mm/vma.h renamed from mm/vma.h~b +3 -0
@@ -267,6 +267,9 @@ static inline void assert_sane_pgoff(str*/ if (!vma_is_anonymous (vma))return ; +/* MAP_PRIVATE-/dev/zero is anon, non-NULL vm_file, but has file pgoff. */ +if (vma->vm_file) +return ;/* If faulted in, could have been remapped. */ if (vma->anon_vma)return ;
mm/vma_internal.h renamed from mm/vma_internal.h~b +0 -1
@@ -23,7 +23,6 @@#include <linux/ksm.h> #include <linux/khugepaged.h> #include <linux/list.h> -#include <linux/major.h> #include <linux/maple_tree.h> #include <linux/mempolicy.h> #include <linux/mm.h>
tools/testing/selftests/mm/merge.c renamed from tools/testing/selftests/mm/merge.c~b +1 -105
118 lines not on this page · raw message
tools/testing/selftests/proc/proc-self-map-files-001.c renamed from tools/testing/selftests/proc/proc-self-map-files-001.c~b +1 -1
@@ -51,7 +51,7 @@ int main(void)int fd;unsigned long a, b; - fd =open ("/proc/self/exe" ,O_RDONLY ); + fd =open ("/dev/zero" ,O_RDONLY );if (fd ==-1 )return 1 ;
tools/testing/selftests/proc/proc-self-map-files-002.c renamed from tools/testing/selftests/proc/proc-self-map-files-002.c~b +1 -1
@@ -57,7 +57,7 @@ int main(void)int fd;unsigned long a, b; - fd =open ("/proc/self/exe" ,O_RDONLY ); + fd =open ("/dev/zero" ,O_RDONLY );if (fd ==-1 )return 1 ;
tools/testing/vma/include/dup.h renamed from tools/testing/vma/include/dup.h~b +2 -38
@@ -15,21 +15,6 @@ struct task_struct *get_current(void);#define MMF_HAS_MDWE 28#define current get_current() -#define MINORBITS 20 -#define MINORMASK ((1U << MINORBITS) - 1) - -#define MAJOR (dev) ((unsigned int) ((dev) >> MINORBITS)) -#define MINOR (dev) ((unsigned int) ((dev) & MINORMASK)) -#define MKDEV (ma, mi) (((ma) << MINORBITS) | (mi)) - -#define S_IFMT 00170000 -#define S_IFCHR 0020000 - -#define S_ISCHR (m) (((m) & S_IFMT) == S_IFCHR) - -#define MEM_MAJOR 1 -#define DEVZERO_MINOR 5 -/* * Define the task command name length as enum, then it can be visible to * BPF programs.
@@ -38,8 +23,6 @@ enum { TASK_COMM_LEN = 16, }; -typedef unsigned short umode_t ; -/* PARTIALLY implemented types. */ struct mm_struct {struct maple_tree mm_mt;
@@ -62,10 +45,6 @@ struct address_space {unsigned long flags;atomic_t i_mmap_writable; }; -struct inode { -umode_t i_mode; -dev_t i_rdev; -};struct file_operations {int (*mmap)(struct file *,struct vm_area_struct *);int (*mmap_prepare)(struct vm_area_desc *);
@@ -73,7 +52,6 @@ struct file_operations {struct file {struct address_space *f_mapping;const struct file_operations *f_op; -struct inode *f_inode; };struct anon_vma_chain {struct anon_vma *anon_vma;
@@ -1660,23 +1638,9 @@ static inline pgoff_t linear_anon_page_iconst pgoff_t pgoff =__linear_anon_page_index (vma, address);VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); -if (vma_is_anonymous (vma)) +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file)VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address));return pgoff; } - -static inline struct inode *file_inode (const struct file *f) -{ -return f->f_inode; -} - -static inline unsigned iminor (const struct inode *inode) -{ -return MINOR (inode->i_rdev); -} - -static inline unsigned imajor (const struct inode *inode) -{ -return MAJOR (inode->i_rdev); -}
tools/testing/vma/tests/mmap.c renamed from tools/testing/vma/tests/mmap.c~b +0 -50
57 lines not on this page · raw message
_
Lorenzo Stoakes (ARM)
maintainerQuoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Quoted 4 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
Not sure what the solution is!
Quoted 23 lines
maybe
hp2:/usr/src/linux-next> git log --oneline | grep "mm/vma: introduce VMA anon page offset field and add helpers"
249646a587dcmm/vma: introduce VMA anon page offset field and add helpersbase-commit: 249646a587dc^
But that requires that Sashiko be able to poke around in linux-next from previous days.
Quoted 11 lines
v5:
* Accumulated tags (thanks everybody!)
* Removed the final 4 patches to be handled later as there are nuances with the/dev/zerostuff we need to figure out, as discussed with David.
* Updated the cover letter to reflect this.
* Added comments tovma_flags_is_cow_mapping(), vma_[desc_]is_cow_mapping() as per Suren.
* Correct typo as per Suren.
* Reworded test comment in patch 16 from 'fault in' to 'trigger a CoW fault' as per David.
* Fix -> 75 char limit in patch 12's commit msg.
Quoted 2 lines
Here's how v5 altered mm.git. It's rather substantial, but mainly selftests:
Thanks for the diff, always useful!
The noise it's mostly because of dropping the final 4 commits, and as you say mostly test stuff that will be sent with whichever approach we decide on for MAP_PRIVATE-/dev/zero in the next cycle.
The actual changes elsewhere are rather trivial otherwise.
What remains, targeting 2nd week of the merge window, is heavily tested + fully reviewed, so all is still very sane :)
Quoted 534 lines
drivers/char/mem.c | 8 include/linux/mm.h | 18 - include/linux/pagemap.h | 3 mm/internal.h | 17 - mm/vma.c | 52 ---- mm/vma.h | 3 mm/vma_internal.h | 1 tools/testing/selftests/mm/merge.c | 106 ---------- tools/testing/selftests/proc/proc-self-map-files-001.c | 2 tools/testing/selftests/proc/proc-self-map-files-002.c | 2 tools/testing/vma/include/dup.h | 40 --- tools/testing/vma/tests/mmap.c | 50 ---- 12 files changed, 40 insertions(+), 262 deletions(-)drivers/char/mem.c renamed from drivers/char/mem.c~b +6
-2@@ -506,7 +506,11 @@ static int mmap_zero_prepare(struct vm_aif (vma_desc_test (desc,VMA_SHARED_BIT ))return shmem_zero_setup_desc (desc); -/* MAP_PRIVATE semantics are taken care for us by core mm. */ +/* +* This is a highly unique situation where we mark a MAP_PRIVATE mapping +* of /dev/zero anonymous, despite it not being. +*/ +vma_desc_set_anonymous (desc);return 0 ; }@@ -694,7 +698,7 @@ static const struct memdev { #ifdef CONFIG_DEVPORT [4 ] = {"port" , &port_fops,0 ,0 }, #endif - [DEVZERO_MINOR] = { "zero", &zero_fops, FMODE_NOWAIT, 0666 }, + [5] = { "zero", &zero_fops, FMODE_NOWAIT, 0666 }, [7] = { "full", &full_fops, 0, 0666 }, [8] = { "random", &random_fops, FMODE_NOWAIT, 0666 }, [9] = { "urandom", &urandom_fops, FMODE_NOWAIT, 0666 },include/linux/mm.h renamed from include/linux/mm.h~b +10
-8@@ -740,9 +740,6 @@ static inline bool fault_flag_allow_retr { FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }, \ { FAULT_FLAG_VMA_LOCK, "VMA_LOCK" } -/* /dev/zero minor device number. Special due to MAP_PRIVATE semantics. */ -#define DEVZERO_MINOR 5 -/* * vm_fault is filled by the pagefault handler and passed to the vma's * ->fault function. The vma's ->fault is responsible for returning a bitmask @@ -1554,6 +1551,11 @@ static inline void vma_set_anonymous(str vma->vm_ops =NULL ; } +static inline void vma_desc_set_anonymous (struct vm_area_desc *desc) +{ + desc->vm_ops =NULL ; +} +static inline bool vma_is_anonymous (const struct vm_area_struct *vma) {return !vma->vm_ops;@@ -2279,8 +2281,7 @@ void unpin_folios(struct folio **folios,* All mappings backed by anonymous folios (all anonymous mappings and most * MAP_PRIVATE-file backed ranges) are CoW mappings. * -* All other mappings (including all writable MAP_SHARED mappings) are -* non-CoW. +* All other mappings (including all MAP_SHARED mappings) are non-CoW. * * The criteria are !VMA_SHARED_BIT, VMA_MAYWRITE_BIT. * @@ -2317,7 +2318,7 @@ static inline bool vma_flags_is_cow_mapp/** * vma_is_cow_mapping() - Is this VMA a CoW mapping? -* @vma: The VMA to check. +* @desc: The VMA to check. * * See vma_flags_is_cow_mapping() for details. * @@ -4407,8 +4408,9 @@ static inline unsigned long vma_pages(co* If @vma is a MAP_PRIVATE file-backed mapping, then this returns the * page offset within the file. * -* Edge cases: nommu does not abide by these and CoW MAP_PRIVATE-pfnmap regions -* have their page offset set to the first PFN in the range. +* Edge cases: nommu does not abide by these, MAP_PRIVATE-/dev/zero satisfies +* vma_is_anonymous() but has file-backed page offset, and MAP_PRIVATE-pfnmap +* regions have their page offset set to the first PFN in the range. * * Returns: The page offset of the start of @vma. */ include/linux/pagemap.h renamed from include/linux/pagemap.h~b +2
-1@@ -1128,7 +1128,8 @@ static inline pgoff_t linear_anon_page_iconst pgoff_t pgoff =__linear_anon_page_index (vma, address);VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); -if (vma_is_anonymous (vma)) +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file)VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address));return pgoff;mm/internal.h renamed from mm/internal.h~b +7
-10@@ -240,18 +240,15 @@ static inline int mmap_file(struct file {int err =vfs_mmap (file, vma); +if (likely (!err)) +return 0 ; +/* -* Either we tried to call the file hook for mmap() and an error arose -* or a driver set vma->vm_ops = NULL intending there to be no VMA -* operations. -* -* In the former case the VMA is in an inconsistent state and we mustn't -* invoke any further hooks on it, in the latter case the hook actually -* wanted no further hooks to be invoked, so fix both by setting dummy -* VMA ops. +* OK, we tried to call the file hook for mmap(), but an error +* arose. The mapping is in an inconsistent state and we must not invoke +* any further hooks on it. */ -if (unlikely (err || !vma->vm_ops)) - vma->vm_ops = &vma_dummy_vm_ops; + vma->vm_ops = &vma_dummy_vm_ops;return err; }mm/vma.c renamed from mm/vma.c~b +7
-45@@ -2621,36 +2621,6 @@ static int __mmap_new_file_vma(struct mmreturn 0 ; } -static bool map_is_dev_zero (const struct mmap_state *map) -{ -const struct file *file = map->file; -struct inode *inode; - -if (!file) -return false; - inode =file_inode (file); -if (!S_ISCHR (inode->i_mode)) -return false; -return imajor (inode) ==MEM_MAJOR &&iminor (inode) ==DEVZERO_MINOR ; -} - -static void map_set_anon (struct mmap_state *map) -{ - map->file =NULL ; - map->vm_ops =NULL ; - map->pgoff = map->addr >>PAGE_SHIFT ; -} - -static bool map_is_private (const struct mmap_state *map) -{ -return !vma_flags_test (&map->vma_flags,VMA_SHARED_BIT ); -} - -static bool map_is_anon (const struct mmap_state *map) -{ -return map_is_private (map) && !map->file; -} -/* * __mmap_new_vma() - Allocate a new VMA for the region, as merging was not * possible. @@ -2664,7 +2634,8 @@ static bool map_is_anon(const struct mmastatic int __mmap_new_vma (struct mmap_state *map,struct vm_area_struct **vmap,struct mmap_action *action) { -const bool is_anon =map_is_anon (map); +const bool is_anon = !map->file && + !vma_flags_test (&map->vma_flags,VMA_SHARED_BIT );struct vma_iterator *vmi = map->vmi;int error =0 ;struct vm_area_struct *vma;@@ -2806,10 +2777,6 @@ static int call_mmap_prepare(struct mmapif (err)return err; -/* Hooks cannot mark themselves anonymous. */ -if (!desc->vm_ops) -return -EINVAL ; - err =call_action_prepare (map, desc);if (err)return err;@@ -2826,21 +2793,16 @@ static int call_mmap_prepare(struct mmap map->vm_ops = desc->vm_ops; map->vm_private_data = desc->private_data; -/* -* MAP_PRIVATE-/dev/zero mappings are an ancient way of getting -* anonymous mappings. Rather than allowing these mappings to be odd -* outliers, simply make them truly anonymous. -*/ -if (map_is_private (map) &&map_is_dev_zero (map)) -map_set_anon (map); -return 0 ; }static void set_vma_user_defined_fields (struct vm_area_struct *vma,struct mmap_state *map) { - vma->vm_ops = map->vm_ops; +if (map->vm_ops) + vma->vm_ops = map->vm_ops; +else /* Only /dev/zero should do this. */ +vma_set_anonymous (vma); vma->vm_private_data = map->vm_private_data; }@@ -2920,7 +2882,7 @@ static unsigned long __mmap_region(struc allocated_new = true; } -if (have_mmap_prepare && !map_is_anon (&map)) +if (have_mmap_prepare)set_vma_user_defined_fields (vma, &map);__mmap_complete (&map, vma);mm/vma.h renamed from mm/vma.h~b +3
-0@@ -267,6 +267,9 @@ static inline void assert_sane_pgoff(str*/ if (!vma_is_anonymous (vma))return ; +/* MAP_PRIVATE-/dev/zero is anon, non-NULL vm_file, but has file pgoff. */ +if (vma->vm_file) +return ;/* If faulted in, could have been remapped. */ if (vma->anon_vma)return ;mm/vma_internal.h renamed from mm/vma_internal.h~b +0
-1@@ -23,7 +23,6 @@#include <linux/ksm.h> #include <linux/khugepaged.h> #include <linux/list.h> -#include <linux/major.h> #include <linux/maple_tree.h> #include <linux/mempolicy.h> #include <linux/mm.h> tools/testing/selftests/mm/merge.c renamed from tools/testing/selftests/mm/merge.c~b +1
-105@@ -1324,7 +1324,7 @@ TEST_F(merge, anon_and_page_offset_mismaASSERT_NE (ptr,MAP_FAILED );/* -* Map another separately and trigger a CoW fault, at page offset 5: +* Map another separately and trigger a CoW fault at page offset 5: * * |-----------| |---------| * | unfaulted | | faulted | @@ -1362,110 +1362,6 @@ TEST_F(merge, anon_and_page_offset_mismaASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +5 * page_size); } -TEST_F(merge, merge_map_private_dev_zero_unfaulted) -{ -struct procmap_fd *procmap = &self->procmap; -unsigned int page_size = self->page_size; -char *carveout = self->carveout; -char *ptr, *ptr2; -int fd_zero; - -if (access ("/dev/zero" ,F_OK )) -SKIP (return,"No /dev/zero." ); - fd_zero =open ("/dev/zero" ,O_RDWR ); -ASSERT_NE (fd_zero,-1 ); - -/* -* Map two MAP_PRIVATE-/dev/zero VMAs next to one another with offset 0 -* each. -* -* With these being made truly anonymous upon mapping, they will -* merge. If they were file-backed VMAs the page offsets would prevent -* merge: -* -* |-----||------| |-------------| -* | ptr || ptr2 | -> | ptr | -* |-----||------| |-------------| -*/ - ptr =mmap (carveout,5 * page_size,PROT_READ |PROT_WRITE , -MAP_FIXED |MAP_PRIVATE , fd_zero,0 ); -if (ptr ==MAP_FAILED ) { -close (fd_zero); -ASSERT_TRUE (false); - } - ptr2 =mmap (&carveout[5 * page_size],5 * page_size, -PROT_READ |PROT_WRITE ,MAP_FIXED |MAP_PRIVATE , fd_zero,0 ); -if (ptr2 ==MAP_FAILED ) { -close (fd_zero); -ASSERT_TRUE (false); - } -close (fd_zero); - -/* Assert that they merged. */ -ASSERT_TRUE (find_vma_procmap (procmap, ptr)); -ASSERT_EQ (procmap->query.vma_start, (unsigned long )ptr); -ASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +10 * page_size); -} - -TEST_F(merge, merge_map_private_dev_zero_faulted_unfaulted) -{ -struct procmap_fd *procmap = &self->procmap; -unsigned int page_size = self->page_size; -char *carveout = self->carveout; -char *ptr, *ptr2; -int fd_zero; - -if (access ("/dev/zero" ,F_OK )) -SKIP (return,"No /dev/zero." ); - fd_zero =open ("/dev/zero" ,O_RDWR ); -ASSERT_NE (fd_zero,-1 ); - -/* -* Map a MAP_PRIVATE mapping of /dev/zero with page offset 0, then fault -* it in: -* -* |-------------------------------| -* | faulted | -* |-------------------------------| -*/ - ptr =mmap (carveout,15 * page_size,PROT_READ |PROT_WRITE , -MAP_FIXED |MAP_PRIVATE , fd_zero,0 ); -if (ptr ==MAP_FAILED ) { -close (fd_zero); -ASSERT_TRUE (false); - } -memset (ptr,'x' ,15 * page_size); - -/* -* Unmap the middle: -* -* |---------| |---------| -* | faulted | | faulted | -* |---------| |---------| -*/ -ASSERT_EQ (munmap (&ptr[5 * page_size],5 * page_size),0 ); - -/* -* Map in a new unfaulted mapping in the middle with page offset 0 - -* this should merge and would not if it were treated as a file rather -* than pure anon: -* -* |---------|-----------|---------| -* | faulted | unfaulted | faulted | -* |---------|-----------|---------| -*/ - ptr2 =mmap (&carveout[5 * page_size],5 * page_size, -PROT_READ |PROT_WRITE ,MAP_FIXED |MAP_PRIVATE , - fd_zero,0 ); -close (fd_zero); -ASSERT_NE (ptr2,MAP_FAILED ); - -/* Assert that they merged. */ -ASSERT_TRUE (find_vma_procmap (procmap, ptr)); -ASSERT_EQ (procmap->query.vma_start, (unsigned long )ptr); -ASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +15 * page_size); -} - TEST_F(merge_with_fork, mremap_faulted_to_unfaulted_prev) {struct procmap_fd *procmap = &self->procmap;tools/testing/selftests/proc/proc-self-map-files-001.c renamed from tools/testing/selftests/proc/proc-self-map-files-001.c~b +1
-1@@ -51,7 +51,7 @@ int main(void)int fd;unsigned long a, b; - fd =open ("/proc/self/exe" ,O_RDONLY ); + fd =open ("/dev/zero" ,O_RDONLY );if (fd ==-1 )return 1 ;tools/testing/selftests/proc/proc-self-map-files-002.c renamed from tools/testing/selftests/proc/proc-self-map-files-002.c~b +1
-1@@ -57,7 +57,7 @@ int main(void)int fd;unsigned long a, b; - fd =open ("/proc/self/exe" ,O_RDONLY ); + fd =open ("/dev/zero" ,O_RDONLY );if (fd ==-1 )return 1 ;tools/testing/vma/include/dup.h renamed from tools/testing/vma/include/dup.h~b +2
-38@@ -15,21 +15,6 @@ struct task_struct *get_current(void);#define MMF_HAS_MDWE 28#define current get_current() -#define MINORBITS 20 -#define MINORMASK ((1U << MINORBITS) - 1) - -#define MAJOR (dev) ((unsigned int) ((dev) >> MINORBITS)) -#define MINOR (dev) ((unsigned int) ((dev) & MINORMASK)) -#define MKDEV (ma, mi) (((ma) << MINORBITS) | (mi)) - -#define S_IFMT 00170000 -#define S_IFCHR 0020000 - -#define S_ISCHR (m) (((m) & S_IFMT) == S_IFCHR) - -#define MEM_MAJOR 1 -#define DEVZERO_MINOR 5 -/* * Define the task command name length as enum, then it can be visible to * BPF programs. @@ -38,8 +23,6 @@ enum { TASK_COMM_LEN = 16, }; -typedef unsigned short umode_t ; -/* PARTIALLY implemented types. */ struct mm_struct {struct maple_tree mm_mt;@@ -62,10 +45,6 @@ struct address_space {unsigned long flags;atomic_t i_mmap_writable; }; -struct inode { -umode_t i_mode; -dev_t i_rdev; -};struct file_operations {int (*mmap)(struct file *,struct vm_area_struct *);int (*mmap_prepare)(struct vm_area_desc *);@@ -73,7 +52,6 @@ struct file_operations {struct file {struct address_space *f_mapping;const struct file_operations *f_op; -struct inode *f_inode; };struct anon_vma_chain {struct anon_vma *anon_vma;@@ -1660,23 +1638,9 @@ static inline pgoff_t linear_anon_page_iconst pgoff_t pgoff =__linear_anon_page_index (vma, address);VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); -if (vma_is_anonymous (vma)) +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file)VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address));return pgoff; } - -static inline struct inode *file_inode (const struct file *f) -{ -return f->f_inode; -} - -static inline unsigned iminor (const struct inode *inode) -{ -return MINOR (inode->i_rdev); -} - -static inline unsigned imajor (const struct inode *inode) -{ -return MAJOR (inode->i_rdev); -}tools/testing/vma/tests/mmap.c renamed from tools/testing/vma/tests/mmap.c~b +0
-50@@ -45,57 +45,7 @@ static bool test_mmap_region_basic(void)return true; } -static int dummy_mmap_prepare (struct vm_area_desc *desc) -{ -return 0 ; -} - -static bool test_pure_anon_dev_zero (void ) -{ -const vma_flags_t vma_flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT , -VMA_MAYREAD_BIT ,VMA_MAYWRITE_BIT ); -const struct file_operations f_op = { - .mmap_prepare = dummy_mmap_prepare, - }; -struct inode inode = { - .i_mode =S_IFCHR , - .i_rdev =MKDEV (MEM_MAJOR ,DEVZERO_MINOR ), - }; -struct file file = { - .f_inode = &inode, - .f_op = &f_op, - }; -struct mm_struct mm = {}; -struct vm_area_struct *vma; -unsigned long addr; -VMA_ITERATOR (vmi, &mm,0 ); - - current->mm = &mm; - -/* -* Map a MAP_PRIVATE-/dev/zero mapping at address 0x300000 with a page -* offset of 0x10, which we expect to be reset to the anonymous page -* offset. -*/ - addr =__mmap_region (&file,0x300000 ,0x3000 , vma_flags,0x10 ,NULL ); -ASSERT_EQ (addr,0x300000 ); - -/* Assert that it truly is an anonymous mapping. */ - vma =vma_lookup (&mm, addr); -ASSERT_NE (vma,NULL ); -ASSERT_TRUE (vma_is_anonymous (vma)); -ASSERT_EQ (vma->vm_file,NULL ); -ASSERT_EQ (vma->vm_private_data,NULL ); -/* Expect anonymous page offsets. */ -ASSERT_EQ (vma->vm_pgoff,0x300 ); -ASSERT_EQ (vma_start_anon_pgoff (vma),0x300 ); - -cleanup_mm (&mm, &vmi); -return true; -} -static void run_mmap_tests (int *num_tests,int *num_fail) {TEST (mmap_region_basic); -TEST (pure_anon_dev_zero); }
Quoted 2 lines
_
--
Cheers, Lorenzo
Matthew Brost
maintainerQuoted 9 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
Questioning your assumptions is useful, even when they turn out to be wrong. Show more lines
Quoted 4 lines
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Yes, it's kind of odd that Sashiko burns more tokens than a full day of breakfast, lunch, and dinner service. Running Sashiko is a bottleneck in my workflow, so I'll defer to others on this list.
Quoted 10 lines
Quoted 4 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
Not sure what the solution is!
We have seen this on the Xe list (our list is based on drm-tip), typically with cross-subsystem patches. Some cross-subsystem patches apply and run correctly, while others do not but public CI flows run based on drm-tip. I do not have a bisect or a clear understanding of what works and what doesn't, but I think it would be very useful if the community could better understand the root cause.
Matt
Quoted 576 lines
Quoted 23 lines
maybe
hp2:/usr/src/linux-next> git log --oneline | grep "mm/vma: introduce VMA anon page offset field and add helpers"
249646a587dcmm/vma: introduce VMA anon page offset field and add helpersbase-commit: 249646a587dc^
But that requires that Sashiko be able to poke around in linux-next from previous days.
Quoted 11 lines
v5:
* Accumulated tags (thanks everybody!)
* Removed the final 4 patches to be handled later as there are nuances with the/dev/zerostuff we need to figure out, as discussed with David.
* Updated the cover letter to reflect this.
* Added comments tovma_flags_is_cow_mapping(), vma_[desc_]is_cow_mapping() as per Suren.
* Correct typo as per Suren.
* Reworded test comment in patch 16 from 'fault in' to 'trigger a CoW fault' as per David.
* Fix -> 75 char limit in patch 12's commit msg.Quoted 2 lines
Here's how v5 altered mm.git. It's rather substantial, but mainly selftests:
Thanks for the diff, always useful!
The noise it's mostly because of dropping the final 4 commits, and as you say mostly test stuff that will be sent with whichever approach we decide on for MAP_PRIVATE-/dev/zero in the next cycle.
The actual changes elsewhere are rather trivial otherwise.
What remains, targeting 2nd week of the merge window, is heavily tested + fully reviewed, so all is still very sane :)
Quoted 534 lines
drivers/char/mem.c | 8 include/linux/mm.h | 18 - include/linux/pagemap.h | 3 mm/internal.h | 17 - mm/vma.c | 52 ---- mm/vma.h | 3 mm/vma_internal.h | 1 tools/testing/selftests/mm/merge.c | 106 ---------- tools/testing/selftests/proc/proc-self-map-files-001.c | 2 tools/testing/selftests/proc/proc-self-map-files-002.c | 2 tools/testing/vma/include/dup.h | 40 --- tools/testing/vma/tests/mmap.c | 50 ---- 12 files changed, 40 insertions(+), 262 deletions(-)drivers/char/mem.c renamed from drivers/char/mem.c~b +6
-2@@ -506,7 +506,11 @@ static int mmap_zero_prepare(struct vm_aif (vma_desc_test (desc,VMA_SHARED_BIT ))return shmem_zero_setup_desc (desc); -/* MAP_PRIVATE semantics are taken care for us by core mm. */ +/* +* This is a highly unique situation where we mark a MAP_PRIVATE mapping +* of /dev/zero anonymous, despite it not being. +*/ +vma_desc_set_anonymous (desc);return 0 ; }@@ -694,7 +698,7 @@ static const struct memdev { #ifdef CONFIG_DEVPORT [4 ] = {"port" , &port_fops,0 ,0 }, #endif - [DEVZERO_MINOR] = { "zero", &zero_fops, FMODE_NOWAIT, 0666 }, + [5] = { "zero", &zero_fops, FMODE_NOWAIT, 0666 }, [7] = { "full", &full_fops, 0, 0666 }, [8] = { "random", &random_fops, FMODE_NOWAIT, 0666 }, [9] = { "urandom", &urandom_fops, FMODE_NOWAIT, 0666 },include/linux/mm.h renamed from include/linux/mm.h~b +10
-8@@ -740,9 +740,6 @@ static inline bool fault_flag_allow_retr { FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }, \ { FAULT_FLAG_VMA_LOCK, "VMA_LOCK" } -/* /dev/zero minor device number. Special due to MAP_PRIVATE semantics. */ -#define DEVZERO_MINOR 5 -/* * vm_fault is filled by the pagefault handler and passed to the vma's * ->fault function. The vma's ->fault is responsible for returning a bitmask @@ -1554,6 +1551,11 @@ static inline void vma_set_anonymous(str vma->vm_ops =NULL ; } +static inline void vma_desc_set_anonymous (struct vm_area_desc *desc) +{ + desc->vm_ops =NULL ; +} +static inline bool vma_is_anonymous (const struct vm_area_struct *vma) {return !vma->vm_ops;@@ -2279,8 +2281,7 @@ void unpin_folios(struct folio **folios,* All mappings backed by anonymous folios (all anonymous mappings and most * MAP_PRIVATE-file backed ranges) are CoW mappings. * -* All other mappings (including all writable MAP_SHARED mappings) are -* non-CoW. +* All other mappings (including all MAP_SHARED mappings) are non-CoW. * * The criteria are !VMA_SHARED_BIT, VMA_MAYWRITE_BIT. * @@ -2317,7 +2318,7 @@ static inline bool vma_flags_is_cow_mapp/** * vma_is_cow_mapping() - Is this VMA a CoW mapping? -* @vma: The VMA to check. +* @desc: The VMA to check. * * See vma_flags_is_cow_mapping() for details. * @@ -4407,8 +4408,9 @@ static inline unsigned long vma_pages(co* If @vma is a MAP_PRIVATE file-backed mapping, then this returns the * page offset within the file. * -* Edge cases: nommu does not abide by these and CoW MAP_PRIVATE-pfnmap regions -* have their page offset set to the first PFN in the range. +* Edge cases: nommu does not abide by these, MAP_PRIVATE-/dev/zero satisfies +* vma_is_anonymous() but has file-backed page offset, and MAP_PRIVATE-pfnmap +* regions have their page offset set to the first PFN in the range. * * Returns: The page offset of the start of @vma. */ include/linux/pagemap.h renamed from include/linux/pagemap.h~b +2
-1@@ -1128,7 +1128,8 @@ static inline pgoff_t linear_anon_page_iconst pgoff_t pgoff =__linear_anon_page_index (vma, address);VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); -if (vma_is_anonymous (vma)) +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file)VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address));return pgoff;mm/internal.h renamed from mm/internal.h~b +7
-10@@ -240,18 +240,15 @@ static inline int mmap_file(struct file {int err =vfs_mmap (file, vma); +if (likely (!err)) +return 0 ; +/* -* Either we tried to call the file hook for mmap() and an error arose -* or a driver set vma->vm_ops = NULL intending there to be no VMA -* operations. -* -* In the former case the VMA is in an inconsistent state and we mustn't -* invoke any further hooks on it, in the latter case the hook actually -* wanted no further hooks to be invoked, so fix both by setting dummy -* VMA ops. +* OK, we tried to call the file hook for mmap(), but an error +* arose. The mapping is in an inconsistent state and we must not invoke +* any further hooks on it. */ -if (unlikely (err || !vma->vm_ops)) - vma->vm_ops = &vma_dummy_vm_ops; + vma->vm_ops = &vma_dummy_vm_ops;return err; }mm/vma.c renamed from mm/vma.c~b +7
-45@@ -2621,36 +2621,6 @@ static int __mmap_new_file_vma(struct mmreturn 0 ; } -static bool map_is_dev_zero (const struct mmap_state *map) -{ -const struct file *file = map->file; -struct inode *inode; - -if (!file) -return false; - inode =file_inode (file); -if (!S_ISCHR (inode->i_mode)) -return false; -return imajor (inode) ==MEM_MAJOR &&iminor (inode) ==DEVZERO_MINOR ; -} - -static void map_set_anon (struct mmap_state *map) -{ - map->file =NULL ; - map->vm_ops =NULL ; - map->pgoff = map->addr >>PAGE_SHIFT ; -} - -static bool map_is_private (const struct mmap_state *map) -{ -return !vma_flags_test (&map->vma_flags,VMA_SHARED_BIT ); -} - -static bool map_is_anon (const struct mmap_state *map) -{ -return map_is_private (map) && !map->file; -} -/* * __mmap_new_vma() - Allocate a new VMA for the region, as merging was not * possible. @@ -2664,7 +2634,8 @@ static bool map_is_anon(const struct mmastatic int __mmap_new_vma (struct mmap_state *map,struct vm_area_struct **vmap,struct mmap_action *action) { -const bool is_anon =map_is_anon (map); +const bool is_anon = !map->file && + !vma_flags_test (&map->vma_flags,VMA_SHARED_BIT );struct vma_iterator *vmi = map->vmi;int error =0 ;struct vm_area_struct *vma;@@ -2806,10 +2777,6 @@ static int call_mmap_prepare(struct mmapif (err)return err; -/* Hooks cannot mark themselves anonymous. */ -if (!desc->vm_ops) -return -EINVAL ; - err =call_action_prepare (map, desc);if (err)return err;@@ -2826,21 +2793,16 @@ static int call_mmap_prepare(struct mmap map->vm_ops = desc->vm_ops; map->vm_private_data = desc->private_data; -/* -* MAP_PRIVATE-/dev/zero mappings are an ancient way of getting -* anonymous mappings. Rather than allowing these mappings to be odd -* outliers, simply make them truly anonymous. -*/ -if (map_is_private (map) &&map_is_dev_zero (map)) -map_set_anon (map); -return 0 ; }static void set_vma_user_defined_fields (struct vm_area_struct *vma,struct mmap_state *map) { - vma->vm_ops = map->vm_ops; +if (map->vm_ops) + vma->vm_ops = map->vm_ops; +else /* Only /dev/zero should do this. */ +vma_set_anonymous (vma); vma->vm_private_data = map->vm_private_data; }@@ -2920,7 +2882,7 @@ static unsigned long __mmap_region(struc allocated_new = true; } -if (have_mmap_prepare && !map_is_anon (&map)) +if (have_mmap_prepare)set_vma_user_defined_fields (vma, &map);__mmap_complete (&map, vma);mm/vma.h renamed from mm/vma.h~b +3
-0@@ -267,6 +267,9 @@ static inline void assert_sane_pgoff(str*/ if (!vma_is_anonymous (vma))return ; +/* MAP_PRIVATE-/dev/zero is anon, non-NULL vm_file, but has file pgoff. */ +if (vma->vm_file) +return ;/* If faulted in, could have been remapped. */ if (vma->anon_vma)return ;mm/vma_internal.h renamed from mm/vma_internal.h~b +0
-1@@ -23,7 +23,6 @@#include <linux/ksm.h> #include <linux/khugepaged.h> #include <linux/list.h> -#include <linux/major.h> #include <linux/maple_tree.h> #include <linux/mempolicy.h> #include <linux/mm.h> tools/testing/selftests/mm/merge.c renamed from tools/testing/selftests/mm/merge.c~b +1
-105@@ -1324,7 +1324,7 @@ TEST_F(merge, anon_and_page_offset_mismaASSERT_NE (ptr,MAP_FAILED );/* -* Map another separately and trigger a CoW fault, at page offset 5: +* Map another separately and trigger a CoW fault at page offset 5: * * |-----------| |---------| * | unfaulted | | faulted | @@ -1362,110 +1362,6 @@ TEST_F(merge, anon_and_page_offset_mismaASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +5 * page_size); } -TEST_F(merge, merge_map_private_dev_zero_unfaulted) -{ -struct procmap_fd *procmap = &self->procmap; -unsigned int page_size = self->page_size; -char *carveout = self->carveout; -char *ptr, *ptr2; -int fd_zero; - -if (access ("/dev/zero" ,F_OK )) -SKIP (return,"No /dev/zero." ); - fd_zero =open ("/dev/zero" ,O_RDWR ); -ASSERT_NE (fd_zero,-1 ); - -/* -* Map two MAP_PRIVATE-/dev/zero VMAs next to one another with offset 0 -* each. -* -* With these being made truly anonymous upon mapping, they will -* merge. If they were file-backed VMAs the page offsets would prevent -* merge: -* -* |-----||------| |-------------| -* | ptr || ptr2 | -> | ptr | -* |-----||------| |-------------| -*/ - ptr =mmap (carveout,5 * page_size,PROT_READ |PROT_WRITE , -MAP_FIXED |MAP_PRIVATE , fd_zero,0 ); -if (ptr ==MAP_FAILED ) { -close (fd_zero); -ASSERT_TRUE (false); - } - ptr2 =mmap (&carveout[5 * page_size],5 * page_size, -PROT_READ |PROT_WRITE ,MAP_FIXED |MAP_PRIVATE , fd_zero,0 ); -if (ptr2 ==MAP_FAILED ) { -close (fd_zero); -ASSERT_TRUE (false); - } -close (fd_zero); - -/* Assert that they merged. */ -ASSERT_TRUE (find_vma_procmap (procmap, ptr)); -ASSERT_EQ (procmap->query.vma_start, (unsigned long )ptr); -ASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +10 * page_size); -} - -TEST_F(merge, merge_map_private_dev_zero_faulted_unfaulted) -{ -struct procmap_fd *procmap = &self->procmap; -unsigned int page_size = self->page_size; -char *carveout = self->carveout; -char *ptr, *ptr2; -int fd_zero; - -if (access ("/dev/zero" ,F_OK )) -SKIP (return,"No /dev/zero." ); - fd_zero =open ("/dev/zero" ,O_RDWR ); -ASSERT_NE (fd_zero,-1 ); - -/* -* Map a MAP_PRIVATE mapping of /dev/zero with page offset 0, then fault -* it in: -* -* |-------------------------------| -* | faulted | -* |-------------------------------| -*/ - ptr =mmap (carveout,15 * page_size,PROT_READ |PROT_WRITE , -MAP_FIXED |MAP_PRIVATE , fd_zero,0 ); -if (ptr ==MAP_FAILED ) { -close (fd_zero); -ASSERT_TRUE (false); - } -memset (ptr,'x' ,15 * page_size); - -/* -* Unmap the middle: -* -* |---------| |---------| -* | faulted | | faulted | -* |---------| |---------| -*/ -ASSERT_EQ (munmap (&ptr[5 * page_size],5 * page_size),0 ); - -/* -* Map in a new unfaulted mapping in the middle with page offset 0 - -* this should merge and would not if it were treated as a file rather -* than pure anon: -* -* |---------|-----------|---------| -* | faulted | unfaulted | faulted | -* |---------|-----------|---------| -*/ - ptr2 =mmap (&carveout[5 * page_size],5 * page_size, -PROT_READ |PROT_WRITE ,MAP_FIXED |MAP_PRIVATE , - fd_zero,0 ); -close (fd_zero); -ASSERT_NE (ptr2,MAP_FAILED ); - -/* Assert that they merged. */ -ASSERT_TRUE (find_vma_procmap (procmap, ptr)); -ASSERT_EQ (procmap->query.vma_start, (unsigned long )ptr); -ASSERT_EQ (procmap->query.vma_end, (unsigned long )ptr +15 * page_size); -} - TEST_F(merge_with_fork, mremap_faulted_to_unfaulted_prev) {struct procmap_fd *procmap = &self->procmap;tools/testing/selftests/proc/proc-self-map-files-001.c renamed from tools/testing/selftests/proc/proc-self-map-files-001.c~b +1
-1@@ -51,7 +51,7 @@ int main(void)int fd;unsigned long a, b; - fd =open ("/proc/self/exe" ,O_RDONLY ); + fd =open ("/dev/zero" ,O_RDONLY );if (fd ==-1 )return 1 ;tools/testing/selftests/proc/proc-self-map-files-002.c renamed from tools/testing/selftests/proc/proc-self-map-files-002.c~b +1
-1@@ -57,7 +57,7 @@ int main(void)int fd;unsigned long a, b; - fd =open ("/proc/self/exe" ,O_RDONLY ); + fd =open ("/dev/zero" ,O_RDONLY );if (fd ==-1 )return 1 ;tools/testing/vma/include/dup.h renamed from tools/testing/vma/include/dup.h~b +2
-38@@ -15,21 +15,6 @@ struct task_struct *get_current(void);#define MMF_HAS_MDWE 28#define current get_current() -#define MINORBITS 20 -#define MINORMASK ((1U << MINORBITS) - 1) - -#define MAJOR (dev) ((unsigned int) ((dev) >> MINORBITS)) -#define MINOR (dev) ((unsigned int) ((dev) & MINORMASK)) -#define MKDEV (ma, mi) (((ma) << MINORBITS) | (mi)) - -#define S_IFMT 00170000 -#define S_IFCHR 0020000 - -#define S_ISCHR (m) (((m) & S_IFMT) == S_IFCHR) - -#define MEM_MAJOR 1 -#define DEVZERO_MINOR 5 -/* * Define the task command name length as enum, then it can be visible to * BPF programs. @@ -38,8 +23,6 @@ enum { TASK_COMM_LEN = 16, }; -typedef unsigned short umode_t ; -/* PARTIALLY implemented types. */ struct mm_struct {struct maple_tree mm_mt;@@ -62,10 +45,6 @@ struct address_space {unsigned long flags;atomic_t i_mmap_writable; }; -struct inode { -umode_t i_mode; -dev_t i_rdev; -};struct file_operations {int (*mmap)(struct file *,struct vm_area_struct *);int (*mmap_prepare)(struct vm_area_desc *);@@ -73,7 +52,6 @@ struct file_operations {struct file {struct address_space *f_mapping;const struct file_operations *f_op; -struct inode *f_inode; };struct anon_vma_chain {struct anon_vma *anon_vma;@@ -1660,23 +1638,9 @@ static inline pgoff_t linear_anon_page_iconst pgoff_t pgoff =__linear_anon_page_index (vma, address);VM_WARN_ON_ONCE (!vma_is_cow_mapping (vma)); -if (vma_is_anonymous (vma)) +/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */ +if (vma_is_anonymous (vma) && !vma->vm_file)VM_WARN_ON_ONCE (pgoff !=linear_page_index (vma, address));return pgoff; } - -static inline struct inode *file_inode (const struct file *f) -{ -return f->f_inode; -} - -static inline unsigned iminor (const struct inode *inode) -{ -return MINOR (inode->i_rdev); -} - -static inline unsigned imajor (const struct inode *inode) -{ -return MAJOR (inode->i_rdev); -}tools/testing/vma/tests/mmap.c renamed from tools/testing/vma/tests/mmap.c~b +0
-50@@ -45,57 +45,7 @@ static bool test_mmap_region_basic(void)return true; } -static int dummy_mmap_prepare (struct vm_area_desc *desc) -{ -return 0 ; -} - -static bool test_pure_anon_dev_zero (void ) -{ -const vma_flags_t vma_flags =mk_vma_flags (VMA_READ_BIT ,VMA_WRITE_BIT , -VMA_MAYREAD_BIT ,VMA_MAYWRITE_BIT ); -const struct file_operations f_op = { - .mmap_prepare = dummy_mmap_prepare, - }; -struct inode inode = { - .i_mode =S_IFCHR , - .i_rdev =MKDEV (MEM_MAJOR ,DEVZERO_MINOR ), - }; -struct file file = { - .f_inode = &inode, - .f_op = &f_op, - }; -struct mm_struct mm = {}; -struct vm_area_struct *vma; -unsigned long addr; -VMA_ITERATOR (vmi, &mm,0 ); - - current->mm = &mm; - -/* -* Map a MAP_PRIVATE-/dev/zero mapping at address 0x300000 with a page -* offset of 0x10, which we expect to be reset to the anonymous page -* offset. -*/ - addr =__mmap_region (&file,0x300000 ,0x3000 , vma_flags,0x10 ,NULL ); -ASSERT_EQ (addr,0x300000 ); - -/* Assert that it truly is an anonymous mapping. */ - vma =vma_lookup (&mm, addr); -ASSERT_NE (vma,NULL ); -ASSERT_TRUE (vma_is_anonymous (vma)); -ASSERT_EQ (vma->vm_file,NULL ); -ASSERT_EQ (vma->vm_private_data,NULL ); -/* Expect anonymous page offsets. */ -ASSERT_EQ (vma->vm_pgoff,0x300 ); -ASSERT_EQ (vma_start_anon_pgoff (vma),0x300 ); - -cleanup_mm (&mm, &vmi); -return true; -} -static void run_mmap_tests (int *num_tests,int *num_fail) {TEST (mmap_region_basic); -TEST (pure_anon_dev_zero); }Quoted 2 lines
_
--
Cheers, Lorenzo
Lorenzo Stoakes (ARM)
maintainerfixing sashiko failure to apply (was Re: [PATCH v5 00/16] mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff)
[ trim cc list ]
+cc Roman
Quoted 14 lines · Matthew Brost
Quoted 9 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
Questioning your assumptions is useful, even when they turn out to be wrong. Show more lines
Quoted 8 lines
Quoted 4 lines
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Yes, it's kind of odd that Sashiko burns more tokens than a full day of breakfast, lunch, and dinner service. Running Sashiko is a bottleneck in my workflow, so I'll defer to others on this list.
Yup, not sure if there are recommended configs for something saner :)
Maybe Roman has some advice on that?
Quoted 12 lines
Quoted 11 lines
Quoted 5 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
Not sure what the solution is!
Quoted 6 lines
We have seen this on the Xe list (our list is based on drm-tip), typically with cross-subsystem patches. Some cross-subsystem patches apply and run correctly, while others do not but public CI flows run based on drm-tip. I do not have a bisect or a clear understanding of what works and what doesn't, but I think it would be very useful if the community could better understand the root cause.
As Mike said, mm-unstable/mm-new is heavily rebased and also carries the old version of the series before the new one is applied, so it's super unclear what the base commit should be there.
But in general, I wonder if it's possible that we could tell sashiko after-the-fact what base commit to look at once the series is in, or re-trigger it somehow once it's in-tree?
Roman - any suggestions on what we could do to help sashiko find things?
(Once mm-next is in place everything with change again, but can address that then :)
Quoted 3 lines
Matt
--
Cheers, Lorenzo
Re: fixing sashiko failure to apply (was Re: [PATCH v5 00/16] mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff)
Quoted 28 lines · Lorenzo Stoakes (ARM)
[ trim cc list ]
+cc Roman
Quoted 20 lines · Matthew Brost
Quoted 10 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
Questioning your assumptions is useful, even when they turn out to be wrong. Show more lines
Quoted 4 lines
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Quoted 7 lines
Quoted 3 lines
Yes, it's kind of odd that Sashiko burns more tokens than a full day of breakfast, lunch, and dinner service. Running Sashiko is a bottleneck in my workflow, so I'll defer to others on this list.
Yup, not sure if there are recommended configs for something saner :)
Maybe Roman has some advice on that?
Sorry, no magic way to save tokens without hurting the quality. But I am curious what are your numbers? Can be model-dependent too. In prod on average it burns 3-4M tokens per patch with Gemini 3.1 Pro, but maybe mm patches are more complex than average, Idk.
One option is to run only some discovery stages (—stages), but this unlikely will save you that much. I’d say use a cheaper and faster model for the development, but it has it’s downsides too.
If you have an example of a patch(set) which is particularly token-hungry, I can take a look.
Quoted 24 lines
Quoted 13 lines
Quoted 12 lines
Quoted 6 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
Not sure what the solution is!
Quoted 6 lines
We have seen this on the Xe list (our list is based on drm-tip), typically with cross-subsystem patches. Some cross-subsystem patches apply and run correctly, while others do not but public CI flows run based on drm-tip. I do not have a bisect or a clear understanding of what works and what doesn't, but I think it would be very useful if the community could better understand the root cause.
As Mike said, mm-unstable/mm-new is heavily rebased and also carries the old version of the series before the new one is applied, so it's super unclear what the base commit should be there.
Quoted 8 lines
But in general, I wonder if it's possible that we could tell sashiko after-the-fact what base commit to look at once the series is in, or re-trigger it somehow once it's in-tree?
Roman - any suggestions on what we could do to help sashiko find things?
(Once mm-next is in place everything with change again, but can address that then :)
I can implement any reasonable logic here, the problem is that my understanding is the current mm process is a bit vague here. Which likely will be also an issue for the mm ci. I’ll merge a support for b4-like dependencies specification soon.
Re re-starting with manual selection it’s on my todo list, but maybe a bit lfurther away, as it requires an authorization, etc.
Thanks
Lorenzo Stoakes (ARM)
maintainerRe: fixing sashiko failure to apply (was Re: [PATCH v5 00/16] mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff)
Quoted 28 lines · Roman Gushchin
Quoted 20 lines
Quoted 20 lines · Matthew Brost
Quoted 10 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
Questioning your assumptions is useful, even when they turn out to be wrong. Show more lines
Quoted 4 lines
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Quoted 7 lines
Quoted 3 lines
Yes, it's kind of odd that Sashiko burns more tokens than a full day of breakfast, lunch, and dinner service. Running Sashiko is a bottleneck in my workflow, so I'll defer to others on this list.
Yup, not sure if there are recommended configs for something saner :)
Maybe Roman has some advice on that?
Quoted 8 lines
Sorry, no magic way to save tokens without hurting the quality. But I am curious what are your numbers? Can be model-dependent too. In prod on average it burns 3-4M tokens per patch with Gemini 3.1 Pro, but maybe mm patches are more complex than average, Idk.
One option is to run only some discovery stages (—stages), but this unlikely will save you that much. I’d say use a cheaper and faster model for the development, but it has it’s downsides too.
If you have an example of a patch(set) which is particularly token-hungry, I can take a look.
Oh well damn, no 3-4M per patch sounds about right actually. I guess that just is what it is then!
Quoted 28 lines
Quoted 28 lines
Quoted 13 lines
Quoted 12 lines
Quoted 6 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
Not sure what the solution is!
Quoted 6 lines
We have seen this on the Xe list (our list is based on drm-tip), typically with cross-subsystem patches. Some cross-subsystem patches apply and run correctly, while others do not but public CI flows run based on drm-tip. I do not have a bisect or a clear understanding of what works and what doesn't, but I think it would be very useful if the community could better understand the root cause.
As Mike said, mm-unstable/mm-new is heavily rebased and also carries the old version of the series before the new one is applied, so it's super unclear what the base commit should be there.
But in general, I wonder if it's possible that we could tell sashiko after-the-fact what base commit to look at once the series is in, or re-trigger it somehow once it's in-tree?
Quoted 8 lines
Quoted 4 lines
Roman - any suggestions on what we could do to help sashiko find things?
(Once mm-next is in place everything with change again, but can address that then :)
I can implement any reasonable logic here, the problem is that my understanding is the current mm process is a bit vague here. Which likely will be also an issue for the mm ci. I’ll merge a support for b4-like dependencies specification soon.
Yeah I suspect things might be tricky with mm given the rebases honestly.
Quoted 3 lines
Re re-starting with manual selection it’s on my todo list, but maybe a bit lfurther away, as it requires an authorization, etc.
Yeah that's the fly in the ointment I guess for many things like giving instant feedback on accuracy, well you want to make sure the person giving it is who you think they are :)
Probably an email -> author with magic link or something but thinking through how to avoid abuse/spam/rate limiting everything etc. is surely all a pain :)
Good to hear it's the TODO list though!
Quoted 2 lines
Thanks
--
Cheers, Lorenzo
Zi Yan
reviewerRe: fixing sashiko failure to apply (was Re: [PATCH v5 00/16] mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff)
<snip>
Quoted 42 lines
Quoted 27 lines
Quoted 27 lines
Quoted 12 lines
Quoted 11 lines
Quoted 5 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
Not sure what the solution is!
Quoted 6 lines
We have seen this on the Xe list (our list is based on drm-tip), typically with cross-subsystem patches. Some cross-subsystem patches apply and run correctly, while others do not but public CI flows run based on drm-tip. I do not have a bisect or a clear understanding of what works and what doesn't, but I think it would be very useful if the community could better understand the root cause.
As Mike said, mm-unstable/mm-new is heavily rebased and also carries the old version of the series before the new one is applied, so it's super unclear what the base commit should be there.
But in general, I wonder if it's possible that we could tell sashiko after-the-fact what base commit to look at once the series is in, or re-trigger it somehow once it's in-tree?
Quoted 8 lines
Quoted 4 lines
Roman - any suggestions on what we could do to help sashiko find things?
(Once mm-next is in place everything with change again, but can address that then :)
I can implement any reasonable logic here, the problem is that my understanding is the current mm process is a bit vague here. Which likely will be also an issue for the mm ci. I’ll merge a support for b4-like dependencies specification soon.
Yeah I suspect things might be tricky with mm given the rebases honestly.
Quoted 3 lines
Re re-starting with manual selection it’s on my todo list, but maybe a bit lfurther away, as it requires an authorization, etc.
Quoted 6 lines
Yeah that's the fly in the ointment I guess for many things like giving instant feedback on accuracy, well you want to make sure the person giving it is who you think they are :)
Probably an email -> author with magic link or something but thinking through how to avoid abuse/spam/rate limiting everything etc. is surely all a pain :)
If b4 is used, the cover letter comes with a fixed change-id. That can be a good id for new patch series replacement. For patches do not use b4, authors can add their own change-id. Without change-id, if the new version comes with a link to the prior version, that can be helpful too. If none is present, author email + patch title might be the last resort. But I think we need to encourage authors to provide some id for their patches to make replacement easier instead of spending too much effort on identifying different patch versions.
Signature · 2 lines
Best Regards, Yan, Zi
Lorenzo Stoakes (ARM)
maintainerRe: fixing sashiko failure to apply (was Re: [PATCH v5 00/16] mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff)
Quoted 50 lines · Zi Yan
<snip>
Quoted 42 lines
Quoted 27 lines
Quoted 27 lines
Quoted 12 lines
Quoted 11 lines
Quoted 5 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
Not sure what the solution is!
Quoted 6 lines
We have seen this on the Xe list (our list is based on drm-tip), typically with cross-subsystem patches. Some cross-subsystem patches apply and run correctly, while others do not but public CI flows run based on drm-tip. I do not have a bisect or a clear understanding of what works and what doesn't, but I think it would be very useful if the community could better understand the root cause.
As Mike said, mm-unstable/mm-new is heavily rebased and also carries the old version of the series before the new one is applied, so it's super unclear what the base commit should be there.
But in general, I wonder if it's possible that we could tell sashiko after-the-fact what base commit to look at once the series is in, or re-trigger it somehow once it's in-tree?
Quoted 8 lines
Quoted 4 lines
Roman - any suggestions on what we could do to help sashiko find things?
(Once mm-next is in place everything with change again, but can address that then :)
I can implement any reasonable logic here, the problem is that my understanding is the current mm process is a bit vague here. Which likely will be also an issue for the mm ci. I’ll merge a support for b4-like dependencies specification soon.
Yeah I suspect things might be tricky with mm given the rebases honestly.
Quoted 3 lines
Re re-starting with manual selection it’s on my todo list, but maybe a bit lfurther away, as it requires an authorization, etc.
Quoted 6 lines
Yeah that's the fly in the ointment I guess for many things like giving instant feedback on accuracy, well you want to make sure the person giving it is who you think they are :)
Probably an email -> author with magic link or something but thinking through how to avoid abuse/spam/rate limiting everything etc. is surely all a pain :)
Quoted 8 lines
If b4 is used, the cover letter comes with a fixed change-id. That can be a good id for new patch series replacement. For patches do not use b4, authors can add their own change-id. Without change-id, if the new version comes with a link to the prior version, that can be helpful too. If none is present, author email + patch title might be the last resort. But I think we need to encourage authors to provide some id for their patches to make replacement easier instead of spending too much effort on identifying different patch versions.
Agree, but with mm constantly rebasing these hashes become potentially less useful :)
Quoted 5 lines
--
Best Regards,
Yan, Zi
--
Cheers, Lorenzo
Mike Rapoport
maintainerQuoted 14 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Quoted 1 line
Quoted 6 lines
Quoted 3 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
It's not only that the tree is rebased, but also that mm-unstable carries the previous version of the patches.
Quoted 4 lines
Not sure what the solution is!
--
Cheers, Lorenzo
Signature · 2 lines
Sincerely yours, Mike.
Lorenzo Stoakes (ARM)
maintainerQuoted 19 lines · Mike Rapoport
Quoted 18 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Quoted 4 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
Quoted 5 lines
Quoted 2 lines
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
It's not only that the tree is rebased, but also that mm-unstable carries the previous version of the patches.
Yeah that's part of the issue, I can't rebase on mm-unstable as a result obviously.
Is hard to know what is sensible to rebase on - if I rebase on base of my old version of the series in mm-unstable that'll probably be a bogus commit at some point due to other rebasing.
Quoted 9 lines
Quoted 5 lines
Not sure what the solution is!
--
Cheers, Lorenzo--
Sincerely yours,
Mike.
--
Cheers, Lorenzo
Mike Rapoport
maintainerQuoted 26 lines · Lorenzo Stoakes (ARM)
Quoted 19 lines · Mike Rapoport
Quoted 18 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Andrew Morton
You'll be mortified to hear that Sashiko wasn't able to find anything to which to apply this.
:))
Well, when it's right it's useful, when it's wrong or suggesting unrelated what-nots it's less useful :>)
I do locally put things through claude + Chris Mason's prompts a lot, I don't always invoke local sashiko as it's very slow and token-heavy or has been so far, but am planning to do that more also in future.
Quoted 4 lines
Sashiko can be guided with a base-commit: tag but I'm not sure how to tell it what tree/branch to try, or even if that's necessary. Perhaps someone can figure this out sometime.
Quoted 5 lines
Quoted 2 lines
b4 gives a base commit, but I think because the trees are rebased it ends up being the incorrect one.
It's not only that the tree is rebased, but also that mm-unstable carries the previous version of the patches.
Quoted 6 lines
Yeah that's part of the issue, I can't rebase on mm-unstable as a result obviously.
Is hard to know what is sensible to rebase on - if I rebase on base of my old version of the series in mm-unstable that'll probably be a bogus commit at some point due to other rebasing.
If the work does not depend on other commits in mm-unstable, it's possible to base on -rcX. But often it's not the case.
Quoted 11 lines
Quoted 8 lines
Quoted 4 lines
Not sure what the solution is!
--
Cheers, Lorenzo--
Sincerely yours,
Mike.--
Cheers, Lorenzo
Signature · 2 lines
Sincerely yours, Mike.