[v3,0/5] mm: further simplify VMA merge operation

Message

Lorenzo Stoakes Jan. 31, 2025, 12:31 p.m. UTC

While significant efforts have been made to improve the VMA merge
operation, there remains remnants of the bad (or rather confusing) old
days, which make the code difficult to understand, more bug prone and
thus harder to modify.

This series attempts to significantly improve matters in a number of
respects - with a focus on simplifying the commit_merge() function which
actually actions the merge operation - and importantly, adjusting the two
most confusing merge cases - those in which we 'adjust' the VMA immediately
adjacent to the one being merged.

One source of confusion are the VMAs being threaded through the operation
themselves - vmg->prev, vmg->vma and vmg->next.

At the start of the operation, vmg->vma is either NULL if a new VMA is
propose to be added, or if not then a pointer to an existing VMA being
modified, and prev/next are (perhaps not present) VMAs sat immediately
before and after the range specified in vmg->start, end, respectively.

However, during the VMA merge operation, we change vmg->start, end and
pgoff to span the newly merged range and vmg->vma to either be:

a. The ultimately returned VMA (in most cases) or
b. A VMA which we will manipulate, but ultimately instead return vmg->next.

Case b. especially here is confusing for somebody reading this code, but
the fact we update this state, along with vmg->start, end, pgoff only makes
matters worse.

We simplify things by replacing vmg->vma with vmg->middle and never
changing it - this is always either NULL (for a new VMA) or the VMA being
modified between vmg->prev and vmg->next.

We further simplify by placing the merged VMA in a new vmg->target field -
whether case b. above is the case or not. The reader of the code can now
simply rely on vmg->middle being the middle VMA and vmg->target being the
ultimately merged VMA.

We additionally tackle the confusing cases where we 'adjust' VMAs other
than the one we ultimately return as the merged VMA (this includes case
b. above). These are:

(1)
	    merge
	<----------->
	|------||--------|    |------------|---|
	| prev || middle | -> |   target   | m |
	|------||--------|    |------------|---|

In which case middle must be adjusted so middle->vm_start is increased as
well as performing the merge.

(2) (equivalent to case b. above)

            <------------->
	|---------||------|    |---|-------------|
	|  middle || next | -> | m |   target    |
	|---------||------|    |---|-------------|

In which case next must be adjusted so next->vm_start is decreased as well
as performing the merge.

This cases have previously been performed by calculating and passing around
a dubious and confusing 'adj_start' parameter along side a pointer to an
'adjust' VMA indicating which VMA requires additional adjustment (middle in
case 1 and next in case 2).

With the VMG structure in place we are able to avoid this by simply setting
a merge flag to describe each case:

(1) Sets the vmg->__adjust_middle_start flag
(2) Sets the vmg->__adjust_next_start flag

By doing so it turns out we can vastly simplify the logic and calculate
what is required to perform the operation.

Taken together the refactorings make it far easier to understand what is
being done even in these more confusing cases, make the code far more
maintainable, debuggable, and testable, providing more internal state
indicating what is happening in the merge operation.

The changes have no functional net impact on the merge operation and
everything should still behave as it did before.

v3:
* Fix seemingly compiler-version dependent printf() specifier formatter issue
  in mm/debug.c per buildbot complaints.

v2:
* Fixed buildbot warnings due to missing a change to a static inline
  stubbed out wrapper for vma_adjust_trans_huge().
* Converted vmg merge_flags to bitfield flags as per Vlastimil.
* Update comments which were rendered inaccurate in 5/5 as per Vlastimil.
https://lore.kernel.org/all/[e-mail]/

v1:
https://lore.kernel.org/all/[e-mail]/

Lorenzo Stoakes (5):
  mm: simplify vma merge structure and expand comments
  mm: further refactor commit_merge()
  mm: eliminate adj_start parameter from commit_merge()
  mm: make vmg->target consistent and further simplify commit_merge()
  mm: completely abstract unnecessary adj_start calculation

 include/linux/huge_mm.h          |   4 +-
 mm/debug.c                       |  28 +--
 mm/huge_memory.c                 |  19 +-
 mm/mmap.c                        |   2 +-
 mm/vma.c                         | 327 ++++++++++++++++---------------
 mm/vma.h                         |  86 ++++++--
 tools/testing/vma/vma.c          |  63 +++---
 tools/testing/vma/vma_internal.h |   4 +-
 8 files changed, 302 insertions(+), 231 deletions(-)

--
2.48.1