VM caches aren't resized in some cases when cutting areas

[hamish]

In some cases, if an area is partially mapped over (using mmap with MAP_FIXED, for example) the cache is not resized or split in two, resulting in memory being leaked for the overlapping region.

See the TODOs in cut_area(): ​http://cgit.haiku-os.org/haiku/tree/src/system/kernel/vm/vm.cpp#n609

[hamish]

Here's what I'm thinking of doing:

To handle the cut-start-of-area case, add a VMCache::Rebase() method as a counterpart to VMCache::Resize() which will move the virtual base of cache and free any pages no longer contained in the region.

To handle the cut-middle-of-area case, add a VMCache::MovePageRange(off_t offset, off_t size, VMCache* destination, off_t destinationOffset) method. Then create a new cache for the second area and move the pages there. I realise this is inconsistent with the other page moving methods because they are called on the destination cache, but it would be nice to iterate the page tree directly instead of calling LookupPage() repeatedly on the source cache.

Does this sound OK?

[bonefish]

Replying to hamish:

Here's what I'm thinking of doing:

To handle the cut-start-of-area case, add a VMCache::Rebase() method as a counterpart to VMCache::Resize() which will move the virtual base of cache and free any pages no longer contained in the region.

Sounds good.

To handle the cut-middle-of-area case, add a VMCache::MovePageRange(off_t offset, off_t size, VMCache* destination, off_t destinationOffset) method. Then create a new cache for the second area and move the pages there. I realise this is inconsistent with the other page moving methods because they are called on the destination cache, but it would be nice to iterate the page tree directly instead of calling LookupPage() repeatedly on the source cache.

I don't see what would speak against iterating through the other cache's page tree.

As an alternative solution a status_t SplitCache(off_t offset, VMCache*& _newCache) could be introduced (or with a pre-allocated cache object, if that makes things easier to use). Would be less flexible, but ATM I can't think of a use case where additional flexibility was required.

[hamish]

Replying to bonefish:

I don't see what would speak against iterating through the other cache's page tree.

Indeed, I didn't realise that the page tree was a public member.

This patch depends on my patch in #8233.

RE your comment there about guard sizes and cache resizing, I'm not sure quite what would be the expected behaviour in this case. Should the guard size be preserved after resizing, or should the guard be resized to prevent previously accessible pages becoming guarded? So I added a VMAnonymous(NoSwap)Cache::SetGuardSize() to let the caller decide what's best for them.

[hamish]

Just realised this doesn't deal with swap properly in the middle-cut case.

[waddlesplash]

Finished by mmlr and applied (with other changes) in hrev54148.

[nielx]

Assign tickets with status=closed and resolution=fixed within the R1/beta2 development window to the R1/beta2 Milestone