Page MenuHomeFreeBSD

vm_object_coalesce(): do not account holes twice
Needs ReviewPublic

Authored by kib on Thu, Nov 27, 9:58 PM.
Tags
None
Referenced Files
F140009633: D53965.id167213.diff
Thu, Dec 18, 11:47 PM
Unknown Object (File)
Mon, Dec 15, 2:27 PM
Unknown Object (File)
Mon, Dec 15, 12:26 PM
Unknown Object (File)
Thu, Dec 11, 12:20 AM
Unknown Object (File)
Wed, Dec 10, 4:18 AM
Unknown Object (File)
Sat, Dec 6, 12:27 AM
Unknown Object (File)
Thu, Dec 4, 3:41 AM
Unknown Object (File)
Wed, Dec 3, 4:03 PM

Details

Reviewers
alc
markj
Summary
alc wrote:
Suppose that the object is OBJ_ONEMAPPING and that we, in fact,
have a single mapping to it. Then, we punch a hole in that mapping.
vm_map_entry_delete() only subtracts from the object's charge when
we shrink the size of the object. Now, suppose that we perform
mmap(MAP_ANON) to reallocate some of the hole. Aren't we going to add to
the charge here, even though vm_map_entry_delete() never subtracted from
the charge for the hole that was created?

Only account the change in the charged object size that was added to it.
Also remove ifdef-ed block that tried to correct charge and that is
not needed.

Noted by:       alc

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Thu, Nov 27, 9:58 PM

vm_map_entry_delete() only subtracts from the object's charge when we shrink the size of the object.

Isn't that the bug? That is, shouldn't vm_map_entry_delete() subtract the size of the hole from the charge if the object has only one mapping?

vm_map_entry_delete() only subtracts from the object's charge when we shrink the size of the object.

Isn't that the bug? That is, shouldn't vm_map_entry_delete() subtract the size of the hole from the charge if the object has only one mapping?

It can be interpreted this way, but fixing it this way is IMO impossible. We do not track dead ranges in the object, but we must do it to handle the case if the range is revived, e.g. by coalescing. Right now the charge is more or less the max of the used range end. Sometimes I even think that object->charge was redundant, and just a flag like charged should mean that whole object size is accounted to object->cred.

IMO the fix I propose is in line with other parts of the swap accounting code.