A race in vm_page_busy_sleep(9).

[kib]

Summary

Suppose that we have an exclusively busy page, and a thread which can accept shared-busy page. In this case, typical code waiting for the page xbusy state to pass is

 		if (vm_page_xbusied(m)) {
                        vm_page_lock(m);
 			VM_OBJECT_WUNLOCK(object);    <---1
			vm_page_busy_sleep(p, "vmopax");
 			goto again;
 		}

Suppose that the xbusy state owner locked the object, unbusied the page and unlocked the object after we are at the line 1, but before we executed the load of the busy_lock word in vm_page_busy_sleep(). If it happens that there is still no waiters recorded for the busy state, the xbusy owner did not acquired the page lock, so it proceeded.

More, suppose that some other thread happen to share-busy the page after xbusy state was relinquished but before the m->busy_lock is read in vm_page_busy_sleep(). Again, that thread only need vm_object lock to proceed. Then, vm_page_busy_sleep() reads busy_lock value equal to the VPB_SHARERS_WORD(1)

In this case, all tests in vm_page_busy_sleep(9) pass and we are going to sleep, despite the page being share-busied.

Update check for m->busy_lock == VPB_UNBUSIED in vm_page_busy_sleep(9) to accept shared-busy state if we only wait for the xbusy state to pass.

Test Plan

Apparently current code does not share-busy pages from parallel threads, but I have plans for it.

[alc]

I just wanted to confirm a couple things that aren't explicitly stated in the summary.

1. When the xbusy owner reacquires and releases the object lock, it is relinquishing its xbusy claim on the page.

2. Because there are no waiters at the time that the xbusy owner relinquishes its claim, it can do so with only an atomic cmpset. In particular, it doesn't have to acquire the page lock. Moreover, another thread can sbusy the page with only an atomic cmpset.

[alc]

sys/vm/vm_page.c

752 ↗

(On Diff #21184)

Since we're modifying the nearby code can we switch this assert to the newer form: vm_page_assert_locked(m)

[kib]

In D8196#170863, @alc wrote:

I just wanted to confirm a couple things that aren't explicitly stated in the summary.

1. When the xbusy owner reacquires and releases the object lock, it is relinquishing its xbusy claim on the page.

By the 'the xbusy owner' you mean the thread which unbusies the xbusy state in my scenario ? Yes.

2. Because there are no waiters at the time that the xbusy owner relinquishes its claim, it can do so with only an atomic cmpset. In particular, it doesn't have to acquire the page lock. Moreover, another thread can sbusy the page with only an atomic cmpset.

Again, yes, applied to the share-busy owner == "another thread" in my scenario.

I edited the description of the review to hopefully address you notes.

[kib]

Change the style of the page lock assert.

Merge sequential if()s with the same 'then' clause in vm_page_busy_sleep().

[alc]

As an aside, I think it would be worthwhile to add comments to the page busy code that describe how the page lock is (and isn't) used. For example, I think that a casual reader of the page busy code might think that the page lock is protecting against races like you've addressed in this change.