Add an explicit free state to busy_lock so that we can handle transientsfrom lockless lookup.
ClosedPublic
Actions

Authored by jeff on Jan 31 2020, 11:00 PM.

Details

Reviewers

alc
dougm
kib
markj

Commits

rS357528: Add an explicit busy state for free pages. This improves behavior with

Summary

This adds a free state to the busy lock. This is necessary for lockless page lookup. If we follow a stale pointer to a page we may find that it has been freed. We may also acquire a busy lock on a page and subsequently have it freed while we are holding it. Lockless lookup must handle both these cases and it is facilitated by this special free value.

This is also useful because buggy code could currently find a stale page and lock it without any error. This lock would then be released silently when the page was allocated. Subsequently both the allocator and the buggy thread could release the same lock.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 29084
Build 27030: arc lint + arc unit

Event Timeline

jeff created this revision.Jan 31 2020, 11:00 PM

Harbormaster completed remote builds in B29084: Diff 67590.Jan 31 2020, 11:00 PM

jeff retitled this revision from Add an explicit free state to busy_lock so that we can handle transients from lockless lookup. to Add an explicit free state to busy_lock so that we can handle transientsfrom lockless lookup..Jan 31 2020, 11:06 PM

jeff edited the summary of this revision. (Show Details)

jeff added reviewers: alc, dougm, kib, markj.

jeff set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptJan 31 2020, 11:07 PM

kib accepted this revision.Feb 1 2020, 11:11 PM

kib added inline comments.

sys/vm/vm_page.h
707	This (significantly) weakens the check in vm_object_terminate_pages(). Might be check that the page is !vm_page_busied() or it is busy_freed ? Also this macro usefulness is doubtful, the check can be done explicitly with less obfuscation in vm_object_terminate_pages().

This revision is now accepted and ready to land.Feb 1 2020, 11:11 PM

jeff added inline comments.Feb 1 2020, 11:16 PM

sys/vm/vm_page.h
707	This is compatible with things like mtx unlocked asserts. Because lockless lookup will involve transient busy acquires for incorrect pages you can only assert about the lock state relative to the current thread. Another thread may follow a stale radix pointer and busy a page only to see that it is not the correct one and start over. This is the same pattern I used in fget_unlocked().