Update vm object lock assertions in the swap pager
ClosedPublic
Actions

Authored by alc on Aug 26 2017, 9:11 PM.

Details

Reviewers

kib
markj

Commits

rS322971: Update a couple vm_object lock assertions in the swap pager to reflect the
rS322970: Switching from a global hash table to per-vm_object radix tries for mapping

Summary

Update a couple vm object lock assertions in the swap pager to reflect the new use of the vm object's lock to synchronize updates to a radix trie mapping per-vm object page indices to on-disk swap blocks.

Fix a typo in a nearby comment.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

alc created this revision.Aug 26 2017, 9:11 PM

kib accepted this revision.Aug 26 2017, 9:22 PM

This revision is now accepted and ready to land.Aug 26 2017, 9:22 PM

markj accepted this revision.Aug 26 2017, 11:02 PM

This change reveals a problem: vm_fault_soft_fast() is (indirectly) calling vm_pager_page_unswapped() with only a read lock.

In D12134#251899, @alc wrote:

This change reveals a problem: vm_fault_soft_fast() is (indirectly) calling vm_pager_page_unswapped() with only a read lock.

My understanding is that this is a valid assert in the following situation: we have a swap object with a valid page which was swapped in and not dirtied. Then, on a write fault, any (fast or normal) fault handlers could free the swap space. It is possible because swap pager page-in leaves the backed swap space intact as (IMO very small) optimization.

I think that as a solution we can leave the swap allocated. If the page going to swap later again, swp_pager_meta_build() correctly frees the old space. This causes some time-limited leak of the swap space, which is IMO acceptable.

Update vm_fault_dirty().

This revision now requires review to proceed.Aug 28 2017, 4:32 AM

In D12134#251911, @kib wrote:

In D12134#251899, @alc wrote:

This change reveals a problem: vm_fault_soft_fast() is (indirectly) calling vm_pager_page_unswapped() with only a read lock.

My understanding is that this is a valid assert in the following situation: we have a swap object with a valid page which was swapped in and not dirtied. Then, on a write fault, any (fast or normal) fault handlers could free the swap space. It is possible because swap pager page-in leaves the backed swap space intact as (IMO very small) optimization.

It might also be the case that we write fault on a just laundered page before it is reclaimed.

I think that as a solution we can leave the swap allocated. If the page going to swap later again, swp_pager_meta_build() correctly frees the old space. This causes some time-limited leak of the swap space, which is IMO acceptable.

Agreed.

kib accepted this revision.Aug 28 2017, 8:43 AM

This revision is now accepted and ready to land.Aug 28 2017, 8:43 AM

markj accepted this revision.Aug 28 2017, 2:16 PM

Closed by commit rS322970: Switching from a global hash table to per-vm_object radix tries for mapping (authored by alc). · Explain WhyAug 28 2017, 4:55 PM

This revision was automatically updated to reflect the committed changes.