vm_fault: drop never-true busy_sleep test

[dougm]

Summary

vm_fault_busy_sleep() tests to see whether the page fs->m matches the value looked up at fs->pindex. At that point, a lock is held on fs->object, and it has been held since before vm_fault_object() also looked up fs->pindex in fs->object and stored the result in fs->m. So the values must match, and the test is not necessary. Drop it.

Test Plan

Replacing the useless test with an assertion, and asserting that the two values match instead, produced no assertion failures in running the first 204 (and counting) stress2 tests.

[kib]

Then why not convert the check into the assertion in the final patch too? IMO it is good to have, since the path from lookup to this busy_sleep() line is somewhat long.

[alc]

This lookup originated here:

commit 87b646630c4892e21446cd096bea6bcaecea33ac
Author: Mark Johnston <[e-mail]>
Date:   Mon Nov 15 11:35:44 2021 -0500

    vm_page: Consolidate page busy sleep mechanisms
    
    - Modify vm_page_busy_sleep() and vm_page_busy_sleep_unlocked() to take
      a VM_ALLOC_* flag indicating whether to sleep on shared-busy, and fix
      up callers.
    - Modify vm_page_busy_sleep() to return a status indicating whether the
      object lock was dropped, and fix up callers.
    - Convert callers of vm_page_sleep_if_busy() to use vm_page_busy_sleep()
      instead.
    - Remove vm_page_sleep_if_(x)busy().
    
    No functional change intended.
    
    Obtained from:  jeff (object_concurrency patches)
    Reviewed by:    kib
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D32947

[alc]

In D51179#1168325, @alc wrote:

This lookup originated here:

commit 87b646630c4892e21446cd096bea6bcaecea33ac
Author: Mark Johnston <[e-mail]>
Date:   Mon Nov 15 11:35:44 2021 -0500

    vm_page: Consolidate page busy sleep mechanisms
    
    - Modify vm_page_busy_sleep() and vm_page_busy_sleep_unlocked() to take
      a VM_ALLOC_* flag indicating whether to sleep on shared-busy, and fix
      up callers.
    - Modify vm_page_busy_sleep() to return a status indicating whether the
      object lock was dropped, and fix up callers.
    - Convert callers of vm_page_sleep_if_busy() to use vm_page_busy_sleep()
      instead.
    - Remove vm_page_sleep_if_(x)busy().
    
    No functional change intended.
    
    Obtained from:  jeff (object_concurrency patches)
    Reviewed by:    kib
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D32947

This lookup was first introduced between stable/4 and stable/6. The lookup was needed in the latter because we were dropping and reacquiring the object lock around the map unlock and possible vput().

[dougm]

In D51179#1168320, @kib wrote:

Then why not convert the check into the assertion in the final patch too? IMO it is good to have, since the path from lookup to this busy_sleep() line is somewhat long.

I don't want to vm_page_lookup appear here without cause, since it would have to be changed to vm_page_lookup_readonly in a future potential change.

[dougm]

In D51179#1168320, @kib wrote:

Then why not convert the check into the assertion in the final patch too? IMO it is good to have, since the path from lookup to this busy_sleep() line is somewhat long.

Because I don't want to have to change this function again in a few days when/if vm_page_lookup_readonly is introduced.

[kib]

In D51179#1168451, @dougm wrote:

In D51179#1168320, @kib wrote:

Then why not convert the check into the assertion in the final patch too? IMO it is good to have, since the path from lookup to this busy_sleep() line is somewhat long.

Because I don't want to have to change this function again in a few days when/if vm_page_lookup_readonly is introduced.

You could assert fs->m->pindex == fs->pindex.