Page MenuHomeFreeBSD

(vm object 5) Move PGA_NOSYNC to VPA_NOSYNC so that the object lock is not required.
ClosedPublic

Authored by jeff on Sep 10 2019, 8:20 PM.

Details

Summary

This patch simply moves the flag field and makes it atomic to eliminate the final dependency on the object lock in vm_page_set_validclean().

I do not fully understand the race described in vm_fault.c:~225 that requires the page lock. This may be the final vestige of page_lock after Mark and I are done so it would be good for someone to suggest a new approach and especially to review this patch in that light. I believe what I have done is correct however.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jeff retitled this revision from Move PGA_NOSYNC to VPA_NOSYNC so that the object lock is not required. to (vm object 5) Move PGA_NOSYNC to VPA_NOSYNC so that the object lock is not required..Sep 10 2019, 8:23 PM
jeff edited the summary of this revision. (Show Details)
jeff added reviewers: alc, kib, markj, dougm.

We have MAP_NOSYNC flag for mmap(2). Suppose we have one mapping with the flag set, and other with the flag clear. Then write fault for the page might come from either mappings, and we either want or not want to set VPO_NOSYNC depending from which mapping the write fault is coming. More, if the VPO_NOSYNC page is re-dirtied from non-NOSYNC mapping, we want to remove VPO_NOSYNC.

Of course this is very weird because of contradictory requirements and the fact that write mappings are not invalidated when the NOSYNC is set. Nonetheless, MAP_NOSYNC is apparently used by users, see where it all started at https://lists.freebsd.org/pipermail/freebsd-hackers/2014-September/045981.html

This revision is now accepted and ready to land.Sep 22 2019, 1:21 PM
markj added inline comments.
sys/vm/vm_page.c
2485 ↗(On Diff #61912)

See my comment below.

2629 ↗(On Diff #61912)

The comment was referring to stray oflags, so it doesn't really make sense here anymore. I think you can just delete it.